diff --git a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/PositionFactory.java b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/PositionFactory.java index 481619b8db7a6..1a12d70a6ecc3 100644 --- a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/PositionFactory.java +++ b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/PositionFactory.java @@ -48,6 +48,7 @@ public static Position create(long ledgerId, long entryId) { return new ImmutablePositionImpl(ledgerId, entryId); } + /** * Create a new position or returns the other instance if it's immutable. * diff --git a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java index 1e5c57bb453ec..75c0c2c6380eb 100644 --- a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java +++ b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java @@ -120,7 +120,7 @@ @SuppressWarnings("checkstyle:javadoctype") public class ManagedCursorImpl implements ManagedCursor { - private static final Comparator ENTRY_COMPARATOR = (e1, e2) -> { + static final Comparator ENTRY_COMPARATOR = (e1, e2) -> { if (e1.getLedgerId() != e2.getLedgerId()) { return e1.getLedgerId() < e2.getLedgerId() ? -1 : 1; } @@ -3923,6 +3923,9 @@ public long[] getBatchPositionAckSet(Position position) { public Position getNextAvailablePosition(Position position) { lock.readLock().lock(); try { + if (individualDeletedMessages.isEmpty()) { + return ledger.getNextValidPosition(position); + } Range range = individualDeletedMessages.rangeContaining(position.getLedgerId(), position.getEntryId()); if (range != null) { @@ -3930,7 +3933,7 @@ public Position getNextAvailablePosition(Position position) { return (nextPosition != null && nextPosition.compareTo(position) > 0) ? nextPosition : position.getNext(); } - return position.getNext(); + return ledger.getNextValidPosition(position); } finally { lock.readLock().unlock(); } diff --git a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java index f6c3de05b59ea..12a0ab7f17463 100644 --- a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java +++ b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java @@ -2385,43 +2385,168 @@ private void internalReadFromLedger(ReadHandle ledger, OpReadEntry opReadEntry) long lastEntry = min(firstEntry + opReadEntry.getNumberOfEntriesToRead() - 1, lastEntryInLedger); - // Filer out and skip unnecessary read entry - if (opReadEntry.skipCondition != null) { - long firstValidEntry = -1L; - long lastValidEntry = -1L; - long entryId = firstEntry; - for (; entryId <= lastEntry; entryId++) { - if (opReadEntry.skipCondition.test(PositionFactory.create(ledger.getId(), entryId))) { - if (firstValidEntry != -1L) { - break; - } - } else { - if (firstValidEntry == -1L) { - firstValidEntry = entryId; - } + Predicate skipCondition = opReadEntry.skipCondition; + if (skipCondition == null) { + if (log.isDebugEnabled()) { + log.debug("[{}] Reading entries from ledger {} - first={} last={}", name, ledger.getId(), firstEntry, + lastEntry); + } + asyncReadEntry(ledger, firstEntry, lastEntry, opReadEntry, opReadEntry.ctx); + return; + } - lastValidEntry = entryId; + // Scan entries and build contiguous read ranges, skipping entries that match the predicate + List entryIds = new ArrayList<>((int) (lastEntry - firstEntry + 1)); + List> ranges = buildRanges(firstEntry, lastEntry, ledger.getId(), + skipCondition, entryIds); + + Position lastReadPosition = PositionFactory.create(ledger.getId(), lastEntry); + if (entryIds.isEmpty()) { + final var nextReadPosition = lastReadPosition.getNext(); + opReadEntry.updateReadPosition(nextReadPosition); + opReadEntry.checkReadCompletion(); + return; + } + + ReadEntriesCallback callback = new BatchReadEntriesCallback(entryIds, opReadEntry, lastReadPosition); + for (Pair pair : ranges) { + asyncReadEntry(ledger, pair.getLeft(), pair.getRight(), + opReadEntry.cursor, callback, opReadEntry.ctx); + } + } + + /** + * Scan entry range [firstEntry, lastEntry], skipping entries where skipCondition returns true, + * and build contiguous read ranges directly. + * + * @param firstEntry first entry id to scan (inclusive) + * @param lastEntry last entry id to scan (inclusive) + * @param ledgerId the ledger id for building positions + * @param skipCondition predicate to determine which entries to skip + * @param entryIds output list that collects all non-skipped entry ids in order + * @return list of contiguous [start, end] pairs + */ + @VisibleForTesting + public static List> buildRanges(long firstEntry, long lastEntry, long ledgerId, + Predicate skipCondition, + List entryIds) { + MutablePositionImpl position = new MutablePositionImpl(); + List> ranges = new ArrayList<>(); + long rangeStart = -1; + long rangeEnd = -1; + for (long entryId = firstEntry; entryId <= lastEntry; entryId++) { + position.changePositionTo(ledgerId, entryId); + if (skipCondition.test(position)) { + if (rangeStart != -1) { + ranges.add(ImmutablePair.of(rangeStart, rangeEnd)); + rangeStart = -1; } + continue; + } + if (rangeStart == -1) { + rangeStart = entryId; } + rangeEnd = entryId; + entryIds.add(entryId); + } + if (rangeStart != -1) { + ranges.add(ImmutablePair.of(rangeStart, rangeEnd)); + } + return ranges; + } - // If all messages in [firstEntry...lastEntry] are filter out, - // then manual call internalReadEntriesComplete to advance read position. - if (firstValidEntry == -1L) { - final var nextReadPosition = PositionFactory.create(ledger.getId(), lastEntry).getNext(); - opReadEntry.updateReadPosition(nextReadPosition); - opReadEntry.checkReadCompletion(); + @VisibleForTesting + public static class BatchReadEntriesCallback implements ReadEntriesCallback { + private final List entryIds; + private final List entries; + private final OpReadEntry callback; + private volatile boolean completed = false; + private final Position lastReadPosition; + + @VisibleForTesting + public BatchReadEntriesCallback(List entryIds, OpReadEntry callback, Position lastReadPosition) { + this.entryIds = entryIds; + this.entries = new ArrayList<>(entryIds.size()); + this.callback = callback; + this.lastReadPosition = lastReadPosition; + } + + @Override + public synchronized void readEntriesComplete(List entries0, Object ctx) { + if (completed) { + for (Entry entry : entries0) { + entry.release(); + } return; } - - firstEntry = firstValidEntry; - lastEntry = lastValidEntry; + entries.addAll(entries0); + // If read empty batch from Bookie, we have to complete the call. + // Otherwise, it maybe blocks forever, see: PR 24515. + if (entries.size() < entryIds.size() && !entries0.isEmpty()) { + return; + } + completed = true; + // Sort to ensure correct order since async range reads may complete out of submission order. + entries.sort(ManagedCursorImpl.ENTRY_COMPARATOR); + // If we want to read [1, 2, 3, 4, 5], but we only read [1, 2, 3], [4,5] are filtered, so we need to pass + // the `lastReadPosition([5])` to make sure the cursor read position is correct. + // If we pass nonnull `lastReadPosition` to call if the entries0.isEmpty, it will skip some entries. + callback.internalReadEntriesComplete(entries, entries0.isEmpty() ? null : lastReadPosition); } - if (log.isDebugEnabled()) { - log.debug("[{}] Reading entries from ledger {} - first={} last={}", name, ledger.getId(), firstEntry, - lastEntry); + @Override + public synchronized void readEntriesFailed(ManagedLedgerException exception, Object ctx) { + if (completed) { + return; + } + completed = true; + // If there are entries been read success, try to let the read operation success as possible. + List entries = filterEntries(); + if (!entries.isEmpty()) { + // Move the read position of the cursor to the next position of the last read entry, + // or we will deliver the same entry to the consumer more than once. + Entry entry = entries.get(entries.size() - 1); + Position position = PositionFactory.create(entry.getLedgerId(), entry.getEntryId()); + Position nextReadPosition = callback.cursor.getNextAvailablePosition(position); + callback.updateReadPosition(nextReadPosition); + } + callback.internalReadEntriesFailed(entries, exception, ctx); + } + + /** + * Filter the entries that have been read success. + *

