Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Issue #4890 trigger leak analysis #5295

Draft
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

reinhapa
Copy link
Member

Analysis and simplifications to tackle the leaking trigger states.

@reinhapa reinhapa force-pushed the bugfix/issue4890_restore_memory_leak branch from 37ec663 to 16b072b Compare April 30, 2024 19:39
@reinhapa
Copy link
Member Author

@line-o I did some clean ups in preparation for looking into the actual leak. As soon I got your minimal restore example, I try to setup a according test, that may show the issue and can be used to reproduce.

@adamretter
Copy link
Contributor

@reinhapa can you tell me a little about your thinking around removing the ThreadLocal please?

@reinhapa
Copy link
Member Author

@reinhapa can you tell me a little about your thinking around removing the ThreadLocal please?

It's to get more easy access to all currently known per-thread entries in order to find the actual leak behaviour as those can be found more easy as within the thread local variable. It's not intended to stay that way when I got to the actual ground of the problem.

@line-o
Copy link
Member

line-o commented May 1, 2024

@reinhapa here is the backup to restore
full-backup-restore-with-triggers.zip

The collection where an update trigger is configured is /db/apps/restore-with-trigger-test

@adamretter
Copy link
Contributor

to find the actual leak behaviour

What is the leak that you are mentioning? is there a GitHub issue for this I can take a look at?

@reinhapa
Copy link
Member Author

reinhapa commented May 2, 2024

What is the leak that you are mentioning? is there a GitHub issue for this I can take a look at?

@adamretter its issue #4890 where trigger state objects accumulate and only released after the full backup is complete (as I understand @line-o)

@line-o
Copy link
Member

line-o commented May 2, 2024

They do not get removed after the backup is completed. One needs to restart the instance to get rid of them.

@reinhapa reinhapa force-pushed the bugfix/issue4890_restore_memory_leak branch from 5eef0e2 to 2c5b318 Compare May 14, 2024 05:06
reinhapa added 6 commits May 27, 2024 19:56
- Use default 'equals' implementation of record
- Store all thread related data within a central concurrent hash map in
  order to better track potential leaking data
@reinhapa reinhapa force-pushed the bugfix/issue4890_restore_memory_leak branch from 474e578 to 1473c21 Compare May 27, 2024 17:56
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot
51.9% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@dizzzz
Copy link
Member

dizzzz commented Aug 26, 2024

disable triggers -> breaks stuff ?

@adamretter
Copy link
Contributor

See: #4890 (comment)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants