Skip to content

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

@hezhangjian hezhangjian 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
lhotari pushed a commit that referenced this pull request Apr 16, 2025
* Allocator support exitOnOutOfMemory config.

(cherry picked from commit 15b106c)
lhotari pushed a commit that referenced this pull request Apr 16, 2025
* Allocator support exitOnOutOfMemory config.

(cherry picked from commit 15b106c)
priyanshu-ctds pushed a commit to datastax/bookkeeper that referenced this pull request Jul 11, 2025
* Allocator support exitOnOutOfMemory config.

(cherry picked from commit 15b106c)
(cherry picked from commit 7b31933)
sandeep-ctds pushed a commit to datastax/bookkeeper that referenced this pull request Jul 22, 2025
* Allocator support exitOnOutOfMemory config.

(cherry picked from commit 15b106c)
(cherry picked from commit 7b31933)
@hangc0276
Copy link
Contributor

@StevenLuMT This PR changed the interface, and we should not cherry-pick it to the patch branch.

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.

6 participants