Skip to content

Commit e0687e2

Browse files
committed
[test] Add test for putAndCleanOverlapRange truncated key handling
- Verifies that original keys are used instead of truncated keys in encloses() check - Ensures buckets are not incorrectly replaced when ranges don't truly enclose - Tests that adding non-overlapping messages maintains correct bucket and message counts - Prevents regression where truncated keys from subRangeMap() pass encloses() incorrectly
1 parent 4065f1f commit e0687e2

1 file changed

Lines changed: 82 additions & 0 deletions

File tree

pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/bucket/BucketDelayedDeliveryTrackerTest.java

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -807,4 +807,86 @@ public void testOverlappingBucketsCleanupDuringRecovery() throws Exception {
807807
storage.clean();
808808
}
809809
}
810+
811+
/**
812+
* Test that putAndCleanOverlapRange correctly uses original keys instead of truncated keys
813+
* when checking if a new range encloses existing buckets.
814+
*
815+
* This prevents the bug where a truncated key from subRangeMap() would incorrectly pass
816+
* the encloses() check, causing a bucket to be replaced when it shouldn't be.
817+
*/
818+
@Test
819+
public void testPutAndCleanOverlapRangeWithTruncatedKeys() throws Exception {
820+
// Setup mocks
821+
AbstractPersistentDispatcherMultipleConsumers testDispatcher =
822+
mock(AbstractPersistentDispatcherMultipleConsumers.class);
823+
Clock testClock = mock(Clock.class);
824+
AtomicLong testClockTime = new AtomicLong();
825+
when(testClock.millis()).then(x -> testClockTime.get());
826+
827+
MockBucketSnapshotStorage storage = new MockBucketSnapshotStorage();
828+
storage.start();
829+
830+
ManagedCursor cursor = new MockManagedCursor("test_truncated_cursor");
831+
doReturn(cursor).when(testDispatcher).getCursor();
832+
doReturn("persistent://public/default/testTruncated / " + cursor.getName())
833+
.when(testDispatcher).getName();
834+
835+
try {
836+
// Create tracker
837+
BucketDelayedDeliveryTracker tracker = new BucketDelayedDeliveryTracker(
838+
testDispatcher, timer, 100000, testClock, true, storage,
839+
3, TimeUnit.MILLISECONDS.toMillis(10), -1, 50);
840+
841+
// Add messages to create a bucket [1-6]
842+
for (int i = 1; i <= 6; i++) {
843+
tracker.addMessage(i, i, i * 10);
844+
}
845+
846+
// Wait for bucket to be created
847+
Awaitility.await().atMost(5, TimeUnit.SECONDS).untilAsserted(() -> {
848+
assertTrue(tracker.getImmutableBuckets().asMapOfRanges().size() >= 1,
849+
"Should have created at least one bucket");
850+
});
851+
852+
int initialBucketCount = tracker.getImmutableBuckets().asMapOfRanges().size();
853+
long initialMessageCount = tracker.getNumberOfDelayedMessages();
854+
855+
// Now add messages that would create a bucket [7-9]
856+
// This should NOT replace the existing bucket [1-6]
857+
for (int i = 7; i <= 9; i++) {
858+
tracker.addMessage(i, i, i * 10);
859+
}
860+
861+
// Wait for new bucket operations
862+
Awaitility.await().atMost(5, TimeUnit.SECONDS).untilAsserted(() -> {
863+
assertTrue(tracker.getImmutableBuckets().asMapOfRanges().values().stream()
864+
.noneMatch(x -> x.merging),
865+
"All buckets should finish processing");
866+
});
867+
868+
// Verify bucket count increased (or stayed same if they got merged)
869+
int finalBucketCount = tracker.getImmutableBuckets().asMapOfRanges().size();
870+
assertTrue(finalBucketCount >= initialBucketCount,
871+
"Bucket count should not decrease when adding non-overlapping ranges");
872+
873+
// Verify all messages are tracked
874+
long finalMessageCount = tracker.getNumberOfDelayedMessages();
875+
assertTrue(finalMessageCount >= initialMessageCount,
876+
String.format("Message count should not decrease: initial=%d, final=%d",
877+
initialMessageCount, finalMessageCount));
878+
879+
// Verify no bucket was incorrectly replaced
880+
// If putAndCleanOverlapRange used truncated keys, it might have incorrectly
881+
// removed a bucket that shouldn't have been removed
882+
tracker.getImmutableBuckets().asMapOfRanges().forEach((range, bucket) -> {
883+
assertTrue(bucket.getNumberBucketDelayedMessages() > 0,
884+
"All buckets should have messages - bucket " + range + " is empty");
885+
});
886+
887+
tracker.close();
888+
} finally {
889+
storage.clean();
890+
}
891+
}
810892
}

0 commit comments

Comments
 (0)