You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by "gaozhangmin (via GitHub)" <gi...@apache.org> on 2025/07/28 06:17:23 UTC
[PR] let the worker can rerplicate the fresh ledgers at first. [bookkeeper]
gaozhangmin opened a new pull request, #4642:
URL: https://github.com/apache/bookkeeper/pull/4642
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.
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] let the worker can rerplicate the fresh ledgers at first. [bookkeeper]
Posted by "gaozhangmin (via GitHub)" <gi...@apache.org>.
gaozhangmin commented on PR #4642:
URL: https://github.com/apache/bookkeeper/pull/4642#issuecomment-3138422192
@gaozhangmin @StevenLuMT PTAL
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] let the worker can rerplicate the fresh ledgers at first. [bookkeeper]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #4642:
URL: https://github.com/apache/bookkeeper/pull/4642#discussion_r2252918456
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerUnderreplicationManager.java:
##########
@@ -567,7 +567,18 @@ private long getLedgerToRereplicateFromHierarchy(String parent, long depth)
return -1;
}
- Collections.shuffle(children);
+ if (depth == 3) {
+ // Avoid workers want to get the same ledger lock.
Review Comment:
The comment has grammatical errors. It should be "Avoid workers wanting to get the same ledger lock" or "Prevent workers from getting the same ledger lock".
```suggestion
// Avoid workers wanting to get the same ledger lock.
```
##########
bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/TestLedgerUnderreplicationManager.java:
##########
@@ -171,6 +173,54 @@ public Long call() {
});
}
+ /**
+ * Test that larger ledgerIds are processed first when getting ledgers to rereplicate.
+ * This verifies the sorting behavior in getLedgerToRereplicateFromHierarchy.
Review Comment:
The word 'Rerepl' should be 'ReReplicate' or 'Replicate' to match the actual method name pattern.
```suggestion
* Test that larger ledgerIds are processed first when getting ledgers to replicate.
* This verifies the sorting behavior in getLedgerToReplicateFromHierarchy.
```
##########
bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/TestLedgerUnderreplicationManager.java:
##########
@@ -171,6 +173,54 @@ public Long call() {
});
}
+ /**
+ * Test that larger ledgerIds are processed first when getting ledgers to rereplicate.
Review Comment:
The word 'rereplicate' should be 're-replicate' or 'replicate' for consistency with standard terminology.
```suggestion
* Test that larger ledgerIds are processed first when getting ledgers to re-replicate.
```
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] let the worker can rerplicate the fresh ledgers at first. [bookkeeper]
Posted by "gaozhangmin (via GitHub)" <gi...@apache.org>.
gaozhangmin commented on PR #4642:
URL: https://github.com/apache/bookkeeper/pull/4642#issuecomment-3173150231
rerun failure checks
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] let the worker can rerplicate the fresh ledgers at first. [bookkeeper]
Posted by "gaozhangmin (via GitHub)" <gi...@apache.org>.
gaozhangmin commented on PR #4642:
URL: https://github.com/apache/bookkeeper/pull/4642#issuecomment-3182830861
rerun failure checks
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] let the worker can rerplicate the fresh ledgers at first. [bookkeeper]
Posted by "gaozhangmin (via GitHub)" <gi...@apache.org>.
gaozhangmin commented on PR #4642:
URL: https://github.com/apache/bookkeeper/pull/4642#issuecomment-3138623356
rerun failure checks
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] let the worker can rerplicate the fresh ledgers at first. [bookkeeper]
Posted by "StevenLuMT (via GitHub)" <gi...@apache.org>.
StevenLuMT commented on PR #4642:
URL: https://github.com/apache/bookkeeper/pull/4642#issuecomment-3152994556
rerun failure checks
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] let the worker can rerplicate the fresh ledgers at first. [bookkeeper]
Posted by "zhannao456 (via GitHub)" <gi...@apache.org>.
zhannao456 commented on PR #4642:
URL: https://github.com/apache/bookkeeper/pull/4642#issuecomment-3138421654
@StevenLuMT PTAL
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] let the worker can rerplicate the fresh ledgers at first. [bookkeeper]
Posted by "StevenLuMT (via GitHub)" <gi...@apache.org>.
StevenLuMT commented on code in PR #4642:
URL: https://github.com/apache/bookkeeper/pull/4642#discussion_r2244150407
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerUnderreplicationManager.java:
##########
@@ -567,7 +567,12 @@ private long getLedgerToRereplicateFromHierarchy(String parent, long depth)
return -1;
}
- Collections.shuffle(children);
+ if (depth == 3) {
+ // Avoid workers want to get the same ledger lock.
+ Collections.shuffle(children);
+ } else {
+ Collections.sort(children, Collections.reverseOrder());
+ }
Review 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.
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] let the worker can rerplicate the fresh ledgers at first. [bookkeeper]
Posted by "StevenLuMT (via GitHub)" <gi...@apache.org>.
StevenLuMT commented on PR #4642:
URL: https://github.com/apache/bookkeeper/pull/4642#issuecomment-3138285520
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]
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org