You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "jojochuang (via GitHub)" <gi...@apache.org> on 2026/07/11 01:56:21 UTC
[PR] HDDS-15828. Fix flaky TestDirectoryDeletingServiceWithFSO snapshot cleanup. [ozone]
jojochuang opened a new pull request, #10726:
URL: https://github.com/apache/ozone/pull/10726
## What changes were proposed in this pull request?
HDDS-15828. Fix flaky TestDirectoryDeletingServiceWithFSO snapshot cleanup.
## What is the link to the Apache JIRA
Previously failed in https://github.com/apache/ozone/actions/runs/29127546564/job/86477855080
Wait for SnapshotDeletingService to purge deleted snapshots before snapshot tests return on the shared mini-cluster, so later tests do not chain off stale global snapshot entries.
## How was this patch tested?
flaky-test-check 10x10 all passed: https://github.com/jojochuang/ozone/actions/runs/29134805647
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15828. Fix flaky TestDirectoryDeletingServiceWithFSO snapshot cleanup. [ozone]
Posted by "arunsarin85 (via GitHub)" <gi...@apache.org>.
arunsarin85 commented on PR #10726:
URL: https://github.com/apache/ozone/pull/10726#issuecomment-5556762666
Hi @SaketaChalamchala @jojochuang
I've addressed all the review comments in a follow-up PR: https://github.com/apache/ozone/pull/11212
Changes made on top of this patch:
- Moved `waitForSnapshotsPurged` and `dirDeletingService.resume()` from the test body into `@AfterEach cleanup()` using instance tracking fields (`snapshotCountAfterTest`, `pendingResumeDds`, `needsTableCleanup`), so services are always resumed and snapshots always purged even when a test fails mid-way.
- Moved `waitForSnapshotsPurged` and `cleanupTables()` from `testDirDeletedTableCleanUpForSnapshot` to `@AfterEach cleanup()` in the same way.
- Removed the redundant `cluster.getOzoneManager().awaitDoubleBufferFlush()` before `waitForSnapshotsPurged` in `testDirDeletedTableCleanUpForSnapshot`.
- Fixed mis-indented try/catch braces inside the `waitForSnapshotsPurged` lambda (checkstyle).
- Removed the redundant `assertTableRowCount` call after `waitFor` in `waitForSnapshotsPurged` - `waitFor` already guarantees the condition before returning.
Flaky Test run after the fix : https://github.com/arunsarin85/ozone/actions/runs/33992835360
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15828. Fix flaky TestDirectoryDeletingServiceWithFSO snapshot cleanup. [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10726:
URL: https://github.com/apache/ozone/pull/10726#issuecomment-5223399233
This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15828. Fix flaky TestDirectoryDeletingServiceWithFSO snapshot cleanup. [ozone]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15828. Fix flaky TestDirectoryDeletingServiceWithFSO snapshot cleanup. [ozone]
Posted by "SaketaChalamchala (via GitHub)" <gi...@apache.org>.
SaketaChalamchala commented on code in PR #10726:
URL: https://github.com/apache/ozone/pull/10726#discussion_r3606015040
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/service/TestDirectoryDeletingServiceWithFSO.java:
##########
@@ -789,6 +773,8 @@ public void testDirDeletedTableCleanUpForSnapshot() throws Exception {
// Manual cleanup deletedDirTable for next tests
client.getObjectStore().deleteSnapshot(volumeName, bucketName, "snap1");
+ cluster.getOzoneManager().awaitDoubleBufferFlush();
+ waitForSnapshotsPurged(snapshotInfoTable, 0);
Review Comment:
Same comment as above. It's better to move `waitForSnapshotsPurged` and `cleanupTables()` if suspended to the test teardown method `cleanup`.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/service/TestDirectoryDeletingServiceWithFSO.java:
##########
@@ -789,6 +773,8 @@ public void testDirDeletedTableCleanUpForSnapshot() throws Exception {
// Manual cleanup deletedDirTable for next tests
client.getObjectStore().deleteSnapshot(volumeName, bucketName, "snap1");
+ cluster.getOzoneManager().awaitDoubleBufferFlush();
Review Comment:
```suggestion
```
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15828. Fix flaky TestDirectoryDeletingServiceWithFSO snapshot cleanup. [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #10726: HDDS-15828. Fix flaky TestDirectoryDeletingServiceWithFSO snapshot cleanup.
URL: https://github.com/apache/ozone/pull/10726
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15828. Fix flaky TestDirectoryDeletingServiceWithFSO snapshot cleanup. [ozone]
Posted by "SaketaChalamchala (via GitHub)" <gi...@apache.org>.
SaketaChalamchala commented on code in PR #10726:
URL: https://github.com/apache/ozone/pull/10726#discussion_r3606011608
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/service/TestDirectoryDeletingServiceWithFSO.java:
##########
@@ -659,23 +659,7 @@ public void testAOSKeyDeletingWithSnapshotCreateParallelExecution()
service.runPeriodicalTaskNow();
store.deleteSnapshot(testVolumeName, testBucketName, snap1);
cluster.getOzoneManager().awaitDoubleBufferFlush();
- cluster.restartOzoneManager();
- cluster.waitForClusterToBeReady();
- cluster.getOzoneManager().awaitDoubleBufferFlush();
- SnapshotDeletingService snapshotDeletingService =
- cluster.getOzoneManager().getKeyManager().getSnapshotDeletingService();
- GenericTestUtils.waitFor(() -> {
- try {
- snapshotDeletingService.runPeriodicalTaskNow();
- cluster.getOzoneManager().awaitDoubleBufferFlush();
- long currentSnapshotCount = cluster.getOzoneManager().getMetadataManager()
- .countRowsInTable(cluster.getOzoneManager().getMetadataManager().getSnapshotInfoTable());
- return currentSnapshotCount <= initialSnapshotCount;
- } catch (Exception e) {
- throw new RuntimeException(e);
- }
- }, 100, 10000);
- assertTableRowCount(cluster.getOzoneManager().getMetadataManager().getSnapshotInfoTable(), initialSnapshotCount);
+ waitForSnapshotsPurged(snapshotInfoTable, initialSnapshotCount);
dirDeletingService.resume();
Review Comment:
Thanks for the patch @jojochuang . It might be better to move `waitForSnapshotsPurged` and `dirDeletingService.resume()` if suspended to the test teardown method `cleanup`
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15828. Fix flaky TestDirectoryDeletingServiceWithFSO snapshot cleanup. [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #10726:
URL: https://github.com/apache/ozone/pull/10726#issuecomment-5825494770
Superceded by https://github.com/apache/ozone/pull/11212
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15828. Fix flaky TestDirectoryDeletingServiceWithFSO snapshot cleanup. [ozone]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #10726:
URL: https://github.com/apache/ozone/pull/10726#discussion_r3573251566
##########
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) {
+ throw new RuntimeException("Failed to run SnapshotDeletingService purge task", e);
+}
+ }, 1000, 120000);
+ assertTableRowCount(snapshotInfoTable, expectedCount);
+ }
Review Comment:
The new waitForSnapshotsPurged() helper has mis-indented try/catch braces inside the lambda, which is likely to violate checkstyle and reduces readability. It also calls assertTableRowCount() after a waitFor() that already waits for the exact same condition, potentially doubling the wait time (up to 4 minutes) on slow runs.
##########
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:
The test suspends SnapshotDeletingService (and DirectoryDeletingService) but only resumes it on the happy path. If the test fails before the resume call, the shared mini-cluster can be left with SnapshotDeletingService suspended, potentially cascading failures into later tests in this class/module. Consider wrapping the body after suspending in a try/finally that resumes both services.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15828. Fix flaky TestDirectoryDeletingServiceWithFSO snapshot cleanup. [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10726:
URL: https://github.com/apache/ozone/pull/10726#issuecomment-5304848594
Thank you for your contribution. This PR is being closed due to inactivity. Please contact a maintainer if you would like to reopen it.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15828. Fix flaky TestDirectoryDeletingServiceWithFSO snapshot cleanup. [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl closed pull request #10726: HDDS-15828. Fix flaky TestDirectoryDeletingServiceWithFSO snapshot cleanup.
URL: https://github.com/apache/ozone/pull/10726
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org