chihsuan commented on code in PR #11212:
URL: https://github.com/apache/ozone/pull/11212#discussion_r3968975585
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/service/TestDirectoryDeletingServiceWithFSO.java:
##########
@@ -149,7 +154,21 @@ public static void teardown() {
}
@AfterEach
- public void cleanup() throws InterruptedException, TimeoutException {
+ public void cleanup() throws Exception {
+ if (snapshotCountAfterTest >= 0) {
+ Table<String, SnapshotInfo> snapshotInfoTable =
+
cluster.getOzoneManager().getMetadataManager().getSnapshotInfoTable();
+ waitForSnapshotsPurged(snapshotInfoTable, snapshotCountAfterTest);
+ snapshotCountAfterTest = -1;
+ }
+ if (pendingResumeDds != null) {
Review Comment:
If this times out or throws, `pendingResumeDds.resume()` is skipped. Would
it make sense to wrap this in a try/finally?
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/service/TestDirectoryDeletingServiceWithFSO.java:
##########
@@ -787,9 +790,10 @@ public void testDirDeletedTableCleanUpForSnapshot() throws
Exception {
assertSubPathsCount(dirDeletingService::getMovedDirsCount, 4);
assertSubPathsCount(dirDeletingService::getDeletedDirsCount, 0);
- // Manual cleanup deletedDirTable for next tests
+ // Manual cleanup deletedDirTable for next tests - completed in @AfterEach
cleanup()
client.getObjectStore().deleteSnapshot(volumeName, bucketName, "snap1");
- cleanupTables();
+ snapshotCountAfterTest = 0;
Review Comment:
Should these flags be set right after the snapshot is created instead? Set
here, a failure earlier in the test may leave snap1 and the dirty tables
behind, which is the case the teardown is meant to cover.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/service/TestDirectoryDeletingServiceWithFSO.java:
##########
@@ -823,6 +827,27 @@ 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);
Review Comment:
Would reusing `assertTableRowCount()` here work? It already polls the same
count every second for two minutes, keeping the test thread out of the service.
```java
cluster.getOzoneManager().getKeyManager().getSnapshotDeletingService().resume();
assertTableRowCount(snapshotInfoTable, expectedCount);
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/service/TestDirectoryDeletingServiceWithFSO.java:
##########
@@ -571,10 +590,11 @@ 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();
Review Comment:
Could we track the need to resume `SnapshotDeletingService` immediately
after suspending it, as we do with `pendingResumeDds`? `snapshotCountAfterTest`
is assigned later, so an exception in between would cause `@AfterEach` to skip
resuming SDS.
--
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]
