Skip to content
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

Merged
merged 11 commits into from
Jul 15, 2024

Conversation

horizonzy
Copy link
Member

@horizonzy horizonzy commented Jun 12, 2023

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.

@hangc0276
Copy link
Contributor

@horizonzy Please fix the check style, thanks.

Copy link
Contributor

@hangc0276 hangc0276 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job!

@hangc0276
Copy link
Contributor

@lhotari Please help take a look, thanks.

@horizonzy
Copy link
Member Author

horizonzy commented Jun 13, 2023

The ci failed. Fixing it now.

@horizonzy
Copy link
Member Author

The ci failed. Fixing it now.

done

Copy link
Contributor

@dlg99 dlg99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great work!

@horizonzy
Copy link
Member Author

rerun failure checks

Copy link
Member

@shoothzj shoothzj left a 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.

Copy link
Member

@StevenLuMT StevenLuMT left a 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;
Copy link
Member

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

Copy link
Member

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

Copy link
Member Author

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.

@StevenLuMT
Copy link
Member

image please fix the checkstyle issue, thanks @horizonzy

@StevenLuMT StevenLuMT merged commit 15b106c into apache:master Jul 15, 2024
22 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants