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

Setting PULSAR_EXTRA_OPTS causes the process to run without significant default JVM options #13382

Open
lhotari opened this issue Dec 17, 2021 · 5 comments
Assignees
Labels
lifecycle/stale Stale type/bug The PR fixed a bug or issue reported a bug

Comments

@lhotari
Copy link
Member

lhotari commented Dec 17, 2021

Describe the bug
When setting PULSAR_EXTRA_OPTS (or BOOKIE_EXTRA_OPTS), the expectation would be that the JVM options would be added to the existing JVM arguments. However, this is not the case.

If PULSAR_EXTRA_OPTS isn't set, these parameters will get set -Dpulsar.allocator.exit_on_oom=true -Dio.netty.recycler.maxCapacityPerThread=4096 .
If you set PULSAR_EXTRA_OPTS, these parameters will be omitted. For example, the pulsar.allocator.exit_on_oom setting will exit the JVM if the PulsarByteBufAllocator cannot allocate a ByteBuf.

Expected behavior
Setting PULSAR_EXTRA_OPTS shouldn't remove the default settings, but instead add to them. One possible solution would be to make the PULSAR_EXTRA_OPTS setting in pulsar_env.sh to be made this way:

# Extra options to be passed to the jvm
PULSAR_EXTRA_OPTS="-Dpulsar.allocator.exit_on_oom=true -Dio.netty.recycler.maxCapacityPerThread=4096
 ${PULSAR_EXTRA_OPTS}"

A similar problem applies to BOOKIE_EXTRA_OPTS handling. Which is used when bin/pulsar bookie is called.

Additional context

Other improvements for environment variable handling was recently done in PR #13025

@sijie
Copy link
Member

sijie commented Dec 17, 2021

@lhotari: in the opposite, people should have the ability to overwrite the defaults. I don't think keeping the defaults is a good idea. If people want to update the PULSAR_EXTRA_OPTS, they should just put all the settings they need there.

@eolivelli
Copy link
Contributor

probably we could have two variables in order to full fill both of the needs

@lhotari
Copy link
Member Author

lhotari commented Dec 17, 2021

@lhotari: in the opposite, people should have the ability to overwrite the defaults. I don't think keeping the defaults is a good idea. If people want to update the PULSAR_EXTRA_OPTS, they should just put all the settings they need there.

I agree.

@sijie Do you see a problem in the behavior that setting PULSAR_EXTRA_OPTS causes the process to run without significant default JVM options, for example -Dpulsar.allocator.exit_on_oom=true?

How does anyone know that relevant settings get overridden when setting PULSAR_EXTRA_OPTS?

@github-actions
Copy link

The issue had no activity for 30 days, mark with Stale label.

@github-actions
Copy link

The issue had no activity for 30 days, mark with Stale label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/stale Stale type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

No branches or pull requests

4 participants