+ * If we want to read [1, 2, 3, 4, 5], but only read [1, 2, 4, 5] successfully, [3] is read failed, + * only return [1,2] to the caller, to make sure the read operation success as possible + * and keep the ordering guarantee. + * + * @return filtered entries + */ + private List filterEntries() { + if (entries.isEmpty()) { + return Collections.emptyList(); + } + // Sort first since async range reads may complete out of submission order. + entries.sort(ManagedCursorImpl.ENTRY_COMPARATOR); + List entries0 = new ArrayList<>(); + int entryIdx = 0; + for (int i = 0; i < entryIds.size() && entryIdx < entries.size(); i++) { + Entry entry = entries.get(entryIdx); + if (entry.getEntryId() == entryIds.get(i)) { + entries0.add(entry); + entryIdx++; + } else { + entry.release(); + entryIdx++; + break; + } + } + // Release the entries that are not in the result. + for (int i = entryIdx; i < entries.size(); i++) { + entries.get(i).release(); + } + return entries0; } - asyncReadEntry(ledger, firstEntry, lastEntry, opReadEntry, opReadEntry.ctx); } protected void asyncReadEntry(ReadHandle ledger, Position position, ReadEntryCallback callback, Object ctx) { @@ -2455,6 +2580,22 @@ protected void asyncReadEntry(ReadHandle ledger, long firstEntry, long lastEntry } } + protected void asyncReadEntry(ReadHandle ledger, long firstEntry, long lastEntry, ManagedCursorImpl cursor, + ReadEntriesCallback callback, Object ctx) { + IntSupplier expectedReadCount = cursor::getNumberOfCursorsAtSamePositionOrBefore; + if (config.getReadEntryTimeoutSeconds() > 0) { + // set readOpCount to uniquely validate if ReadEntryCallbackWrapper is already recycled + long readOpCount = READ_OP_COUNT_UPDATER.incrementAndGet(this); + long createdTime = System.nanoTime(); + ReadEntryCallbackWrapper readCallback = ReadEntryCallbackWrapper.create(name, ledger.getId(), firstEntry, + callback, readOpCount, createdTime, ctx); + lastReadCallback = readCallback; + entryCache.asyncReadEntry(ledger, firstEntry, lastEntry, expectedReadCount, readCallback, readOpCount); + } else { + entryCache.asyncReadEntry(ledger, firstEntry, lastEntry, expectedReadCount, callback, ctx); + } + } + static final class ReadEntryCallbackWrapper implements ReadEntryCallback, ReadEntriesCallback { volatile ReadEntryCallback readEntryCallback; diff --git a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/MutablePositionImpl.java b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/MutablePositionImpl.java new file mode 100644 index 0000000000000..897c36e416059 --- /dev/null +++ b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/MutablePositionImpl.java @@ -0,0 +1,79 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.bookkeeper.mledger.impl; + +import org.apache.bookkeeper.mledger.Position; + +final class MutablePositionImpl implements Position { + + // Non-volatile: this class is designed for single-threaded reuse within a method scope + // and should not be shared across threads or stored as a long-lived reference. + private long ledgerId; + private long entryId; + + MutablePositionImpl(long ledgerId, long entryId) { + this.ledgerId = ledgerId; + this.entryId = entryId; + } + + MutablePositionImpl() { + this.ledgerId = -1; + this.entryId = -1; + } + + /** + * Change the ledgerId and entryId. + * + * @param ledgerId + * @param entryId + */ + public void changePositionTo(long ledgerId, long entryId) { + this.ledgerId = ledgerId; + this.entryId = entryId; + } + + @Override + public long getLedgerId() { + return ledgerId; + } + + @Override + public long getEntryId() { + return entryId; + } + + /** + * String representation of virtual cursor - LedgerId:EntryId. + */ + @Override + public String toString() { + return ledgerId + ":" + entryId; + } + + @Override + public int hashCode() { + return hashCodeForPosition(); + } + + @Override + public boolean equals(Object obj) { + return obj instanceof Position && compareTo((Position) obj) == 0; + } + +} diff --git a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpReadEntry.java b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpReadEntry.java index b618a25aa3d75..26df4895e4314 100644 --- a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpReadEntry.java +++ b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpReadEntry.java @@ -21,6 +21,7 @@ import io.netty.util.Recycler; import io.netty.util.Recycler.Handle; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Predicate; @@ -31,6 +32,7 @@ import org.apache.bookkeeper.mledger.ManagedLedgerException.TooManyRequestsException; import org.apache.bookkeeper.mledger.Position; import org.apache.bookkeeper.mledger.PositionFactory; +import org.apache.commons.collections4.CollectionUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -79,8 +81,8 @@ public static OpReadEntry create(ManagedCursorImpl cursor, Position readPosition return op; } - private void internalReadEntriesComplete(List returnedEntries) { - if (returnedEntries.isEmpty()) { + void internalReadEntriesComplete(List returnedEntries, Position lastReadPosition) { + if (returnedEntries.isEmpty() && lastReadPosition == null) { log.warn("[{}] Read no entries unexpectedly", this); checkReadCompletion(); return; @@ -99,13 +101,13 @@ private void internalReadEntriesComplete(List returnedEntries) { } // Entries might be released after `filterReadEntries`, so retrieve the last position before that - final var lastPosition = returnedEntries.get(entriesCount - 1).getPosition(); + final var lastPosition = lastReadPosition != null ? lastReadPosition + : returnedEntries.get(entriesCount - 1).getPosition(); final var filteredEntries = cursor.filterReadEntries(returnedEntries); entries.addAll(filteredEntries); // if entries have been filtered out then try to skip reading of already deletedMessages in that range - final Position nexReadPosition = entriesCount != filteredEntries.size() - ? cursor.getNextAvailablePosition(lastPosition) : lastPosition.getNext(); + final Position nexReadPosition = cursor.getNextAvailablePosition(lastPosition); updateReadPosition(nexReadPosition); checkReadCompletion(); } @@ -113,7 +115,7 @@ private void internalReadEntriesComplete(List returnedEntries) { @Override public void readEntriesComplete(List returnedEntries, Object ctx) { try { - internalReadEntriesComplete(returnedEntries); + internalReadEntriesComplete(returnedEntries, null); } catch (Throwable throwable) { log.error("[{}] Fallback to readEntriesFailed for exception in readEntriesComplete", this, throwable); readEntriesFailed(ManagedLedgerException.getManagedLedgerException(throwable), ctx); @@ -123,16 +125,18 @@ public void readEntriesComplete(List returnedEntries, Object ctx) { @Override public void readEntriesFailed(ManagedLedgerException exception, Object ctx) { try { - internalReadEntriesFailed(exception, ctx); + internalReadEntriesFailed(null, exception, ctx); } catch (Throwable throwable) { // At least we should complete the callback fail(ManagedLedgerException.getManagedLedgerException(throwable), ctx); } } - private void internalReadEntriesFailed(ManagedLedgerException exception, Object ctx) { + void internalReadEntriesFailed(Collection ret, ManagedLedgerException exception, Object ctx) { cursor.readOperationCompleted(); - + if (CollectionUtils.isNotEmpty(ret)) { + entries.addAll(ret); + } if (!entries.isEmpty()) { // There were already some entries that were read before, we can return them complete(ctx); diff --git a/managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedCursorTest.java b/managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedCursorTest.java index 80f6bea9e36a4..f1bcc5dbe602e 100644 --- a/managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedCursorTest.java +++ b/managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedCursorTest.java @@ -5014,7 +5014,7 @@ public void testReadEntriesWithSkipDeletedEntries() throws Exception { }) .when(ledger) .asyncReadEntry(Mockito.any(ReadHandle.class), Mockito.anyLong(), - Mockito.anyLong(), Mockito.any(), Mockito.any()); + Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any()); @Cleanup ManagedCursor cursor = ledger.openCursor("c"); @@ -5120,7 +5120,7 @@ public void testReadEntriesWithSkipDeletedEntriesAndWithSkipConditions() throws }) .when(ledger) .asyncReadEntry(Mockito.any(ReadHandle.class), Mockito.anyLong(), - Mockito.anyLong(), Mockito.any(), Mockito.any()); + Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any()); @Cleanup ManagedCursor cursor = ledger.openCursor("c"); diff --git a/managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerTest.java b/managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerTest.java index 7c92d1b4c5976..80ddca0fa193b 100644 --- a/managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerTest.java +++ b/managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerTest.java @@ -5344,4 +5344,350 @@ public void addFailed(ManagedLedgerException exception, Object ctx) { // Verify properties are preserved after cursor reset assertEquals(cursor.getProperties(), expectedProperties); } + + + @Test + public void testBatchReadEntriesCallback() throws Exception { + @Cleanup + ManagedLedgerImpl ledger = + (ManagedLedgerImpl) factory.open("testBatchReadEntriesCallback"); + @Cleanup + ManagedCursorImpl cursor = (ManagedCursorImpl) ledger.openCursor("test-cursor"); + for (int i = 0; i < 10; i++) { + ledger.addEntry(("dummy-entry-" + i).getBytes(Encoding)); + } + + CountDownLatch latch = new CountDownLatch(1); + AtomicBoolean failed = new AtomicBoolean(false); + List entries = new ArrayList<>(); + OpReadEntry opReadEntry = OpReadEntry.create(cursor, cursor.readPosition, 10, new ReadEntriesCallback() { + @Override + public void readEntriesComplete(List entries0, Object ctx) { + entries.addAll(entries0); + latch.countDown(); + } + + @Override + public void readEntriesFailed(ManagedLedgerException exception, Object ctx) { + failed.set(true); + latch.countDown(); + } + }, null, ledger.lastConfirmedEntry, position -> position.getEntryId() % 2 == 0, true); + + List entryIds = List.of(1L, 3L, 5L, 7L, 9L); + ManagedLedgerImpl.BatchReadEntriesCallback callback = new ManagedLedgerImpl + .BatchReadEntriesCallback(entryIds, opReadEntry, null); + long ledgerId = ledger.currentLedger.getId(); + + callback.readEntriesComplete(List.of(EntryImpl.create(ledgerId, 1, new byte[1])), null); + callback.readEntriesComplete(List.of(EntryImpl.create(ledgerId, 3, new byte[3])), null); + callback.readEntriesComplete(List.of(EntryImpl.create(ledgerId, 7, new byte[7])), null); + callback.readEntriesFailed( + new ManagedLedgerException.InvalidCursorPositionException("Invalid cursor position"), null); + // After call readEntriesFailed, the following readEntriesComplete should be ignored. + callback.readEntriesComplete(List.of(EntryImpl.create(ledgerId, 5, new byte[5])), null); + + latch.await(); + // should not fail + assertFalse(failed.get()); + assertEquals(entries.size(), 2); + + // `entries` should be only the entries with entryId 1 and 3. + assertEquals(entries.get(0).getEntryId(), 1); + assertEquals(entries.get(1).getEntryId(), 3); + + // ReadPosition should be updated to [4] + assertEquals(cursor.getReadPosition().getEntryId(), 4); + } + + @Test + public void testReadEntriesFromDifferentLedgersWithSkipCondition() throws Exception { + ManagedLedgerConfig config = new ManagedLedgerConfig(); + config.setMaxEntriesPerLedger(5); + config.setMinimumRolloverTime(0, TimeUnit.SECONDS); + @Cleanup + ManagedLedgerImpl ledger = (ManagedLedgerImpl) factory.open("testReadEntriesWithSkipCondition", config); + ledger = Mockito.spy(ledger); + + AtomicInteger counter = new AtomicInteger(); + Mockito.doAnswer(inv -> { + counter.incrementAndGet(); + return inv.callRealMethod(); + }).when(ledger).asyncReadEntries(Mockito.any()); + @Cleanup + ManagedCursorImpl cursor = (ManagedCursorImpl) ledger.openCursor("test-cursor"); + + Position lastPosition = null; + for (int i = 0; i < 12; i++) { + lastPosition = ledger.addEntry(("dummy-entry-" + i).getBytes(Encoding)); + } + + CountDownLatch latch = new CountDownLatch(1); + AtomicBoolean failed = new AtomicBoolean(false); + List entries = new ArrayList<>(); + cursor.asyncReadEntriesWithSkip(100, -1, new ReadEntriesCallback() { + @Override + public void readEntriesComplete(List entries0, Object ctx) { + entries.addAll(entries0); + latch.countDown(); + } + + @Override + public void readEntriesFailed(ManagedLedgerException exception, Object ctx) { + failed.set(true); + latch.countDown(); + } + }, null, PositionFactory.LATEST, position -> position.getEntryId() % 2 == 0); + + latch.await(); + assertFalse(failed.get()); + assertEquals(entries.size(), 5); + // Read entries from 3 ledgers, the counter is 3. + assertEquals(counter.get(), 3); + Position readPosition = cursor.getReadPosition(); + assertTrue(readPosition.getLedgerId() == lastPosition.getLedgerId() + && readPosition.getEntryId() == lastPosition.getEntryId() + 1); + } + + @Test + public void testReadEntriesFromOneSameLedgerWithSkipCondition() throws Exception { + ManagedLedgerConfig config = new ManagedLedgerConfig(); + @Cleanup + ManagedLedgerImpl ledger = (ManagedLedgerImpl) factory.open("testReadEntriesWithSkipCondition", config); + ledger = Mockito.spy(ledger); + + AtomicInteger counter = new AtomicInteger(); + Mockito.doAnswer(inv -> { + counter.incrementAndGet(); + return inv.callRealMethod(); + }).when(ledger).asyncReadEntries(Mockito.any()); + + @Cleanup + ManagedCursorImpl cursor = (ManagedCursorImpl) ledger.openCursor("test-cursor"); + + Position lastPosition = null; + for (int i = 0; i < 10; i++) { + lastPosition = ledger.addEntry(("dummy-entry-" + i).getBytes(Encoding)); + } + + CountDownLatch latch = new CountDownLatch(1); + AtomicBoolean failed = new AtomicBoolean(false); + List entries = new ArrayList<>(); + cursor.asyncReadEntriesWithSkip(100, -1, new ReadEntriesCallback() { + @Override + public void readEntriesComplete(List entries0, Object ctx) { + entries.addAll(entries0); + latch.countDown(); + } + + @Override + public void readEntriesFailed(ManagedLedgerException exception, Object ctx) { + failed.set(true); + latch.countDown(); + } + }, null, PositionFactory.LATEST, position -> position.getEntryId() % 2 == 0); + + latch.await(); + assertEquals(counter.get(), 1); + + assertFalse(failed.get()); + assertEquals(entries.size(), 5); + + Position readPosition = cursor.getReadPosition(); + assertTrue(readPosition.getLedgerId() == lastPosition.getLedgerId() + && readPosition.getEntryId() == lastPosition.getEntryId() + 1); + } + + // ===== Unit tests for buildRanges ===== + + private static List> callBuildRanges(long firstEntry, long lastEntry, long ledgerId, + Predicate skipCondition, + List entryIds) { + return ManagedLedgerImpl.buildRanges(firstEntry, lastEntry, ledgerId, skipCondition, entryIds); + } + + @Test + public void testBuildRangesNoSkip() { + List entryIds = new ArrayList<>(); + List> ranges = callBuildRanges(0, 4, 1, pos -> false, entryIds); + + assertEquals(ranges.size(), 1); + assertEquals(ranges.get(0).getLeft().longValue(), 0L); + assertEquals(ranges.get(0).getRight().longValue(), 4L); + assertEquals(entryIds, Arrays.asList(0L, 1L, 2L, 3L, 4L)); + } + + @Test + public void testBuildRangesAllSkipped() { + List entryIds = new ArrayList<>(); + List> ranges = callBuildRanges(0, 4, 1, pos -> true, entryIds); + + assertTrue(ranges.isEmpty()); + assertTrue(entryIds.isEmpty()); + } + + @Test + public void testBuildRangesSingleEntryNotSkipped() { + List entryIds = new ArrayList<>(); + List> ranges = callBuildRanges(5, 5, 1, pos -> false, entryIds); + + assertEquals(ranges.size(), 1); + assertEquals(ranges.get(0).getLeft().longValue(), 5L); + assertEquals(ranges.get(0).getRight().longValue(), 5L); + assertEquals(entryIds, Collections.singletonList(5L)); + } + + @Test + public void testBuildRangesSingleEntrySkipped() { + List entryIds = new ArrayList<>(); + List> ranges = callBuildRanges(5, 5, 1, pos -> true, entryIds); + + assertTrue(ranges.isEmpty()); + assertTrue(entryIds.isEmpty()); + } + + @Test + public void testBuildRangesSkipFirst() { + List entryIds = new ArrayList<>(); + List> ranges = callBuildRanges(0, 4, 1, + pos -> pos.getEntryId() == 0, entryIds); + + assertEquals(ranges.size(), 1); + assertEquals(ranges.get(0).getLeft().longValue(), 1L); + assertEquals(ranges.get(0).getRight().longValue(), 4L); + assertEquals(entryIds, Arrays.asList(1L, 2L, 3L, 4L)); + } + + @Test + public void testBuildRangesSkipLast() { + List entryIds = new ArrayList<>(); + List> ranges = callBuildRanges(0, 4, 1, + pos -> pos.getEntryId() == 4, entryIds); + + assertEquals(ranges.size(), 1); + assertEquals(ranges.get(0).getLeft().longValue(), 0L); + assertEquals(ranges.get(0).getRight().longValue(), 3L); + assertEquals(entryIds, Arrays.asList(0L, 1L, 2L, 3L)); + } + + @Test + public void testBuildRangesMultipleDisjointRanges() { + // Skip entry 2 and 5: ranges should be [0,1], [3,4], [6,6] + List entryIds = new ArrayList<>(); + List> ranges = callBuildRanges(0, 6, 1, + pos -> pos.getEntryId() == 2 || pos.getEntryId() == 5, entryIds); + + assertEquals(ranges.size(), 3); + assertEquals(ranges.get(0).getLeft().longValue(), 0L); + assertEquals(ranges.get(0).getRight().longValue(), 1L); + assertEquals(ranges.get(1).getLeft().longValue(), 3L); + assertEquals(ranges.get(1).getRight().longValue(), 4L); + assertEquals(ranges.get(2).getLeft().longValue(), 6L); + assertEquals(ranges.get(2).getRight().longValue(), 6L); + assertEquals(entryIds, Arrays.asList(0L, 1L, 3L, 4L, 6L)); + } + + @Test + public void testBuildRangesConsecutiveSkipsInMiddle() { + // Skip entries 2,3,4: ranges should be [0,1], [5,6] + List entryIds = new ArrayList<>(); + List> ranges = callBuildRanges(0, 6, 1, + pos -> pos.getEntryId() >= 2 && pos.getEntryId() <= 4, entryIds); + + assertEquals(ranges.size(), 2); + assertEquals(ranges.get(0).getLeft().longValue(), 0L); + assertEquals(ranges.get(0).getRight().longValue(), 1L); + assertEquals(ranges.get(1).getLeft().longValue(), 5L); + assertEquals(ranges.get(1).getRight().longValue(), 6L); + assertEquals(entryIds, Arrays.asList(0L, 1L, 5L, 6L)); + } + + @Test + public void testBuildRangesSkipEvenEntries() { + // Skip even entries: each range is a single entry [1,1], [3,3], [5,5] + List entryIds = new ArrayList<>(); + List> ranges = callBuildRanges(0, 5, 1, + pos -> pos.getEntryId() % 2 == 0, entryIds); + + assertEquals(ranges.size(), 3); + assertEquals(ranges.get(0), Pair.of(1L, 1L)); + assertEquals(ranges.get(1), Pair.of(3L, 3L)); + assertEquals(ranges.get(2), Pair.of(5L, 5L)); + assertEquals(entryIds, Arrays.asList(1L, 3L, 5L)); + } + + @Test + public void testBuildRangesPredicateUsesLedgerId() { + // Skip entries in ledger 1, but predicate checks ledgerId + List entryIds = new ArrayList<>(); + List> ranges = callBuildRanges(0, 2, 1, + pos -> pos.getLedgerId() == 1, entryIds); + + assertTrue(ranges.isEmpty()); + assertTrue(entryIds.isEmpty()); + + // Now with a different ledgerId, nothing should be skipped + entryIds = new ArrayList<>(); + ranges = callBuildRanges(0, 2, 2, + pos -> pos.getLedgerId() == 1, entryIds); + + assertEquals(ranges.size(), 1); + assertEquals(ranges.get(0), Pair.of(0L, 2L)); + assertEquals(entryIds, Arrays.asList(0L, 1L, 2L)); + } + + @Test + public void testBuildRangesLargeEntryIds() { + List entryIds = new ArrayList<>(); + // Use values large enough to test long range but not so large as to overflow + // the ArrayList capacity calculation in the caller + long first = 999_999_999_999L; + long last = 999_999_999_999L + 2; + List> ranges = callBuildRanges(first, last, 1, pos -> false, entryIds); + + assertEquals(ranges.size(), 1); + assertEquals(ranges.get(0).getLeft().longValue(), first); + assertEquals(ranges.get(0).getRight().longValue(), last); + assertEquals(entryIds.size(), 3); + assertEquals(entryIds.get(0).longValue(), first); + assertEquals(entryIds.get(2).longValue(), last); + } + + @Test + public void testBuildRangesOnlyMiddleKept() { + // Only keep entry 3, skip everything else + List entryIds = new ArrayList<>(); + List> ranges = callBuildRanges(0, 6, 1, + pos -> pos.getEntryId() != 3, entryIds); + + assertEquals(ranges.size(), 1); + assertEquals(ranges.get(0), Pair.of(3L, 3L)); + assertEquals(entryIds, Collections.singletonList(3L)); + } + + @Test + public void testBuildRangesEmptyEntriesIdsList() { + // Verify entryIds list starts empty and is populated correctly + List entryIds = new ArrayList<>(); + callBuildRanges(10, 12, 5, pos -> pos.getEntryId() == 11, entryIds); + + assertEquals(entryIds, Arrays.asList(10L, 12L)); + } + + @Test + public void testBuildRangesPositionFieldsCorrect() { + // Verify that the predicate receives correct ledgerId and entryId + List seenLedgerIds = new ArrayList<>(); + List seenEntryIds = new ArrayList<>(); + List entryIds = new ArrayList<>(); + + callBuildRanges(3, 5, 99, pos -> { + seenLedgerIds.add(pos.getLedgerId()); + seenEntryIds.add(pos.getEntryId()); + return false; + }, entryIds); + + assertEquals(seenLedgerIds, Arrays.asList(99L, 99L, 99L)); + assertEquals(seenEntryIds, Arrays.asList(3L, 4L, 5L)); + } } diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/MessageRedeliveryTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/MessageRedeliveryTest.java index 962749fbd49e9..4577db8c4167e 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/MessageRedeliveryTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/MessageRedeliveryTest.java @@ -218,7 +218,7 @@ public void testRedelivery(boolean useOpenRangeSet) throws Exception { assertEquals(cursor.getIndividuallyDeletedMessagesSet().size(), 0); // markDelete position should be one position behind read position - assertEquals(cursor.getReadPosition(), cursor.getMarkDeletedPosition().getNext()); + assertEquals(cursor.getReadPosition(), cursor.getNextAvailablePosition(cursor.getMarkDeletedPosition())); producer.close(); consumer2.close();