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

remove duplicated configuration #11283

Merged
merged 1 commit into from Jul 14, 2021
Merged

Conversation

aloyszhang
Copy link
Contributor

@aloyszhang aloyszhang commented Jul 12, 2021

Motivation

Remove duplicated configuration when createBkClientConfiguration

Currently, the configuration for BookKeeper client's NumWorkerThreads was duplicated in
https://github.com/apache/pulsar/blob/master/pulsar-broker/src/main/java/org/apache/pulsar/broker/BookKeeperClientFactoryImpl.java?#L115
and
https://github.com/apache/pulsar/blob/master/pulsar-broker/src/main/java/org/apache/pulsar/broker/BookKeeperClientFactoryImpl.java?#L126

So NumWorkerThreads is set to 1 finally which is not expected.

Modifications

remove the duplicated hardcode for setNumWorkerThreads to make NumWorkerThreads configurable

Verifying this change

This change is a code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API: (no)
  • The schema: (no)
  • The default values of configurations: (no)
  • The wire protocol: (no)
  • The rest endpoints: (no)
  • The admin cli options: (no)
  • Anything that affects deployment: (no)

Documentation

  • Does this pull request introduce a new feature? (no)

@aloyszhang
Copy link
Contributor Author

/pulsarbot run-failure-checks

@sijie sijie added this to the 2.9.0 milestone Jul 14, 2021
@sijie sijie added component/bookkeeper area/config release/2.8.1 type/bug The PR fixed a bug or issue reported a bug labels Jul 14, 2021
@BewareMyPower BewareMyPower merged commit 481feea into apache:master Jul 14, 2021
codelipenghui pushed a commit that referenced this pull request Jul 23, 2021
### Motivation
Remove duplicated configuration when `createBkClientConfiguration` 

Currently, the configuration for BookKeeper client's `NumWorkerThreads` was duplicated in 
https://github.com/apache/pulsar/blob/master/pulsar-broker/src/main/java/org/apache/pulsar/broker/BookKeeperClientFactoryImpl.java?#L115
and 
https://github.com/apache/pulsar/blob/master/pulsar-broker/src/main/java/org/apache/pulsar/broker/BookKeeperClientFactoryImpl.java?#L126

So `NumWorkerThreads` is  set to 1 finally which is not expected.
### Modifications

remove the duplicated hardcode for `setNumWorkerThreads` to make `NumWorkerThreads` configurable

(cherry picked from commit 481feea)
@codelipenghui codelipenghui added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Jul 23, 2021
@aloyszhang aloyszhang deleted the remove-dup branch August 2, 2021 02:29
bharanic-dev pushed a commit to bharanic-dev/pulsar that referenced this pull request Mar 18, 2022
### Motivation
Remove duplicated configuration when `createBkClientConfiguration` 

Currently, the configuration for BookKeeper client's `NumWorkerThreads` was duplicated in 
https://github.com/apache/pulsar/blob/master/pulsar-broker/src/main/java/org/apache/pulsar/broker/BookKeeperClientFactoryImpl.java?#L115
and 
https://github.com/apache/pulsar/blob/master/pulsar-broker/src/main/java/org/apache/pulsar/broker/BookKeeperClientFactoryImpl.java?#L126

So `NumWorkerThreads` is  set to 1 finally which is not expected.
### Modifications

remove the duplicated hardcode for `setNumWorkerThreads` to make `NumWorkerThreads` configurable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/config cherry-picked/branch-2.8 Archived: 2.8 is end of life release/2.8.1 type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants