[hadoop] branch trunk updated: YARN-11463. Node Labels root directory creation doesn't have a retry logic - addendum (#5614)

[bteke]

This is an automated email from the ASF dual-hosted git repository.

bteke pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/hadoop.git


The following commit(s) were added to refs/heads/trunk by this push:
     new bd607951c02 YARN-11463. Node Labels root directory creation doesn't 
have a retry logic - addendum (#5614)
bd607951c02 is described below

commit bd607951c02f17d1b443cfa3f5690385db67da08
Author: Peter Szucs <116345192+p-sz...@users.noreply.github.com>
AuthorDate: Thu May 4 12:27:25 2023 +0200

    YARN-11463. Node Labels root directory creation doesn't have a retry logic 
- addendum (#5614)
---
 .../yarn/nodelabels/store/AbstractFSNodeStore.java |  6 +--
 .../nodelabels/TestFileSystemNodeLabelsStore.java  | 44 +++++++++++++++++++---
 2 files changed, 41 insertions(+), 9 deletions(-)

diff --git 
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/store/AbstractFSNodeStore.java
 
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/store/AbstractFSNodeStore.java
index a697be19512..6e47b2afb08 100644
--- 
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/store/AbstractFSNodeStore.java
+++ 
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/store/AbstractFSNodeStore.java
@@ -69,9 +69,9 @@ public abstract class AbstractFSNodeStore<M> {
     int maxRetries = 
conf.getInt(YarnConfiguration.NODE_STORE_ROOT_DIR_NUM_RETRIES,
         YarnConfiguration.NODE_STORE_ROOT_DIR_NUM_DEFAULT_RETRIES);
     int retryCount = 0;
-    boolean success = fs.mkdirs(fsWorkingPath);
+    boolean success = false;
 
-    while (!success && retryCount < maxRetries) {
+    while (!success && retryCount <= maxRetries) {
       try {
         if (!fs.exists(fsWorkingPath)) {
           success = fs.mkdirs(fsWorkingPath);
@@ -80,7 +80,7 @@ public abstract class AbstractFSNodeStore<M> {
         }
       } catch (IOException e) {
         retryCount++;
-        if (retryCount >= maxRetries) {
+        if (retryCount > maxRetries) {
           throw e;
         }
         try {
diff --git 
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestFileSystemNodeLabelsStore.java
 
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestFileSystemNodeLabelsStore.java
index a861b0654ea..517958367d8 100644
--- 
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestFileSystemNodeLabelsStore.java
+++ 
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestFileSystemNodeLabelsStore.java
@@ -348,24 +348,56 @@ public class TestFileSystemNodeLabelsStore extends 
NodeLabelTestBase {
 
   @MethodSource("getParameters")
   @ParameterizedTest
-  void testRootMkdirOnInitStore(String className) throws Exception {
+  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 {
+    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);
+    verifyMkdirsCount(mockStore, expectedMkdirsCount);
+  }
+
+  @MethodSource("getParameters")
+  @ParameterizedTest
+  void testRootMkdirOnInitStoreRetryLogic(String className) throws Exception {
+    initTestFileSystemNodeLabelsStore(className);
+    final FileSystem mockFs = Mockito.mock(FileSystem.class);
+    final FileSystemNodeLabelsStore mockStore = 
createMockNodeLabelsStore(mockFs);
+    final int expectedMkdirsCount = 2;
+
+    Mockito.when(mockStore.getFs().exists(Mockito.any(Path.class)))
+        .thenReturn(false).thenReturn(false).thenReturn(true);
+    verifyMkdirsCount(mockStore, expectedMkdirsCount);
+  }
+
+  private FileSystemNodeLabelsStore createMockNodeLabelsStore(FileSystem 
mockFs) {
     FileSystemNodeLabelsStore mockStore = new FileSystemNodeLabelsStore() {
       public void initFileSystem(Configuration config) throws IOException {
         setFs(mockFs);
       }
     };
-
     mockStore.setFs(mockFs);
-    verifyMkdirsCount(mockStore, true, 1);
+    return mockStore;
   }
 
   private void verifyMkdirsCount(FileSystemNodeLabelsStore store,
-      boolean existsRetVal, int expectedNumOfCalls)
+      int expectedNumOfCalls)
       throws Exception {
-    Mockito.when(store.getFs().exists(Mockito.any(
-        Path.class))).thenReturn(existsRetVal);
     store.init(conf, mgr);
     Mockito.verify(store.getFs(), Mockito.times(
         expectedNumOfCalls)).mkdirs(Mockito.any(Path


---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-commits-h...@hadoop.apache.org