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/10/11 12:09:10 UTC

[GitHub] [pulsar] lhotari opened a new pull request, #18003: [improve][docs] Improve docs for Bookkeeper E, Qw and Qa.

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

   ### Motivation
   
   Important details are missing for documenting ensemble (E) size, write quorum (Qw) size and ack quorum (Qa) size.
   For example, it is not well known that there's a rule that E == Qw to use sticky reads or that the max number of bookie failures is Qa-1. Some might realize this only in production when they lose a bookie and cannot recover.
   
   ### Modifications
   
   - improve documentation for persistence policies parameters
   
   ### Documentation
   
   <!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
   
   - [x] `doc` <!-- Your PR contains doc changes -->
   - [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
   - [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
   - [ ] `doc-complete` <!-- Docs have been already added -->
   
   ### Matching PR in forked repository
   
   PR in forked repository: <!-- ENTER URL HERE -->
   
   <!--
   After opening this PR, the build in apache/pulsar will fail and instructions will
   be provided for opening a PR in the PR author's forked repository.
   
   apache/pulsar pull requests should be first tested in your own fork since the 
   apache/pulsar CI based on GitHub Actions has constrained resources and quota.
   GitHub Actions provides separate quota for pull requests that are executed in 
   a forked repository.
   
   The tests will be run in the forked repository until all PR review comments have
   been handled, the tests pass and the PR is approved by a reviewer.
   -->
   


-- 
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 #18003: [improve][docs] Improve docs for persistence policies parameters E, Qw and Qa.

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


##########
site2/docs/administration-zk-bk.md:
##########
@@ -272,35 +272,54 @@ You can set persistence policies for BookKeeper at the [namespace](reference-ter
 
 Use the [`set-persistence`](/tools/pulsar-admin/) subcommand and specify a namespace as well as any policies that you want to apply. The available flags are:
 
-Flag | Description | Default
-:----|:------------|:-------
-`-a`, `--bookkeeper-ack-quorum` | The number of acks (guaranteed copies) to wait on for each entry | 0
-`-e`, `--bookkeeper-ensemble` | The number of [bookies](reference-terminology.md#bookie) to use for topics in the namespace | 0
-`-w`, `--bookkeeper-write-quorum` | The number of writes to make for each entry | 0
-`-r`, `--ml-mark-delete-max-rate` | Throttling rate for mark-delete operations (0 means no throttle) | 0
+Flag | Description                                                                                                                | Default
+:----|:---------------------------------------------------------------------------------------------------------------------------|:-------
+`-e`, `--bookkeeper-ensemble` | Ensemble (E) size, Number of [bookies](reference-terminology.md#bookie) to use for storing entries in a ledger.| 0
+`-w`, `--bookkeeper-write-quorum` | Write quorum (Q<sub>w</sub>) size, Replication factor for storing entries (messages) in a ledger.                                            | 0
+`-a`, `--bookkeeper-ack-quorum` | Ack quorum (Q<sub>a</sub>) size, Number of guaranteed copies (acks to wait for before a write is considered completed)                          | 0
+`-r`, `--ml-mark-delete-max-rate` | Throttling rate for mark-delete operations (0 means no throttle)                                                           | 0
+
+Please notice that sticky reads enabled by `bookkeeperEnableStickyReads=true` aren’t used unless ensemble size (E) equals write quorum (Q<sub>w</sub>) size. Sticky reads improve the efficiency of the Bookkeeper read ahead cache when all reads for a single ledger are sent to a single bookie.
+
+Some rules for choosing the values:
+
+Rule | Description |
+:----|:------------|
+E >= Q<sub>w</sub> >= Q<sub>a</sub>|Ensemble size must be larger or equal than write quorum size, write quorum size must be larger or equal than ack quorum size.
+Max bookie failures = Q<sub>a</sub>-1, |This rule must be fulfilled if data durability is desired in case of bookie failures. To safely tolerate at least one bookie failure at a time in the ensemble, Q<sub>a</sub> must be set to a value at least 2.
+E == Q<sub>w</sub> | Sticky reads enabled by `bookkeeperEnableStickyReads=true` aren't used unless ensemble size (E) equals write quorum (Q<sub>w</sub>) size.
 
 The following is an example:
 
 ```shell
 pulsar-admin namespaces set-persistence my-tenant/my-ns \
---bookkeeper-ack-quorum 2 \
---bookkeeper-ensemble 3
+--bookkeeper-ensemble 3 \
+--bookkeeper-write-quorum 3 \
+--bookkeeper-ack-quorum 3
+```
+
+short example:

Review Comment:
   ```suggestion
   Short example:
   ```



-- 
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] lhotari merged pull request #18003: [improve][docs] Improve docs for persistence policies parameters E, Qw and Qa.

Posted by GitBox <gi...@apache.org>.
lhotari merged PR #18003:
URL: https://github.com/apache/pulsar/pull/18003


-- 
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] codecov-commenter commented on pull request #18003: [improve][docs] Improve docs for persistence policies parameters E, Qw and Qa.

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #18003:
URL: https://github.com/apache/pulsar/pull/18003#issuecomment-1276589812

   # [Codecov](https://codecov.io/gh/apache/pulsar/pull/18003?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > :exclamation: No coverage uploaded for pull request base (`master@92b4708`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#section-missing-base-commit).
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pulsar/pull/18003/graphs/tree.svg?width=650&height=150&src=pr&token=acYqCpsK9J&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/pulsar/pull/18003?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff            @@
   ##             master   #18003   +/-   ##
   =========================================
     Coverage          ?   70.83%           
     Complexity        ?      437           
   =========================================
     Files             ?       26           
     Lines             ?     2246           
     Branches          ?      245           
   =========================================
     Hits              ?     1591           
     Misses            ?      482           
     Partials          ?      173           
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | unittests | `70.83% <0.00%> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   


-- 
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 a diff in pull request #18003: [improve][docs] Improve docs for persistence policies parameters E, Qw and Qa.

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


##########
conf/broker.conf:
##########
@@ -1014,6 +1014,8 @@ bookkeeperClientMinAvailableBookiesInIsolationGroups=
 # Enable/disable having read operations for a ledger to be sticky to a single bookie.
 # If this flag is enabled, the client will use one single bookie (by preference) to read
 # all entries for a ledger.
+# Please notice that sticky reads aren’t used unless ensemble size equals write quorum size for the ledger's
+# persistence policies.

Review Comment:
   Nit: the double negative might be confusing. Here is my attempt at an alternative wording, but mine doesn't feel completely right either.
   
   ```suggestion
   # Please notice that enabling sticky reads also requires that the ensemble size equals write quorum size for the ledger's
   # persistence policies.
   ```



-- 
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