[incubator-uniffle] branch master updated: Fix Flaky Test: AppBalanceSelectStorageStrategyTest#selectStorageTest (#450)

[roryqi]

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

roryqi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-uniffle.git


The following commit(s) were added to refs/heads/master by this push:
     new 3ef80ed0 Fix Flaky Test: 
AppBalanceSelectStorageStrategyTest#selectStorageTest (#450)
3ef80ed0 is described below

commit 3ef80ed06c170f59a016c5440ac33ef97d8b9d45
Author: jokercurry <84573424+smallzhongf...@users.noreply.github.com>
AuthorDate: Thu Dec 29 09:54:22 2022 +0800

    Fix Flaky Test: AppBalanceSelectStorageStrategyTest#selectStorageTest (#450)
    
    ### What changes were proposed in this pull request?
    When testing, close the scheduler thread detectStorageScheduler.
    
    ### Why are the changes needed?
    When testing, in order to ensure that multiple paths are normal, the method 
of manually scheduling `detectStorage` is used.
    ```
    
applicationManager.getRemoteStoragePathRankValue().get(remotePath1).getCostTime().set(0);
    
applicationManager.getRemoteStoragePathRankValue().get(remotePath2).getCostTime().set(0);
    ```
    The original repair may still fail to write the hdfs file later.
    
    ### Does this PR introduce _any_ user-facing change?
    No.
    
    ### How was this patch tested?
    origin uts.
---
 .../java/org/apache/uniffle/coordinator/ApplicationManager.java  | 9 ++++++++-
 .../strategy/storage/AppBalanceSelectStorageStrategyTest.java    | 4 ++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git 
a/coordinator/src/main/java/org/apache/uniffle/coordinator/ApplicationManager.java
 
b/coordinator/src/main/java/org/apache/uniffle/coordinator/ApplicationManager.java
index ecc78c0d..5315c8ff 100644
--- 
a/coordinator/src/main/java/org/apache/uniffle/coordinator/ApplicationManager.java
+++ 
b/coordinator/src/main/java/org/apache/uniffle/coordinator/ApplicationManager.java
@@ -61,6 +61,7 @@ public class ApplicationManager {
   private final Map<String, RankValue> remoteStoragePathRankValue;
   private final Map<String, String> remoteStorageToHost = 
Maps.newConcurrentMap();
   private final Map<String, RemoteStorageInfo> availableRemoteStorageInfo;
+  private final ScheduledExecutorService detectStorageScheduler;
   private Map<String, Map<String, Long>> currentUserAndApp = 
Maps.newConcurrentMap();
   private Map<String, String> appIdToUser = Maps.newConcurrentMap();
   private QuotaManager quotaManager;
@@ -97,7 +98,7 @@ public class ApplicationManager {
     scheduledExecutorService.scheduleAtFixedRate(
         this::statusCheck, expired / 2, expired / 2, TimeUnit.MILLISECONDS);
     // the thread for checking if the storage is normal
-    ScheduledExecutorService detectStorageScheduler = 
Executors.newSingleThreadScheduledExecutor(
+    detectStorageScheduler = Executors.newSingleThreadScheduledExecutor(
         ThreadUtils.getThreadFactory("detectStoragesScheduler-%d"));
     // should init later than the refreshRemoteStorage init
     
detectStorageScheduler.scheduleAtFixedRate(selectStorageStrategy::detectStorage,
 1000,
@@ -255,6 +256,12 @@ public class ApplicationManager {
     return hasErrorInStatusCheck;
   }
 
+  @VisibleForTesting
+  public void closeDetectStorageScheduler() {
+    // this method can only be used during testing
+    detectStorageScheduler.shutdownNow();
+  }
+
   private void statusCheck() {
     List<Map<String, Long>> appAndNums = 
Lists.newArrayList(currentUserAndApp.values());
     Map<String, Long> appIds = Maps.newHashMap();
diff --git 
a/coordinator/src/test/java/org/apache/uniffle/coordinator/strategy/storage/AppBalanceSelectStorageStrategyTest.java
 
b/coordinator/src/test/java/org/apache/uniffle/coordinator/strategy/storage/AppBalanceSelectStorageStrategyTest.java
index c92c8d88..a3955104 100644
--- 
a/coordinator/src/test/java/org/apache/uniffle/coordinator/strategy/storage/AppBalanceSelectStorageStrategyTest.java
+++ 
b/coordinator/src/test/java/org/apache/uniffle/coordinator/strategy/storage/AppBalanceSelectStorageStrategyTest.java
@@ -58,12 +58,15 @@ public class AppBalanceSelectStorageStrategyTest {
     conf.set(CoordinatorConf.COORDINATOR_REMOTE_STORAGE_SELECT_STRATEGY, 
APP_BALANCE);
     conf.setLong(CoordinatorConf.COORDINATOR_REMOTE_STORAGE_SCHEDULE_TIME, 
1000);
     applicationManager = new ApplicationManager(conf);
+    // to ensure that the reading and writing of hdfs can be controlled
+    applicationManager.closeDetectStorageScheduler();
   }
 
   @Test
   public void selectStorageTest() throws Exception {
     String remoteStoragePath = remotePath1 + Constants.COMMA_SPLIT_CHAR + 
remotePath2;
     applicationManager.refreshRemoteStorage(remoteStoragePath, "");
+    applicationManager.getSelectStorageStrategy().detectStorage();
     assertEquals(0, 
applicationManager.getRemoteStoragePathRankValue().get(remotePath1).getAppNum().get());
     assertEquals(0, 
applicationManager.getRemoteStoragePathRankValue().get(remotePath2).getAppNum().get());
     String storageHost1 = "path1";
@@ -123,6 +126,7 @@ public class AppBalanceSelectStorageStrategyTest {
     String remoteStoragePath = remotePath1 + Constants.COMMA_SPLIT_CHAR + 
remotePath2
         + Constants.COMMA_SPLIT_CHAR + remotePath3;
     applicationManager.refreshRemoteStorage(remoteStoragePath, "");
+    applicationManager.getSelectStorageStrategy().detectStorage();
     String testApp1 = "application_testAppId";
     // init detectStorageScheduler
     Thread.sleep(2000);