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

[Config] Add readWorkerThreadsThrottlingEnabled to conf/bookkeeper.conf #12666

Merged

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented Nov 8, 2021

Motivation

Modifications

  • Add readWorkerThreadsThrottlingEnabled=true to conf/bookkeeper.conf since that is the default value in BookKeeper that gets used unless the setting is explicitly set to false.

Open questions

- apache/bookkeeper#2646 added "Auto-throttle read operations" which is
  enabled by default
@lhotari lhotari added component/bookkeeper doc-not-needed Your PR changes do not impact docs labels Nov 8, 2021
@lhotari lhotari added this to the 2.10.0 milestone Nov 8, 2021
@lhotari lhotari self-assigned this Nov 8, 2021
@lhotari
Copy link
Member Author

lhotari commented Nov 8, 2021

@dlg99 Please review

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

LGTM

good idea

@eolivelli
Copy link
Contributor

@merlimat can you please comment on the 'open question' ?

This is the line for whom is interested in checking out the source code in BK
https://github.com/apache/bookkeeper/pull/2646/files#diff-329b56111e8bed86869212b7f1b7027a2ee791cd8476465d127d345128cc9b84R86

@merlimat
Copy link
Contributor

This is the line for whom is interested in checking out the source code in BK

The await() is safe since we're (in practice) always doing the read operations from a BK pool of threads dedicated to read, and not from Netty IO threads.

The purpose is indeed to apply back-pressure and avoid unlimited memory usage in the Bookie.

@merlimat merlimat merged commit fc6d6da into apache:master Nov 10, 2021
codelipenghui pushed a commit that referenced this pull request Nov 18, 2021
…nf (#12666)

- apache/bookkeeper#2646 added "Auto-throttle read operations" which is
  enabled by default

(cherry picked from commit fc6d6da)
@codelipenghui codelipenghui added cherry-picked/branch-2.8 Archived: 2.8 is end of life release/2.8.2 and removed release/2.8.3 labels Nov 18, 2021
eolivelli pushed a commit to eolivelli/pulsar that referenced this pull request Nov 29, 2021
…nf (apache#12666)

- apache/bookkeeper#2646 added "Auto-throttle read operations" which is
  enabled by default
codelipenghui pushed a commit that referenced this pull request Dec 21, 2021
…nf (#12666)

- apache/bookkeeper#2646 added "Auto-throttle read operations" which is
  enabled by default

(cherry picked from commit fc6d6da)
@codelipenghui codelipenghui added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Dec 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/broker cherry-picked/branch-2.8 Archived: 2.8 is end of life cherry-picked/branch-2.9 Archived: 2.9 is end of life doc-not-needed Your PR changes do not impact docs release/2.8.2 release/2.9.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants