let the worker can rerplicate the fresh ledgers at first.#4642
let the worker can rerplicate the fresh ledgers at first.#4642gaozhangmin wants to merge 5 commits intoapache:masterfrom
Conversation
| Collections.shuffle(children); | ||
| } else { | ||
| Collections.sort(children, Collections.reverseOrder()); | ||
| } |
There was a problem hiding this comment.
This code isn't very readable; I recommend optimizing it.
What does the magic number 3 mean? I recommend including constants to explain their names.
|
CI Error: /home/runner/work/bookkeeper/bookkeeper/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/TestLedgerUnderreplicationManager.java:212:62: ',' is not followed by whitespace. [WhitespaceAfter] |
|
@gaozhangmin @StevenLuMT PTAL |
|
rerun failure checks |
1 similar comment
|
rerun failure checks |
StevenLuMT
requested review from
Copilot,
dlg99,
eolivelli,
hezhangjian,
jiazhai and
zymap
There was a problem hiding this comment.
Pull Request Overview
This PR modifies the ledger replication logic to prioritize newer ledgers by processing larger ledger IDs first. The change aims to improve efficiency by allowing older ledgers to expire before replication if necessary.
- Implements descending order sorting for ledger IDs during replication to prioritize newer ledgers
- Maintains shuffling behavior at depth 3 to avoid lock contention between workers
- Adds comprehensive test coverage to verify the new sorting behavior
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| ZkLedgerUnderreplicationManager.java | Implements conditional sorting logic to process larger ledger IDs first while preserving shuffle behavior at depth 3 |
| TestLedgerUnderreplicationManager.java | Adds test case to verify that larger ledger IDs are processed before smaller ones |
bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerUnderreplicationManager.java
Outdated
Show resolved
Hide resolved
...erver/src/test/java/org/apache/bookkeeper/replication/TestLedgerUnderreplicationManager.java
Outdated
Show resolved
Hide resolved
...erver/src/test/java/org/apache/bookkeeper/replication/TestLedgerUnderreplicationManager.java
Outdated
Show resolved
Hide resolved
…ion/TestLedgerUnderreplicationManager.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…edgerUnderreplicationManager.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
rerun failure checks |
|
rerun failure checks |
To prioritize replication of newly created ledgers, we sort ledger IDs in descending order.
This ensures that recent ledgers (with larger IDs) are replicated first.
As a result, some older ledgers may expire before they are ever replicated, which helps to reduce unnecessary data replication and improve efficiency.