-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
This commit will fix defect 28687 #59
Conversation
@sekler please write a comprehensive description of the bug you want to fix |
Updated the commit description |
if (System.currentTimeMillis() > (gcTimer + MINIMUM_GC_DELTA)){ | ||
logger.warn("Trying to initiate garbage collection"); | ||
//Todo find a better solution than manually triggering GC | ||
System.gc(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe add a log msg after the GC ended? at least in debug...
any test for this? |
This commit will fix defect 28687
Sorry for responding so late. But it just seem so wrong!! Why not wait for the JVM to trigger GC? And really, there should really be a flag to turn this hack off. |
And last but completely not least. Don't add todos to code, either open an issue or rethink-it. score code is not for handling tasks. |
Hi :) That is the issue. That in current check if we see that the memory is about to end we do not poll - meaning do not ask java for more memory. That is why JVM does not do GC. It stays this way for minutes not polling and not doing GC (proven in profiler). We could completely remove the check of memory, but then we will fail on OutOfMemory or we can ask JVM to GC and once it is done we can continue to poll. Regarding the flag to disable it - I agree. Will be added. |
This fixed was issued because we saw flows being delayed for minutes due to false positive check on the free memory, the bottom line is that our current check is naive, and does not take into account what can be freed in the heap, IMO it's either the added GC, or we should remove this check completely as it does not reflect the actual state of the JVM |
Agree with @orius123 on the flag to turn this off!! |
#60 : Add a flag to turn GC hack off |
The defect described flows that are executed in delay, this was caused by a false triggering of the defense mechanism in the InBuffer that does not pull new messages when there are less than 50Mb of free memory, however this mechanism does not take into account GC, and the amount of memory that can be released as a result of GC