HDFS-14654. RBF: TestRouterRpc#testNamenodeMetrics is flaky. Contributed by Chen Zhang.

hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/MockResolver.java

@@ -58,6 +58,7 @@ public class MockResolver
   private Set<FederationNamespaceInfo> namespaces = new HashSet<>();
   private String defaultNamespace = null;
   private boolean disableDefaultNamespace = false;
+  private volatile boolean disableRegistration = false;
 
   public MockResolver() {
     this.cleanRegistrations();
@@ -98,6 +99,15 @@ public class MockResolver
     this.namespaces = new HashSet<>();
   }
 
+  /*
+   * Disable NameNode auto registration for test. This method usually used after
+   * {@link MockResolver#cleanRegistrations()}, and before {@link
+   * MockResolver#registerNamenode()}
+   */
+  public void setDisableRegistration(boolean isDisable) {
+    disableRegistration = isDisable;
+  }
+
   @Override
   public void updateActiveNamenode(
       String nsId, InetSocketAddress successfulAddress) {
@@ -125,7 +135,7 @@ public class MockResolver
   }
 
   @Override
-  public List<? extends FederationNamenodeContext>
+  public synchronized List<? extends FederationNamenodeContext>
       getNamenodesForNameserviceId(String nameserviceId) {
     // Return a copy of the list because it is updated periodically
     List<? extends FederationNamenodeContext> namenodes =
@@ -137,8 +147,8 @@ public class MockResolver
   }
 
   @Override
-  public List<? extends FederationNamenodeContext> getNamenodesForBlockPoolId(
-      String blockPoolId) {
+  public synchronized List<? extends FederationNamenodeContext>
+      getNamenodesForBlockPoolId(String blockPoolId) {
     // Return a copy of the list because it is updated periodically
     List<? extends FederationNamenodeContext> namenodes =
         this.resolver.get(blockPoolId);
@@ -226,6 +236,9 @@ public class MockResolver
   @Override
   public synchronized boolean registerNamenode(NamenodeStatusReport report)
       throws IOException {
+    if (disableRegistration) {
+      return false;
+    }
 
     MockNamenodeContext context = new MockNamenodeContext(
         report.getRpcAddress(), report.getServiceAddress(),
@@ -263,8 +276,9 @@ public class MockResolver
   }
 
   @Override
-  public Set<FederationNamespaceInfo> getNamespaces() throws IOException {
-    return this.namespaces;
+  public synchronized Set<FederationNamespaceInfo> getNamespaces()
+      throws IOException {
+    return Collections.unmodifiableSet(this.namespaces);
   }
 
   @Override

hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRpc.java

@@ -1575,6 +1575,13 @@ public class TestRouterRpc {
         .setSafeMode(HdfsConstants.SafeModeAction.SAFEMODE_LEAVE);
   }
 
+  /*
+   * This case is used to test NameNodeMetrics on 2 purposes:
+   * 1. NameNodeMetrics should be cached, since the cost of gathering the
+   * metrics is expensive
+   * 2. Metrics cache should updated regularly
+   * 3. Without any subcluster available, we should return an empty list
+   */
   @Test
   public void testNamenodeMetrics() throws Exception {
     final NamenodeBeanMetrics metrics =
@@ -1606,6 +1613,8 @@ public class TestRouterRpc {
     MockResolver resolver =
         (MockResolver) router.getRouter().getNamenodeResolver();
     resolver.cleanRegistrations();
+    resolver.setDisableRegistration(true);
+    try {
       GenericTestUtils.waitFor(new Supplier<Boolean>() {
         @Override
         public Boolean get() {
@@ -1613,11 +1622,13 @@ public class TestRouterRpc {
         }
       }, 500, 5 * 1000);
       assertEquals("{}", metrics.getLiveNodes());
-
+    } finally {
       // Reset the registrations again
+      resolver.setDisableRegistration(false);
       cluster.registerNamenodes();
       cluster.waitNamenodeRegistration();
     }
+  }
 
   @Test
   public void testRBFMetricsMethodsRelayOnStateStore() {