From f58bfce8b7d0550e2f4cc934c1274dc046c8e5e2 Mon Sep 17 00:00:00 2001 From: ninjazhou <843520313@qq.com> Date: Tue, 17 Mar 2026 12:09:36 +0800 Subject: [PATCH] [FIX] fix SyncLedgerIterator.hasNext() failing to iterate across ZK ledger ranges --- .../apache/bookkeeper/client/BookKeeper.java | 17 +- .../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 0c7dbf78367..9c70f582983 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 SyncLedgerIterator(LedgerRangeIterator iterator, ListLedgersResultImpl pa 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 d7c4c9ba8f3..d4bef135284 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 void testListLedgers() } } + /** + * Unlike {@link #testListLedgers()} which only uses 10 ledgers within a single ZK range, + * this test creates 10010 ledgers to verify cross-range iteration. + * + *

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: + *

+ * + *

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