You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "echonesis (via GitHub)" <gi...@apache.org> on 2026/07/20 11:19:44 UTC

[PR] HDDS-10314. Speed up TestDatanodeHddsVolumeFailureDetection [ozone]

echonesis opened a new pull request, #10819:
URL: https://github.com/apache/ozone/pull/10819

   ## What changes were proposed in this pull request?
   
   This PR reduces the execution time of `TestDatanodeHddsVolumeFailureDetection` by reusing one `MiniOzoneCluster` for each container schema version instead of creating a new cluster for every test method.
   
   The test class is converted to a JUnit parameterized class covering schema V2 and schema V3. After each test, the failed volume is restored so that the cluster can be reused safely:
   - Schema V3 restarts the datanode because a volume failure closes the shared RocksDB instance.
   - Schema V2 restores the per-container volume and adds it back to the active volume map.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-10314
   
   ## How was this patch tested?
   
   Local Test
   ```
   mvn -pl :ozone-integration-test test -Dtest=TestDatanodeHddsVolumeFailureDetection -DskipShade -DskipRecon
   ```
   
   GitHub Actions CI: https://github.com/echonesis/ozone/actions/runs/29737666866


-- 
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-10314. Speed up TestDatanodeHddsVolumeFailureDetection [ozone]

Posted by "chihsuan (via GitHub)" <gi...@apache.org>.
chihsuan commented on code in PR #10819:
URL: https://github.com/apache/ozone/pull/10819#discussion_r3638136059


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/volume/TestDatanodeHddsVolumeFailureDetection.java:
##########
@@ -60,176 +67,219 @@
 import org.apache.hadoop.ozone.container.common.volume.HddsVolume;
 import org.apache.hadoop.ozone.container.common.volume.MutableVolumeSet;
 import org.apache.hadoop.ozone.container.common.volume.StorageVolume;
+import org.apache.hadoop.ozone.container.common.volume.StorageVolume.VolumeState;
 import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainerData;
 import org.apache.hadoop.ozone.container.ozoneimpl.OzoneContainer;
 import org.apache.hadoop.ozone.dn.DatanodeTestUtils;
-import org.junit.jupiter.params.ParameterizedTest;
+import org.apache.ozone.test.GenericTestUtils;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.api.parallel.Execution;
+import org.junit.jupiter.api.parallel.ExecutionMode;
+import org.junit.jupiter.params.AfterParameterizedClassInvocation;
+import org.junit.jupiter.params.BeforeParameterizedClassInvocation;
+import org.junit.jupiter.params.Parameter;
+import org.junit.jupiter.params.ParameterizedClass;
 import org.junit.jupiter.params.provider.ValueSource;
 
 /**
  * This class tests datanode can detect failed volumes.
  */
