Skip to content

Conversation

@wangjialing218
Copy link
Contributor

Motivation

As #4105, we could config pulsar in container environments through environment variables.
Current, both broker and pulsar tools (admin, client, perf) use PULSAR_MEM to config jvm memory.
Consider we create a pod for broker through kubernetes, we set containers.resources.limits.memory to 25Gi, set env PULSAR_MEM to -Xms10g -Xmx10g -XX:MaxDirectMemorySize=10g, broker will start with this jvm memory config.
When we run pulsar-admin or pulsar-client in this pod, they will start with broker's jvm memory config since PULSAR_MEM was already set, that will cause pod's total memory exceed the limit.

Modifications

Rename PULSAR_MEM to PULSAR_TOOL_MEM for pulsar tools, we could set PULSAR_TOOL_MEM through environment variables for pulsar tools, or just use the default jvm memory setting.

Verifying this change

This change is a trivial rework

Documentation

  • doc-not-needed

Copy link
Member

@dave2wave dave2wave left a comment

Choose a reason for hiding this comment

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

Changes are required

@github-actions
Copy link

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

@github-actions github-actions bot added the Stale label Aug 17, 2022
Copy link
Member

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

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

I'm afraid that this patch causes breaking changes because users can depend on this env var (PULSAR_MEM) to set the memory when running tools.

@tisonkun
Copy link
Member

Close as no consensus. I suggest you start a thread on the dev@ mailing list for such breaking changes.

@tisonkun tisonkun closed this Dec 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants