[
https://issues.apache.org/jira/browse/HDFS-17927?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18084514#comment-18084514
]
ASF GitHub Bot commented on HDFS-17927:
---------------------------------------
boky01 commented on code in PR #8517:
URL: https://github.com/apache/hadoop/pull/8517#discussion_r3328468281
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDiskError.java:
##########
@@ -94,47 +92,21 @@ public void tearDown() throws Exception {
* Test to check that a DN goes down when all its volumes have failed.
*/
@Test
+ @Timeout(value = 60)
public void testShutdown() throws Exception {
- if (System.getProperty("os.name").startsWith("Windows")) {
- /**
- * This test depends on OS not allowing file creations on a directory
- * that does not have write permissions for the user. Apparently it is
- * not the case on Windows (at least under Cygwin), and possibly AIX.
- * This is disabled on Windows.
- */
- return;
- }
- // Bring up two more datanodes
- cluster.startDataNodes(conf, 2, true, null, null);
- cluster.waitActive();
+ assumeNotWindows();
final int dnIndex = 0;
- String bpid = cluster.getNamesystem().getBlockPoolId();
- File storageDir = cluster.getInstanceStorageDir(dnIndex, 0);
- File dir1 = MiniDFSCluster.getRbwDir(storageDir, bpid);
- storageDir = cluster.getInstanceStorageDir(dnIndex, 1);
- File dir2 = MiniDFSCluster.getRbwDir(storageDir, bpid);
+ final File dir1 = cluster.getInstanceStorageDir(dnIndex, 0);
+ final File dir2 = cluster.getInstanceStorageDir(dnIndex, 1);
+ final DataNode dn = cluster.getDataNodes().get(dnIndex);
try {
- // make the data directory of the first datanode to be readonly
- assertTrue(dir1.setReadOnly(), "Couldn't chmod local vol");
- assertTrue(dir2.setReadOnly(), "Couldn't chmod local vol");
-
- // create files and make sure that first datanode will be down
- DataNode dn = cluster.getDataNodes().get(dnIndex);
- long deadline = Time.monotonicNow() + 60000;
- for (int i=0; dn.isDatanodeUp(); i++) {
- if (Time.monotonicNow() > deadline) {
- fail("DataNode stayed UP for 60s despite Disk Errors.");
- }
- Path fileName = new Path("/test.txt"+i);
- DFSTestUtil.createFile(fs, fileName, 1024, (short)2, 1L);
- DFSTestUtil.waitReplication(fs, fileName, (short)2);
- fs.delete(fileName, true);
- Thread.sleep(200);
- }
+ DataNodeTestUtils.injectDataDirFailure(dir1, dir2);
+ dn.checkDiskError();
+ GenericTestUtils.waitFor(() -> !dn.isDatanodeUp(), 100, 30000);
+ assertFalse(dn.isDatanodeUp(),
Review Comment:
Is it possible to reach this line?
I mean if dn.isDatanodeUp() is true, then GenericTestUtils.waitFor(() will
throw an exception, so if GenericTestUtils.waitFor() did not throw exception
that means the DN is down.
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDiskError.java:
##########
@@ -94,47 +92,21 @@ public void tearDown() throws Exception {
* Test to check that a DN goes down when all its volumes have failed.
*/
@Test
+ @Timeout(value = 60)
public void testShutdown() throws Exception {
- if (System.getProperty("os.name").startsWith("Windows")) {
- /**
- * This test depends on OS not allowing file creations on a directory
- * that does not have write permissions for the user. Apparently it is
- * not the case on Windows (at least under Cygwin), and possibly AIX.
- * This is disabled on Windows.
- */
- return;
- }
- // Bring up two more datanodes
- cluster.startDataNodes(conf, 2, true, null, null);
Review Comment:
Are we sure the 2 additional DNs are not required neither in this test case
nor to keep the following test cases stable (in the setup phase we are creating
a single DN cluster)?
> Make TestDiskError.testShutdown use deterministic volume failure injection
> --------------------------------------------------------------------------
>
> Key: HDFS-17927
> URL: https://issues.apache.org/jira/browse/HDFS-17927
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: test
> Reporter: Siyao Meng
> Assignee: Siyao Meng
> Priority: Major
> Labels: pull-request-available
>
> `TestDiskError.testShutdown()` currently drives DataNode shutdown by
> repeatedly creating files until `dn.isDatanodeUp()` becomes false. If
> shutdown does not happen promptly, the test can loop for a long time and
> generate excessive logs.
> Replaces file-create loop with deterministic failure injection for both
> DataNode data dirs, then explicitly runs `dn.checkDiskError()` and waits for
> the DataNode to exit.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
