Copilot commented on code in PR #10726:
URL: https://github.com/apache/ozone/pull/10726#discussion_r3562880577
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/service/TestDirectoryDeletingServiceWithFSO.java:
##########
@@ -571,7 +571,7 @@ public void
testAOSKeyDeletingWithSnapshotCreateParallelExecution()
Table<String, SnapshotInfo> snapshotInfoTable =
omMetadataManager.getSnapshotInfoTable();
Table<String, OmKeyInfo> deletedDirTable =
omMetadataManager.getDeletedDirTable();
Table<String, String> renameTable =
omMetadataManager.getSnapshotRenamedTable();
-
cluster.getOzoneManager().getKeyManager().getSnapshotDeletingService().shutdown();
+
cluster.getOzoneManager().getKeyManager().getSnapshotDeletingService().suspend();
DirectoryDeletingService dirDeletingService =
cluster.getOzoneManager().getKeyManager().getDirDeletingService();
// Suspend KeyDeletingService
dirDeletingService.suspend();
Review Comment:
This test suspends SnapshotDeletingService and DirectoryDeletingService on
the shared static mini-cluster, but only resumes them on the normal success
path. If an assertion/exception occurs before the resume calls, later tests in
this class can run with background services still suspended, causing cross-test
flakiness.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/service/TestDirectoryDeletingServiceWithFSO.java:
##########
@@ -823,6 +809,28 @@ static void assertSubPathsCount(LongSupplier pathCount,
long expectedCount)
1000, 120000);
}
+ /**
+ * Wait until SnapshotDeletingService has purged snapshots and
snapshotInfoTable
+ * reaches the expected row count.
+ */
+ private void waitForSnapshotsPurged(Table<String, SnapshotInfo>
snapshotInfoTable,
+ int expectedCount) throws Exception {
+ SnapshotDeletingService snapshotDeletingService =
+ cluster.getOzoneManager().getKeyManager().getSnapshotDeletingService();
+ snapshotDeletingService.resume();
+ GenericTestUtils.waitFor(() -> {
+ try {
+ snapshotDeletingService.runPeriodicalTaskNow();
+ cluster.getOzoneManager().awaitDoubleBufferFlush();
+ return cluster.getOzoneManager().getMetadataManager()
+ .countRowsInTable(snapshotInfoTable) == expectedCount;
+ } catch (Exception e) {
+ return false;
+ }
Review Comment:
Swallowing exceptions inside the wait loop (returning false) can hide the
real failure and only surface as a timeout after 2 minutes. This makes
diagnosing snapshot purge failures harder; rethrow to fail fast with the
underlying cause.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
