-
Notifications
You must be signed in to change notification settings - Fork 901
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
Allocator support exitOnOutOfMemory config. #3984
Allocator support exitOnOutOfMemory config. #3984
Conversation
@horizonzy Please fix the check style, thanks. |
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.
Good job!
@lhotari Please help take a look, thanks. |
The ci failed. Fixing it now. |
done |
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.
great work!
rerun failure checks |
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.
could you please rebase this code? we have deprecated powermock usage now.
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.
LGTM
*/ | ||
@Slf4j | ||
public class ShutdownUtil { | ||
private static final Method log4j2ShutdownMethod; |
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.
I have a question, this is copied from pulsar-common directly, why not import pulsar-common in bookkeeper/bookkeeper-common-allocator/pom.xml
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.
Except for the ShutdownUtil problem, the rest are pretty good,LGTM @horizonzy
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.
It's a minor import, we needn't import new dependency.
please fix the checkstyle issue, thanks @horizonzy |
Descriptions of the changes in this PR:
In some cases, if the process can't allocate memory, we would better exit the JVM, or the bookie can't serve, and the client always timeout.
Add config
allocatorExitOnOutOfMemory
. When the allocator encounters oom, exit the JVM.