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

[improve][ws] Add memory limit configuration for Pulsar client used in Websocket proxy #22666

Merged
merged 1 commit into from
May 8, 2024

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented May 7, 2024

Motivation

There's no explicit flow control in the Websocket proxy for producers. If too many messages are pushed, the proxy (or broker, if ws proxy runs with the broker) will run out of memory.

Modifications

add webSocketPulsarClientMemoryLimitInMB configuration option to limit the memory.
by default, it is unlimited. Can be set to a value such as 64 to keep the direct memory used by the Pulsar client under 64 MB.

Documentation

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

@lhotari lhotari added this to the 3.3.0 milestone May 7, 2024
@lhotari lhotari self-assigned this May 7, 2024
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label May 7, 2024
@heesung-sn
Copy link
Contributor

heesung-sn commented May 7, 2024

nit: can we add comments in the config files and *ServiceConfiguration that it is disabled, by default?

@lhotari lhotari merged commit 80d4675 into apache:master May 8, 2024
50 of 53 checks passed
Technoboy- pushed a commit that referenced this pull request May 8, 2024
lhotari added a commit that referenced this pull request May 14, 2024
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request May 15, 2024
…n Websocket proxy (apache#22666)

(cherry picked from commit 80d4675)
(cherry picked from commit e5515c5)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request May 16, 2024
…n Websocket proxy (apache#22666)

(cherry picked from commit 80d4675)
(cherry picked from commit e5515c5)
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

4 participants