You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/05/24 21:36:11 UTC

[GitHub] [pulsar] michaeljmarshall opened a new pull request, #15763: Support dlog bookie client configuration via pass through config

michaeljmarshall opened a new pull request, #15763:
URL: https://github.com/apache/pulsar/pull/15763

   ### Motivation
   
   Enable pass through configuration support for the bookkeeper clients created by `DistributedLog` and `BookKeeperPackagesStorage`. This support is already present for the managed ledger bookkeeper client, which takes all `bookkeeper_` prefixed configs. In order to simplify configuration, I reused the `bookkeeper_` prefix since it's reasonable to assume that these configs will be the same.
   
   ### Modifications
   
   * Create `PulsarConfigurationUtils` class to reduce code duplication for mapping prefixed config classes into other config classes. This class is not available in the `pulsar-package-management` module, so the code is copied. I don't think we should expose this code in the client jars.
   * Update the `BookKeeperPackagesStorageConfiguration` to expose the underlying `properties` object.
   * Add documentation to the `broker.conf` and the `functions_worker.yml` files.
   
   ### Verifying this change
   
   I added tests to ensure that the new methods work as expected.
   
   ### Does this pull request potentially affect one of the following parts:
   
   This is a backwards compatible change. The only nuance is that the package management dlog bookie client will inherit the same configs that the broker already uses for the bookie client. I think this is preferable.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] michaeljmarshall commented on pull request #15763: Support dlog bookie client configuration via pass through config

Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on PR #15763:
URL: https://github.com/apache/pulsar/pull/15763#issuecomment-1136735304

   @Anonymitaet - thank you for your feedback.
   
   Making this a draft temporarily. I will include some additional work tomorrow.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] michaeljmarshall commented on pull request #15763: Support dlog bookie client configuration via pass through config

Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on PR #15763:
URL: https://github.com/apache/pulsar/pull/15763#issuecomment-1139773457

   This PR is superseded by https://github.com/apache/pulsar/pull/15818.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] Anonymitaet commented on a diff in pull request #15763: Support dlog bookie client configuration via pass through config

Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on code in PR #15763:
URL: https://github.com/apache/pulsar/pull/15763#discussion_r881141844


##########
conf/broker.conf:
##########
@@ -1429,6 +1432,10 @@ packagesReplicas=1
 # The bookkeeper ledger root path
 packagesManagementLedgerRootPath=/ledgers
 
+# When using BookKeeperPackagesStorageProvider, you can configure the
+# bookkeeper client by prefixing configurations with "bookkeeper_".
+# This config will apply to managed ledger bookkeeper clients, as well.

Review Comment:
   ```suggestion
   # This configuration applies to managed ledger bookkeeper clients as well.
   ```



##########
conf/broker.conf:
##########
@@ -950,8 +950,11 @@ managedLedgerDefaultAckQuorum=2
 # in case of lack of enough bookies
 #bookkeeper_opportunisticStriping=false
 
-# you can add other configuration options for the BookKeeper client
-# by prefixing them with bookkeeper_
+# You can add other configuration options for the BookKeeper client
+# by prefixing them with "bookkeeper_". These configurations will be applied

Review Comment:
   ```suggestion
   # by prefixing them with "bookkeeper_". These configurations are applied
   ```
   Write in the simple present tense as much as possible if you are covering facts that were, are, and forever shall be true. https://docs.google.com/document/d/1lc5j4RtuLIzlEYCBo97AC8-U_3Erzs_lxpkDuseU0n4/edit#bookmark=id.e8uqh1awkcnp



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] michaeljmarshall closed pull request #15763: Support dlog bookie client configuration via pass through config

Posted by GitBox <gi...@apache.org>.
michaeljmarshall closed pull request #15763: Support dlog bookie client configuration via pass through config
URL: https://github.com/apache/pulsar/pull/15763


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org