+@ParameterizedClass
+@ValueSource(booleans = {true, false})
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
+@Execution(ExecutionMode.SAME_THREAD)
 class TestDatanodeHddsVolumeFailureDetection {
   private static final int KEY_SIZE = 128;
 
-  @ParameterizedTest
-  @ValueSource(booleans = {true, false})
-  void corruptChunkFile(boolean schemaV3) throws Exception {
-    try (MiniOzoneCluster cluster = newCluster(schemaV3)) {
-      try (OzoneClient client = cluster.newClient()) {
-        OzoneBucket bucket = TestDataUtil.createVolumeAndBucket(client);
-
-        // write a file
-        String keyName = UUID.randomUUID().toString();
-        long containerId = createKey(bucket, keyName);
-
-        // corrupt chunk file by rename file->dir
-        HddsDatanodeService dn = cluster.getHddsDatanodes().get(0);
-        OzoneContainer oc = dn.getDatanodeStateMachine().getContainer();
-        MutableVolumeSet volSet = oc.getVolumeSet();
-        StorageVolume vol0 = volSet.getVolumesList().get(0);
-        HddsVolume volume = assertInstanceOf(HddsVolume.class, vol0);
-        Path chunksPath = Paths.get(
-            volume.getStorageDir().getPath(),
-            volume.getClusterID(),
-            Storage.STORAGE_DIR_CURRENT,
-            Storage.CONTAINER_DIR + "0",
-            String.valueOf(containerId),
-            OzoneConsts.STORAGE_DIR_CHUNKS
-        );
-        File[] chunkFiles = chunksPath.toFile().listFiles();
-        assertNotNull(chunkFiles);
-
-        try {
-          for (File chunkFile : chunkFiles) {
-            DatanodeTestUtils.injectDataFileFailure(chunkFile);
-          }
-
-          // simulate bad volume by removing write permission on root dir
-          // refer to HddsVolume.check()
-          DatanodeTestUtils.simulateBadVolume(vol0);
-
-          // read written file to trigger checkVolumeAsync
-          readKeyToTriggerCheckVolumeAsync(bucket, keyName);
-
-          // should trigger checkVolumeAsync and
-          // a failed volume should be detected
-          DatanodeTestUtils.waitForHandleFailedVolume(volSet, 1);
-        } finally {
-          // restore for cleanup
-          DatanodeTestUtils.restoreBadVolume(vol0);
-          for (File chunkFile : chunkFiles) {
-            DatanodeTestUtils.restoreDataFileFromFailure(chunkFile);
-          }
-        }
-      }
+  @Parameter
+  private boolean schemaV3;
+
+  private MiniOzoneCluster cluster;
+
+  @BeforeParameterizedClassInvocation
+  void initCluster() throws Exception {
+    cluster = newCluster(schemaV3);
+  }
+
+  @AfterEach
+  void restoreVolume() throws Exception {
+    MutableVolumeSet volumeSet = cluster.getHddsDatanodes().get(0)
+        .getDatanodeStateMachine().getContainer().getVolumeSet();
+    if (!volumeSet.getVolumesList().isEmpty()) {
+      return;
+    }
+    // A schema V3 volume failure closes the shared DB, which requires a
+    // datanode restart. Schema V2 uses per-container DBs, so restoring the
+    // volume to the active map is sufficient for the next test.
+    if (schemaV3) {
+      cluster.restartHddsDatanode(0, true);
+      return;
     }
+    StorageVolume volume = volumeSet.getFailedVolumesList().get(0);
+    volume.setState(VolumeState.NORMAL);
+    volume.start();
+    volumeSet.setVolumeMapForTesting(

Review Comment:
   Would restarting the V2 datanode also be a simpler way to avoid carrying this state into the next test? None of this breaks the current tests, but it is a contaminated state that future tests could trip over.
   
   If you prefer to keep the in-place restore for speed, a short comment documenting that the volume intentionally stays in `failedVolumeMap` (and why that is safe) would help the next reader.
   



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/volume/TestDatanodeHddsVolumeFailureDetection.java:
##########
@@ -240,64 +290,55 @@ void corruptDbFile(boolean schemaV3) throws Exception {
    * As a workaround, we test the helper method directly.
    * As we test the helper method directly, we cannot test for schemas older than V3.
    *
-   * @param schemaV3
    * @throws Exception

Review Comment:
   nit: the `@param schemaV3` was removed but the bare `@throws Exception` remains. This javadoc could drop it too.



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/volume/TestDatanodeHddsVolumeFailureDetection.java:
##########
@@ -319,6 +367,10 @@ private static MiniOzoneCluster newCluster(boolean schemaV3)
     // keep the cache size = 1, so we could trigger io exception on
     // reading on-disk db instance
     ozoneConfig.setInt(OZONE_CONTAINER_CACHE_SIZE, 1);
+    ozoneConfig.setTimeDuration(HDDS_HEARTBEAT_INTERVAL, 100, TimeUnit.MILLISECONDS);
+    ozoneConfig.setTimeDuration(OZONE_SCM_HEARTBEAT_PROCESS_INTERVAL, 100, TimeUnit.MILLISECONDS);
+    ozoneConfig.setTimeDuration(OZONE_SCM_STALENODE_INTERVAL, 1, TimeUnit.SECONDS);
+    ozoneConfig.setTimeDuration(OZONE_SCM_DEADNODE_INTERVAL, 2, TimeUnit.SECONDS);

Review Comment:
   Are the 1s stale and 2s dead intervals needed for the restart flow? I wonder whether they could be tight on a busy CI runner. If slightly larger values (for example stale 3s / dead 6s) still keep the runtime win, they would leave more headroom.



-- 
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-10314. Speed up TestDatanodeHddsVolumeFailureDetection [ozone]

Posted by "echonesis (via GitHub)" <gi...@apache.org>.
echonesis commented on PR #10819:
URL: https://github.com/apache/ozone/pull/10819#issuecomment-5060690486

   Thanks @chihsuan for the review.
   
   I tested the 3s/6s intervals together with restarting the V2 datanode locally. The runtime increased to about 121 seconds, which removes most of the intended speedup.
   
   There are roughly six datanode restarts across the V2 and V3 invocations. Since `restartHddsDatanode` waits for SCM to stop considering the datanode healthy, increasing the stale interval from 1s to 3s raises the minimum stale wait from about 6 seconds total to about 18 seconds total.
   Restarting V2 also adds three restarts that the in-place restore previously avoided, plus their shutdown, startup, heartbeat, and pipeline recovery costs.
   
   To preserve the purpose of this change, I propose keeping the safer 3s/6s intervals but restoring V2 in place. I’ll add a comment explicitly documenting that the V2 volume intentionally remains in failedVolumeMap, that subsequent tests use the restored active volume, and that the cluster is closed after the parameterized invocation. V3 will continue to restart because its shared DB is closed after a volume failure.
   
   What do you think?


-- 
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-10314. Speed up TestDatanodeHddsVolumeFailureDetection [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #10819:
URL: https://github.com/apache/ozone/pull/10819


-- 
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-10314. Speed up TestDatanodeHddsVolumeFailureDetection [ozone]

Posted by "echonesis (via GitHub)" <gi...@apache.org>.
echonesis commented on code in PR #10819:
URL: https://github.com/apache/ozone/pull/10819#discussion_r3659097819


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/volume/TestDatanodeHddsVolumeFailureDetection.java:
##########
@@ -60,176 +67,220 @@
 import org.apache.hadoop.ozone.container.common.volume.HddsVolume;
 import org.apache.hadoop.ozone.container.common.volume.MutableVolumeSet;
 import org.apache.hadoop.ozone.container.common.volume.StorageVolume;
+import org.apache.hadoop.ozone.container.common.volume.StorageVolume.VolumeState;
 import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainerData;
 import org.apache.hadoop.ozone.container.ozoneimpl.OzoneContainer;
 import org.apache.hadoop.ozone.dn.DatanodeTestUtils;
-import org.junit.jupiter.params.ParameterizedTest;
+import org.apache.ozone.test.GenericTestUtils;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.api.parallel.Execution;
+import org.junit.jupiter.api.parallel.ExecutionMode;
+import org.junit.jupiter.params.AfterParameterizedClassInvocation;
+import org.junit.jupiter.params.BeforeParameterizedClassInvocation;
+import org.junit.jupiter.params.Parameter;
+import org.junit.jupiter.params.ParameterizedClass;
 import org.junit.jupiter.params.provider.ValueSource;
 
 /**
  * This class tests datanode can detect failed volumes.
  */
+@ParameterizedClass
+@ValueSource(booleans = {true, false})
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
+@Execution(ExecutionMode.SAME_THREAD)
 class TestDatanodeHddsVolumeFailureDetection {
   private static final int KEY_SIZE = 128;
 
-  @ParameterizedTest
-  @ValueSource(booleans = {true, false})
-  void corruptChunkFile(boolean schemaV3) throws Exception {
-    try (MiniOzoneCluster cluster = newCluster(schemaV3)) {
-      try (OzoneClient client = cluster.newClient()) {
-        OzoneBucket bucket = DataTestUtil.createVolumeAndBucket(client);
-
-        // write a file
-        String keyName = UUID.randomUUID().toString();
-        long containerId = createKey(bucket, keyName);
-
-        // corrupt chunk file by rename file->dir
-        HddsDatanodeService dn = cluster.getHddsDatanodes().get(0);
-        OzoneContainer oc = dn.getDatanodeStateMachine().getContainer();
-        MutableVolumeSet volSet = oc.getVolumeSet();
-        StorageVolume vol0 = volSet.getVolumesList().get(0);
-        HddsVolume volume = assertInstanceOf(HddsVolume.class, vol0);
-        Path chunksPath = Paths.get(
-            volume.getStorageDir().getPath(),
-            volume.getClusterID(),
-            Storage.STORAGE_DIR_CURRENT,
-            Storage.CONTAINER_DIR + "0",
-            String.valueOf(containerId),
-            OzoneConsts.STORAGE_DIR_CHUNKS
-        );
-        File[] chunkFiles = chunksPath.toFile().listFiles();
-        assertNotNull(chunkFiles);
-
-        try {
-          for (File chunkFile : chunkFiles) {
-            DatanodeTestUtils.injectDataFileFailure(chunkFile);
-          }
-
-          // simulate bad volume by removing write permission on root dir
-          // refer to HddsVolume.check()
-          DatanodeTestUtils.simulateBadVolume(vol0);
-
-          // read written file to trigger checkVolumeAsync
-          readKeyToTriggerCheckVolumeAsync(bucket, keyName);
-
-          // should trigger checkVolumeAsync and
-          // a failed volume should be detected
-          DatanodeTestUtils.waitForHandleFailedVolume(volSet, 1);
-        } finally {
-          // restore for cleanup
-          DatanodeTestUtils.restoreBadVolume(vol0);
-          for (File chunkFile : chunkFiles) {
-            DatanodeTestUtils.restoreDataFileFromFailure(chunkFile);
-          }
-        }
-      }
+  @Parameter
+  private boolean schemaV3;
+
+  private MiniOzoneCluster cluster;
+
+  @BeforeParameterizedClassInvocation
+  void initCluster() throws Exception {
+    cluster = newCluster(schemaV3);
+  }
+
+  @AfterEach
+  void restoreVolume() throws Exception {
+    MutableVolumeSet volumeSet = cluster.getHddsDatanodes().get(0)
+        .getDatanodeStateMachine().getContainer().getVolumeSet();
+    if (!volumeSet.getVolumesList().isEmpty()) {
+      return;
+    }
+    // A schema V3 volume failure closes the shared DB, which requires a datanode restart.
+    // Schema V2 uses per-container DBs, so reactivate the volume to avoid the restart cost.
+    // It intentionally remains in failedVolumeMap: later tests use the active map, the next
+    // failure replaces the entry for the same path, and this cluster is closed after this invocation.
+    if (schemaV3) {
+      cluster.restartHddsDatanode(0, true);

Review Comment:
   Thanks for pointing this out!
   The cluster now starts with four datanodes, and each test uses the datanode selected for its container. 
   After each test, we wait for SCM to observe that datanode’s failed volume, ensuring the next test uses a different datanode. 
   This removes the need to restart or restore datanodes between tests.



-- 
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-10314. Speed up TestDatanodeHddsVolumeFailureDetection [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #10819:
URL: https://github.com/apache/ozone/pull/10819#issuecomment-5107509847

   Thanks @echonesis for the patch, @chihsuan for the review.


-- 
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-10314. Speed up TestDatanodeHddsVolumeFailureDetection [ozone]

Posted by "echonesis (via GitHub)" <gi...@apache.org>.
echonesis commented on PR #10819:
URL: https://github.com/apache/ozone/pull/10819#issuecomment-5173779629

   Thanks @adoroszlai for the review and merge, @chihsuan for the review.


-- 
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-10314. Speed up TestDatanodeHddsVolumeFailureDetection [ozone]

Posted by "echonesis (via GitHub)" <gi...@apache.org>.
echonesis commented on code in PR #10819:
URL: https://github.com/apache/ozone/pull/10819#discussion_r3638845921


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/volume/TestDatanodeHddsVolumeFailureDetection.java:
##########
@@ -240,64 +290,55 @@ void corruptDbFile(boolean schemaV3) throws Exception {
    * As a workaround, we test the helper method directly.
    * As we test the helper method directly, we cannot test for schemas older than V3.
    *
-   * @param schemaV3
    * @throws Exception

Review Comment:
   Thanks for pointing this out!
   I’ll remove the bare `@throws Exception` tag as well.



-- 
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-10314. Speed up TestDatanodeHddsVolumeFailureDetection [ozone]

Posted by "echonesis (via GitHub)" <gi...@apache.org>.
echonesis commented on code in PR #10819:
URL: https://github.com/apache/ozone/pull/10819#discussion_r3638833664


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/volume/TestDatanodeHddsVolumeFailureDetection.java:
##########
@@ -319,6 +367,10 @@ private static MiniOzoneCluster newCluster(boolean schemaV3)
     // keep the cache size = 1, so we could trigger io exception on
     // reading on-disk db instance
     ozoneConfig.setInt(OZONE_CONTAINER_CACHE_SIZE, 1);
+    ozoneConfig.setTimeDuration(HDDS_HEARTBEAT_INTERVAL, 100, TimeUnit.MILLISECONDS);
+    ozoneConfig.setTimeDuration(OZONE_SCM_HEARTBEAT_PROCESS_INTERVAL, 100, TimeUnit.MILLISECONDS);
+    ozoneConfig.setTimeDuration(OZONE_SCM_STALENODE_INTERVAL, 1, TimeUnit.SECONDS);
+    ozoneConfig.setTimeDuration(OZONE_SCM_DEADNODE_INTERVAL, 2, TimeUnit.SECONDS);

Review Comment:
   Yes, the stale interval is used by `restartHddsDatanode`, which waits for SCM to stop considering the datanode healthy before starting it again.
   The dead interval was set alongside it to satisfy the required stale/dead relationship. I agree that `1s/2s` leaves little headroom on a busy CI runner.
   I’ll change them to `3s/6s`; this should still preserve the runtime improvement from reusing the cluster.



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/volume/TestDatanodeHddsVolumeFailureDetection.java:
##########
@@ -60,176 +67,219 @@
 import org.apache.hadoop.ozone.container.common.volume.HddsVolume;
 import org.apache.hadoop.ozone.container.common.volume.MutableVolumeSet;
 import org.apache.hadoop.ozone.container.common.volume.StorageVolume;
+import org.apache.hadoop.ozone.container.common.volume.StorageVolume.VolumeState;
 import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainerData;
 import org.apache.hadoop.ozone.container.ozoneimpl.OzoneContainer;
 import org.apache.hadoop.ozone.dn.DatanodeTestUtils;
-import org.junit.jupiter.params.ParameterizedTest;
+import org.apache.ozone.test.GenericTestUtils;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.api.parallel.Execution;
+import org.junit.jupiter.api.parallel.ExecutionMode;
+import org.junit.jupiter.params.AfterParameterizedClassInvocation;
+import org.junit.jupiter.params.BeforeParameterizedClassInvocation;
+import org.junit.jupiter.params.Parameter;
+import org.junit.jupiter.params.ParameterizedClass;
 import org.junit.jupiter.params.provider.ValueSource;
 
 /**
  * This class tests datanode can detect failed volumes.
  */
+@ParameterizedClass
+@ValueSource(booleans = {true, false})
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
+@Execution(ExecutionMode.SAME_THREAD)
 class TestDatanodeHddsVolumeFailureDetection {
   private static final int KEY_SIZE = 128;
 
-  @ParameterizedTest
-  @ValueSource(booleans = {true, false})
-  void corruptChunkFile(boolean schemaV3) throws Exception {
-    try (MiniOzoneCluster cluster = newCluster(schemaV3)) {
-      try (OzoneClient client = cluster.newClient()) {
-        OzoneBucket bucket = TestDataUtil.createVolumeAndBucket(client);
-
-        // write a file
-        String keyName = UUID.randomUUID().toString();
-        long containerId = createKey(bucket, keyName);
-
-        // corrupt chunk file by rename file->dir
-        HddsDatanodeService dn = cluster.getHddsDatanodes().get(0);
-        OzoneContainer oc = dn.getDatanodeStateMachine().getContainer();
-        MutableVolumeSet volSet = oc.getVolumeSet();
-        StorageVolume vol0 = volSet.getVolumesList().get(0);
-        HddsVolume volume = assertInstanceOf(HddsVolume.class, vol0);
-        Path chunksPath = Paths.get(
-            volume.getStorageDir().getPath(),
-            volume.getClusterID(),
-            Storage.STORAGE_DIR_CURRENT,
-            Storage.CONTAINER_DIR + "0",
-            String.valueOf(containerId),
-            OzoneConsts.STORAGE_DIR_CHUNKS
-        );
-        File[] chunkFiles = chunksPath.toFile().listFiles();
-        assertNotNull(chunkFiles);
-
-        try {
-          for (File chunkFile : chunkFiles) {
-            DatanodeTestUtils.injectDataFileFailure(chunkFile);
-          }
-
-          // simulate bad volume by removing write permission on root dir
-          // refer to HddsVolume.check()
-          DatanodeTestUtils.simulateBadVolume(vol0);
-
-          // read written file to trigger checkVolumeAsync
-          readKeyToTriggerCheckVolumeAsync(bucket, keyName);
-
-          // should trigger checkVolumeAsync and
-          // a failed volume should be detected
-          DatanodeTestUtils.waitForHandleFailedVolume(volSet, 1);
-        } finally {
-          // restore for cleanup
-          DatanodeTestUtils.restoreBadVolume(vol0);
-          for (File chunkFile : chunkFiles) {
-            DatanodeTestUtils.restoreDataFileFromFailure(chunkFile);
-          }
-        }
-      }
+  @Parameter
+  private boolean schemaV3;
+
+  private MiniOzoneCluster cluster;
+
+  @BeforeParameterizedClassInvocation
+  void initCluster() throws Exception {
+    cluster = newCluster(schemaV3);
+  }
+
+  @AfterEach
+  void restoreVolume() throws Exception {
+    MutableVolumeSet volumeSet = cluster.getHddsDatanodes().get(0)
+        .getDatanodeStateMachine().getContainer().getVolumeSet();
+    if (!volumeSet.getVolumesList().isEmpty()) {
+      return;
+    }
+    // A schema V3 volume failure closes the shared DB, which requires a
+    // datanode restart. Schema V2 uses per-container DBs, so restoring the
+    // volume to the active map is sufficient for the next test.
+    if (schemaV3) {
+      cluster.restartHddsDatanode(0, true);
+      return;
     }
+    StorageVolume volume = volumeSet.getFailedVolumesList().get(0);
+    volume.setState(VolumeState.NORMAL);
+    volume.start();
+    volumeSet.setVolumeMapForTesting(

Review Comment:
   The in-place restore was originally intended as a test optimization.  Unlike schema V3, schema V2 uses per-container DBs, so restoring the volume to the active map was sufficient for the existing tests and avoided a datanode restart.
   However, I agree that leaving it in failedVolumeMap creates an unnecessarily contaminated state. 
   Restarting the V2 datanode is cleaner and still preserves the main benefit of reusing the cluster.
   I’ll update `restoreVolume()` to restart the datanode for both schema versions and remove the in-place V2 restoration logic.



-- 
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-10314. Speed up TestDatanodeHddsVolumeFailureDetection [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #10819:
URL: https://github.com/apache/ozone/pull/10819#discussion_r3657487967


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/volume/TestDatanodeHddsVolumeFailureDetection.java:
##########
@@ -60,176 +67,220 @@
 import org.apache.hadoop.ozone.container.common.volume.HddsVolume;
 import org.apache.hadoop.ozone.container.common.volume.MutableVolumeSet;
 import org.apache.hadoop.ozone.container.common.volume.StorageVolume;
+import org.apache.hadoop.ozone.container.common.volume.StorageVolume.VolumeState;
 import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainerData;
 import org.apache.hadoop.ozone.container.ozoneimpl.OzoneContainer;
 import org.apache.hadoop.ozone.dn.DatanodeTestUtils;
-import org.junit.jupiter.params.ParameterizedTest;
+import org.apache.ozone.test.GenericTestUtils;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.api.parallel.Execution;
+import org.junit.jupiter.api.parallel.ExecutionMode;
+import org.junit.jupiter.params.AfterParameterizedClassInvocation;
+import org.junit.jupiter.params.BeforeParameterizedClassInvocation;
+import org.junit.jupiter.params.Parameter;
+import org.junit.jupiter.params.ParameterizedClass;
 import org.junit.jupiter.params.provider.ValueSource;
 
 /**
  * This class tests datanode can detect failed volumes.
  */
+@ParameterizedClass
+@ValueSource(booleans = {true, false})
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
+@Execution(ExecutionMode.SAME_THREAD)
 class TestDatanodeHddsVolumeFailureDetection {
   private static final int KEY_SIZE = 128;
 
-  @ParameterizedTest
-  @ValueSource(booleans = {true, false})
-  void corruptChunkFile(boolean schemaV3) throws Exception {
-    try (MiniOzoneCluster cluster = newCluster(schemaV3)) {
-      try (OzoneClient client = cluster.newClient()) {
-        OzoneBucket bucket = DataTestUtil.createVolumeAndBucket(client);
-
-        // write a file
-        String keyName = UUID.randomUUID().toString();
-        long containerId = createKey(bucket, keyName);
-
-        // corrupt chunk file by rename file->dir
-        HddsDatanodeService dn = cluster.getHddsDatanodes().get(0);
-        OzoneContainer oc = dn.getDatanodeStateMachine().getContainer();
-        MutableVolumeSet volSet = oc.getVolumeSet();
-        StorageVolume vol0 = volSet.getVolumesList().get(0);
-        HddsVolume volume = assertInstanceOf(HddsVolume.class, vol0);
-        Path chunksPath = Paths.get(
-            volume.getStorageDir().getPath(),
-            volume.getClusterID(),
-            Storage.STORAGE_DIR_CURRENT,
-            Storage.CONTAINER_DIR + "0",
-            String.valueOf(containerId),
-            OzoneConsts.STORAGE_DIR_CHUNKS
-        );
-        File[] chunkFiles = chunksPath.toFile().listFiles();
-        assertNotNull(chunkFiles);
-
-        try {
-          for (File chunkFile : chunkFiles) {
-            DatanodeTestUtils.injectDataFileFailure(chunkFile);
-          }
-
-          // simulate bad volume by removing write permission on root dir
-          // refer to HddsVolume.check()
-          DatanodeTestUtils.simulateBadVolume(vol0);
-
-          // read written file to trigger checkVolumeAsync
-          readKeyToTriggerCheckVolumeAsync(bucket, keyName);
-
-          // should trigger checkVolumeAsync and
-          // a failed volume should be detected
-          DatanodeTestUtils.waitForHandleFailedVolume(volSet, 1);
-        } finally {
-          // restore for cleanup
-          DatanodeTestUtils.restoreBadVolume(vol0);
-          for (File chunkFile : chunkFiles) {
-            DatanodeTestUtils.restoreDataFileFromFailure(chunkFile);
-          }
-        }
-      }
+  @Parameter
+  private boolean schemaV3;
+
+  private MiniOzoneCluster cluster;
+
+  @BeforeParameterizedClassInvocation
+  void initCluster() throws Exception {
+    cluster = newCluster(schemaV3);
+  }
+
+  @AfterEach
+  void restoreVolume() throws Exception {
+    MutableVolumeSet volumeSet = cluster.getHddsDatanodes().get(0)
+        .getDatanodeStateMachine().getContainer().getVolumeSet();
+    if (!volumeSet.getVolumesList().isEmpty()) {
+      return;
+    }
+    // A schema V3 volume failure closes the shared DB, which requires a datanode restart.
+    // Schema V2 uses per-container DBs, so reactivate the volume to avoid the restart cost.
+    // It intentionally remains in failedVolumeMap: later tests use the active map, the next
+    // failure replaces the entry for the same path, and this cluster is closed after this invocation.
+    if (schemaV3) {
+      cluster.restartHddsDatanode(0, true);

Review Comment:
   Can we avoid datanode restart by starting the cluster with more datanodes and using a different one for each test case?



-- 
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