Skip to content

HDDS-13661. Fix flaky TestKeyDeletingService#testPurgeKeysRequestBatching #10025

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
adoroszlai merged 1 commit into from
Apr 3, 2026
Merged

HDDS-13661. Fix flaky TestKeyDeletingService#testPurgeKeysRequestBatching #10025

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 16 additions & 16 deletions ...zone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestKeyDeletingService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1166,18 +1166,19 @@ void cleanup() {
}

private void customizeConfig() {
// Define a small Ratis limit to force multiple batches for testing
// The actual byte size of protobuf messages depends on content.
// A small value like 1KB or 2KB should ensure batching for ~10-20 keys.
final int testRatisLimitBytes = 1024; // 2 KB to encourage multiple batches, 90% of the actualRatisLimitBytes.
// Set the specific Ratis limit for this test
// Define a small Ratis limit to force multiple batches for testing.
// With 50 keys and 1024 bytes limit (90% = ~921 bytes effective), this
// produces ~7 batches, confirming the batching logic works correctly.
final int testRatisLimitBytes = 1024;
conf.setStorageSize(OMConfigKeys.OZONE_OM_RATIS_LOG_APPENDER_QUEUE_BYTE_LIMIT,
testRatisLimitBytes, StorageUnit.BYTES);
// Use a very large service interval so the background thread never fires
// during the test, preventing concurrent processing with runPeriodicalTaskNow().
conf.setTimeDuration(OZONE_BLOCK_DELETING_SERVICE_INTERVAL, 1, TimeUnit.DAYS);
Copy link
Copy Markdown
Member

@peterxcli peterxcli Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should suspend the background worker instead.

Copy link
Copy Markdown
Contributor Author

@arunsarin85 arunsarin85 Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting the interval to 1 day is effectively "suspending the background worker for the test duration" without needing code changes.

Reason:
The service is already suspended at the start of the test, but runPeriodicalTaskNow() requires a resume() first - shouldRun() is checked inside KeyDeletingTask.call(), so a suspended service silently does nothing.

Problem:
That when resume() is called, the 100ms scheduler is back up and running. Since runPeriodicalTaskNow() takes about 15 seconds, the background thread fires about 150 times at the same time, which could make the captured request count higher.

}

@Test
@DisplayName("Verify PurgeKeysRequest is batched according to Ratis byte limit")
@Flaky("HDDS-13661")
void testPurgeKeysRequestBatching() throws Exception {
keyDeletingService.suspend();

Expand All @@ -1191,20 +1192,19 @@ void testPurgeKeysRequestBatching() throws Exception {
// Mock submitRequest to capture requests and return success
mockedRatisUtils.when(() -> OzoneManagerRatisUtils.submitRequest(
any(OzoneManager.class),
requestCaptor.capture(), // Capture the OMRequest here
requestCaptor.capture(),
any(),
anyLong()))
.thenAnswer(invocation -> {
// Return a successful OMResponse for each captured request
return OzoneManagerProtocolProtos.OMResponse.newBuilder()
.setCmdType(OzoneManagerProtocolProtos.Type.PurgeKeys)
.setStatus(OzoneManagerProtocolProtos.Status.OK)
.build();
});
.thenAnswer(invocation -> OzoneManagerProtocolProtos.OMResponse.newBuilder()
.setCmdType(OzoneManagerProtocolProtos.Type.PurgeKeys)
.setStatus(OzoneManagerProtocolProtos.Status.OK)
.build());

final int numKeysToCreate = 50; // Create enough keys to ensure multiple batches
// Create and delete keys using the test-specific managers
final int numKeysToCreate = 50;
createAndDeleteKeys(numKeysToCreate, 1);
// Wait for all delete operations to be flushed from the DoubleBuffer to
// RocksDB, so that getPendingDeletionKeys() can see all 50 entries.
om.awaitDoubleBufferFlush();

keyDeletingService.resume();

Expand Down
Loading