[improve] Add num not adhering placement ledgers replicated metric for ReplicationWorker #3652
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zymap
requested changes
Nov 18, 2022
| @@ -1262,6 +1268,9 @@ protected EnsemblePlacementPolicy initializeEnsemblePlacementPolicy(ClientConfig | |||
| assertNull(stat1); | |||
| }); | |||
|
|
|||
| assertTrue("NUM_NOT_ADHERING_PLACEMENT_LEDGERS_REPLICATED", | |||
There was a problem hiding this comment.
Please add a new test to make sure the number is increased as expected.
There was a problem hiding this comment.
Good suggestion. I found a bug about numLedgersReplicated update. I have submit another PR #3654 to fix it. PTAL. I will update this PR when that PR is merged.
|
@horizonzy Please help take a look at this PR, thanks. |
Fine |
hangc0276
reviewed
Nov 25, 2022
| @@ -1262,6 +1268,9 @@ protected EnsemblePlacementPolicy initializeEnsemblePlacementPolicy(ClientConfig | |||
| assertNull(stat1); | |||
| }); | |||
|
|
|||
| assertTrue("NUM_NOT_ADHERING_PLACEMENT_LEDGERS_REPLICATED", | |||
hangc0276
pushed a commit
that referenced
this pull request
Feb 15, 2023
### Motivation When I want to add num not adhering placement ledgers replicated metric for RW(#3652), I found the numLedgersReplicated statistic to be inaccurate. When we could not find a target bookie to replicate the ledger, the numLedgersReplicated metric will still increase. 
Codecov Report
@@ Coverage Diff @@
## master #3652 +/- ##
============================================
- Coverage 68.32% 59.81% -8.51%
+ Complexity 6753 5947 -806
============================================
Files 473 473
Lines 40972 40979 +7
Branches 5241 5243 +2
============================================
- Hits 27993 24511 -3482
- Misses 10720 14341 +3621
+ Partials 2259 2127 -132
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
rerun failure checks |
1 similar comment
|
rerun failure checks |
|
Hi, @zymap @hangc0276 I have updated this PR. Could you help take a look? Thanks. |
wenbingshen
changed the title
zymap
approved these changes
Mar 2, 2023
hangc0276
approved these changes
Mar 7, 2023
Ghatage
pushed a commit
to sijie/bookkeeper
that referenced
this pull request
Jul 12, 2024
### Motivation When I want to add num not adhering placement ledgers replicated metric for RW(apache#3652), I found the numLedgersReplicated statistic to be inaccurate. When we could not find a target bookie to replicate the ledger, the numLedgersReplicated metric will still increase. 
Ghatage
pushed a commit
to sijie/bookkeeper
that referenced
this pull request
Jul 12, 2024
…ionWorker (apache#3652) ### Motivation We have `NUM_FULL_OR_PARTIAL_LEDGERS_REPLICATED` in `ReplicationWorker`, which includes both `DATA_LOSS` and `DATA_NOT_ADHERING_PLACEMENT` data repair types. It is meaningful to add `NUM_NOT_ADHERING_PLACEMENT_LEDGERS_REPLICATED` metric for `DATA_NOT_ADHERING_PLACEMENT` type separately.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
We have
NUM_FULL_OR_PARTIAL_LEDGERS_REPLICATEDinReplicationWorker, which includes bothDATA_LOSSandDATA_NOT_ADHERING_PLACEMENTdata repair types. It is meaningful to addNUM_NOT_ADHERING_PLACEMENT_LEDGERS_REPLICATEDmetric forDATA_NOT_ADHERING_PLACEMENTtype separately.