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: [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]
