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

[fix] [cli] Fix Broker crashed by too much memory usage of pulsar tools #20031

Merged
merged 2 commits into from
Apr 11, 2023

Conversation

poorbarcode
Copy link
Contributor

@poorbarcode poorbarcode commented Apr 6, 2023

Motivation

After #15868, we allow PULSAR_MEM & PULSAR_GC to be overridden in pulsar_tool_env.sh.

Many users set -Xms to 2G or larger in PULSAR_MEM, this will make the tools(such as pulsar-admin) cost a lot of memory, and if users execute pulsar-admin or another tool on the machine where the Broker is deployed, the current device will not have enough memory to allocate, resulting in a broker crash.

Modifications

When PULSAR_MEM is overridden in pulsar_tool_env.sh, delete parameter -Xms

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Apr 6, 2023
@poorbarcode poorbarcode changed the title [fix] [cli] fix too much memory used by pulsar tools [fix] [cli] Fix Broker crashed by too much memory usage of pulsar tools Apr 6, 2023
@poorbarcode poorbarcode self-assigned this Apr 6, 2023
@poorbarcode poorbarcode added this to the 3.0.0 milestone Apr 6, 2023
# Discard parameter "-Xms" of $PULSAR_MEM, which tends to be the Broker's minimum memory, to avoid using too much
# memory by tools.
if [ -n "$PULSAR_MEM" ]; then
PULSAR_MEM_REWRITE="-Xmx128m -XX:MaxDirectMemorySize=128m"
Copy link
Member

Choose a reason for hiding this comment

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

This shell seems to set PULSAR_MEM to -Xmx128m -XX:MaxDirectMemorySize=128m?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If PULSAR_MEM exists, -Xmx and -XX:MaxDirectMemorySize will be overridden to 128m, and other options in PULSAR_MEM will not be changed.

Copy link
Member

@coderzc coderzc Apr 10, 2023

Choose a reason for hiding this comment

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

We can just remove the -Xms, -Xmx and -XX:MaxDirectMemorySize don't need overriding to 128m

Copy link
Member

@coderzc coderzc left a comment

Choose a reason for hiding this comment

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

LGTM

@poorbarcode poorbarcode modified the milestones: 3.0.0, 3.1.0 Apr 10, 2023
@poorbarcode poorbarcode force-pushed the fix/tools_use_a_log_mem branch from eb60a33 to 20715ec Compare April 11, 2023 05:38
@poorbarcode
Copy link
Contributor Author

rebase master

@poorbarcode poorbarcode force-pushed the fix/tools_use_a_log_mem branch from f57f10e to 20715ec Compare April 11, 2023 07:59
@poorbarcode poorbarcode modified the milestones: 3.1.0, 3.0.0 Apr 11, 2023
@poorbarcode poorbarcode merged commit 4f503fd into apache:master Apr 11, 2023
@poorbarcode poorbarcode deleted the fix/tools_use_a_log_mem branch April 11, 2023 13:47
poorbarcode added a commit that referenced this pull request Apr 11, 2023
…ls (#20031)

### Motivation
After #15868, we allow `PULSAR_MEM` & `PULSAR_GC` to be overridden in `pulsar_tool_env.sh`.

Many users set `-Xms` to `2G` or larger in `PULSAR_MEM`, this will make the tools(such as `pulsar-admin`) cost a lot of memory, and if users execute `pulsar-admin` or another tool on the machine where the Broker is deployed, the current device will not have enough memory to allocate, resulting in a broker crash.

### Modifications

When `PULSAR_MEM` is overridden  in `pulsar_tool_env.sh`, delete parameter `-Xms`

(cherry picked from commit 4f503fd)
poorbarcode added a commit that referenced this pull request Apr 11, 2023
…ls (#20031)

### Motivation
After #15868, we allow `PULSAR_MEM` & `PULSAR_GC` to be overridden in `pulsar_tool_env.sh`.

Many users set `-Xms` to `2G` or larger in `PULSAR_MEM`, this will make the tools(such as `pulsar-admin`) cost a lot of memory, and if users execute `pulsar-admin` or another tool on the machine where the Broker is deployed, the current device will not have enough memory to allocate, resulting in a broker crash.

### Modifications

When `PULSAR_MEM` is overridden  in `pulsar_tool_env.sh`, delete parameter `-Xms`

(cherry picked from commit 4f503fd)
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request May 11, 2023
…ls (apache#20031)

### Motivation
After apache#15868, we allow `PULSAR_MEM` & `PULSAR_GC` to be overridden in `pulsar_tool_env.sh`.

Many users set `-Xms` to `2G` or larger in `PULSAR_MEM`, this will make the tools(such as `pulsar-admin`) cost a lot of memory, and if users execute `pulsar-admin` or another tool on the machine where the Broker is deployed, the current device will not have enough memory to allocate, resulting in a broker crash.

### Modifications

When `PULSAR_MEM` is overridden  in `pulsar_tool_env.sh`, delete parameter `-Xms`

(cherry picked from commit 4f503fd)
(cherry picked from commit 1fe05d5)
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.

5 participants