This is an automated email from the ASF dual-hosted git repository.
shenwenbing pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
The following commit(s) were added to refs/heads/master by this push:
new 8a8f4d607d [FIX] fix SyncLedgerIterator.hasNext() failing to iterate
across ZK ledger ranges (#4731)
8a8f4d607d is described below
commit 8a8f4d607dc7056a75178316dc563445165ad460
Author: zhou zhuohan <[email protected]>
AuthorDate: Tue Mar 24 11:59:32 2026 +0800
[FIX] fix SyncLedgerIterator.hasNext() failing to iterate across ZK ledger
ranges (#4731)
---
.../org/apache/bookkeeper/client/BookKeeper.java | 17 ++-
.../bookkeeper/client/api/LedgerMetadataTest.java | 146 +++++++++++++++++++++
2 files changed, 156 insertions(+), 7 deletions(-)
diff --git
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java
index 0c7dbf7836..9c70f58298 100644
---
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java
+++
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java
@@ -1583,17 +1583,20 @@ public class BookKeeper implements
org.apache.bookkeeper.client.api.BookKeeper {
this.parent = parent;
}
- @Override
+ /**
+ * ZooKeeper stores ledgers in a hierarchical tree (e.g.
/ledgers/00/0000/...).
+ * {@code iterator} (LedgerRangeIterator) traverses the intermediate
tree nodes (ranges/buckets),
+ * while {@code currentRange} iterates over the leaf-level ledger IDs
within a single range.
+ *
+ * Therefore, when {@code currentRange} has no more leaf nodes, we
need to check
+ * {@code iterator.hasNext()} to determine if there are more ranges to
advance to.
+ */
public boolean hasNext() throws IOException {
parent.checkClosed();
- if (currentRange != null) {
- if (currentRange.hasNext()) {
- return true;
- }
- } else if (iterator.hasNext()) {
+ if (currentRange != null && currentRange.hasNext()) {
return true;
}
- return false;
+ return iterator.hasNext();
}
@Override
diff --git
a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/api/LedgerMetadataTest.java
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/api/LedgerMetadataTest.java
index d7c4c9ba8f..d4bef13528 100644
---
a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/api/LedgerMetadataTest.java
+++
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/api/LedgerMetadataTest.java
@@ -190,4 +190,150 @@ public class LedgerMetadataTest extends
BookKeeperClusterTestCase {
}
}
+ /**
+ * Unlike {@link #testListLedgers()} which only uses 10 ledgers within a
single ZK range,
+ * this test creates 10010 ledgers to verify cross-range iteration.
+ *
+ * <p>The HierarchicalLedgerManager stores ledgers in a ZK tree where each
range (znode)
+ * holds at most 10,000 leaf nodes. This limit is determined by the
4-digit last level of
+ * the ledger ID path (0000-9999), as defined in:
+ * <ul>
+ * <li>{@code LegacyHierarchicalLedgerManager} — 2-4-4 split,
+ * path: {root}/{level1}/{level2}/L{level3}</li>
+ * <li>{@code LongHierarchicalLedgerManager} — 3-4-4-4-4 split,
+ * path: {root}/{level0}/{level1}/{level2}/{level3}/L{level4}</li>
+ * </ul>
+ *
+ * <p>By using numOfLedgers = 10010 (> 10000), ledgers will span across
multiple ranges,
+ * exercising the SyncLedgerIterator's ability to advance from one
exhausted range to the
+ * next via LedgerRangeIterator.
+ */
+ @Test
+ public void testListLedgersLargeScale()
+ throws Exception {
+ // 10010 > 10000 (max ledgers per ZK range, determined by 4-digit last
level 0000-9999),
+ // ensuring cross-range iteration is covered
+ int numOfLedgers = 10010;
+
+ ClientConfiguration conf = new ClientConfiguration();
+ conf.setMetadataServiceUri(zkUtil.getMetadataServiceUri());
+
+ try (BookKeeper bkc = BookKeeper.newBuilder(conf).build();) {
+ long[] ledgerIds = new long[numOfLedgers];
+ for (int i = 0; i < numOfLedgers; i++) {
+
+ try (WriteHandle l = bkc
+ .newCreateLedgerOp()
+ .withDigestType(DigestType.CRC32)
+ .withPassword("testPasswd".getBytes())
+ .execute()
+ .get();) {
+ ledgerIds[i] = l.getId();
+ }
+ }
+
+ try (ListLedgersResult result =
FutureUtils.result(bkc.newListLedgersOp().execute());) {
+ int count = 0;
+
+ for (long ledgerId : result.toIterable()) {
+ assertEquals(ledgerIds[count++], ledgerId);
+ }
+
+ assertEquals(numOfLedgers, count, "Unexpected ledgers count");
+ try {
+ result.iterator();
+ fail("Should thrown error");
+ } catch (IllegalStateException e) {
+ // ok
+ }
+ try {
+ result.toIterable();
+ fail("Should thrown error");
+ } catch (IllegalStateException e) {
+ // ok
+ }
+ }
+
+ try (ListLedgersResult result =
FutureUtils.result(bkc.newListLedgersOp().execute());) {
+ int count = 0;
+
+ for (LedgersIterator iterator = result.iterator();
iterator.hasNext();) {
+ long ledgerId = iterator.next();
+ assertEquals(ledgerIds[count++], ledgerId);
+
+ }
+ assertEquals(numOfLedgers, count, "Unexpected ledgers count");
+ try {
+ result.iterator();
+ fail("Should thrown error");
+ } catch (IllegalStateException e) {
+ // ok
+ }
+ try {
+ result.toIterable();
+ fail("Should thrown error");
+ } catch (IllegalStateException e) {
+ // ok
+ }
+ }
+ }
+
+ // check closed
+ {
+ ListLedgersResult result =
FutureUtils.result(bkc.newListLedgersOp().execute());
+ result.close();
+ try {
+ result.toIterable();
+ fail("Should thrown error");
+ } catch (IllegalStateException e) {
+ // ok
+ }
+
+ try {
+ result.iterator();
+ fail("Should thrown error");
+ } catch (IllegalStateException e) {
+ // ok
+ }
+ }
+
+ { // iterator
+ ListLedgersResult result =
FutureUtils.result(bkc.newListLedgersOp().execute());
+ LedgersIterator it = result.iterator();
+ result.close();
+ try {
+ it.hasNext();
+ fail("Should thrown error");
+ } catch (IllegalStateException e) {
+ // ok
+ }
+
+ try {
+ it.next();
+ fail("Should thrown error");
+ } catch (IllegalStateException e) {
+ // ok
+ }
+ }
+
+ { // iterable
+ ListLedgersResult result =
FutureUtils.result(bkc.newListLedgersOp().execute());
+ Iterator<Long> it = result.toIterable().iterator();
+ result.close();
+ try {
+ it.hasNext();
+ fail("Should thrown error");
+ } catch (IllegalStateException e) {
+ // ok
+ }
+
+ try {
+ it.next();
+ fail("Should thrown error");
+ } catch (IllegalStateException e) {
+ // ok
+ }
+ }
+ }
+
}
