YARN-11463. Node Labels root directory creation doesn't have a retry logic - 2nd addendum (#5670)

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/store/AbstractFSNodeStore.java

@@ -65,6 +65,12 @@ public abstract class AbstractFSNodeStore<M> {
     this.fsWorkingPath = fsStorePath;
     this.manager = mgr;
     initFileSystem(conf);
+    initNodeStoreRootDirectory(conf);
+    this.replication = conf.getInt(YarnConfiguration.FS_STORE_FILE_REPLICATION,
+        YarnConfiguration.DEFAULT_FS_STORE_FILE_REPLICATION);
+  }
+
+  private void initNodeStoreRootDirectory(Configuration conf) throws IOException {
     // mkdir of root dir path with retry logic
     int maxRetries = conf.getInt(YarnConfiguration.NODE_STORE_ROOT_DIR_NUM_RETRIES,
         YarnConfiguration.NODE_STORE_ROOT_DIR_NUM_DEFAULT_RETRIES);
@@ -73,11 +79,7 @@ public abstract class AbstractFSNodeStore<M> {
 
     while (!success && retryCount <= maxRetries) {
       try {
-        if (!fs.exists(fsWorkingPath)) {
-          success = fs.mkdirs(fsWorkingPath);
-        } else {
-          success = true;
-        }
+        success = fs.mkdirs(fsWorkingPath);
       } catch (IOException e) {
         retryCount++;
         if (retryCount > maxRetries) {
@@ -91,8 +93,6 @@ public abstract class AbstractFSNodeStore<M> {
         }
       }
     }
-    this.replication = conf.getInt(YarnConfiguration.FS_STORE_FILE_REPLICATION,
-        YarnConfiguration.DEFAULT_FS_STORE_FILE_REPLICATION);
     LOG.info("Created store directory :" + fsWorkingPath);
   }
 

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestFileSystemNodeLabelsStore.java

@@ -24,6 +24,7 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.Map;
 
+import org.apache.hadoop.hdfs.server.namenode.SafeModeException;
 import org.apache.hadoop.thirdparty.com.google.common.collect.ImmutableMap;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Timeout;
@@ -348,27 +349,13 @@ public class TestFileSystemNodeLabelsStore extends NodeLabelTestBase {
 
   @MethodSource("getParameters")
   @ParameterizedTest
-  void testRootMkdirOnInitStoreWhenRootDirectoryAlreadyExists(String className) throws Exception {
-    initTestFileSystemNodeLabelsStore(className);
-    final FileSystem mockFs = Mockito.mock(FileSystem.class);
-    final FileSystemNodeLabelsStore mockStore = createMockNodeLabelsStore(mockFs);
-    final int expectedMkdirsCount = 0;
-
-    Mockito.when(mockStore.getFs().exists(Mockito.any(Path.class)))
-        .thenReturn(true);
-    verifyMkdirsCount(mockStore, expectedMkdirsCount);
-  }
-
-  @MethodSource("getParameters")
-  @ParameterizedTest
-  void testRootMkdirOnInitStoreWhenRootDirectoryNotExists(String className) throws Exception {
+  void testRootMkdirOnInitStore(String className) throws Exception {
     initTestFileSystemNodeLabelsStore(className);
     final FileSystem mockFs = Mockito.mock(FileSystem.class);
     final FileSystemNodeLabelsStore mockStore = createMockNodeLabelsStore(mockFs);
     final int expectedMkdirsCount = 1;
 
-    Mockito.when(mockStore.getFs().exists(Mockito.any(Path.class)))
-        .thenReturn(false).thenReturn(true);
+    Mockito.when(mockStore.getFs().mkdirs(Mockito.any(Path.class))).thenReturn(true);
     verifyMkdirsCount(mockStore, expectedMkdirsCount);
   }
 
@@ -378,10 +365,11 @@ public class TestFileSystemNodeLabelsStore extends NodeLabelTestBase {
     initTestFileSystemNodeLabelsStore(className);
     final FileSystem mockFs = Mockito.mock(FileSystem.class);
     final FileSystemNodeLabelsStore mockStore = createMockNodeLabelsStore(mockFs);
-    final int expectedMkdirsCount = 2;
+    final int expectedMkdirsCount = 3;
 
-    Mockito.when(mockStore.getFs().exists(Mockito.any(Path.class)))
-        .thenReturn(false).thenReturn(false).thenReturn(true);
+    Mockito.when(mockStore.getFs().mkdirs(Mockito.any(Path.class)))
+        .thenThrow(SafeModeException.class).thenThrow(SafeModeException.class)
+        .thenReturn(true);
     verifyMkdirsCount(mockStore, expectedMkdirsCount);
   }