Address comments

Author: Danny Becker

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/NamenodeProtocolTranslatorPB.java

@@ -138,13 +138,10 @@ public long getMostRecentCheckpointTxId() throws IOException {
 
   @Override
   public long getMostRecentNameNodeFileTxId(NNStorage.NameNodeFile nnf) throws IOException {
-    try {
-      return rpcProxy.getMostRecentNameNodeFileTxId(NULL_CONTROLLER,
-          GetMostRecentNameNodeFileTxIdRequestProto.newBuilder()
-              .setNameNodeFile(nnf.toString()).build()).getTxId();
-    } catch (ServiceException e) {
-      throw ProtobufHelper.getRemoteException(e);
-    }
+    return rpcProxy.getMostRecentNameNodeFileTxId(NULL_CONTROLLER,
+        GetMostRecentNameNodeFileTxIdRequestProto.newBuilder()
+            .setNameNodeFile(nnf.toString()).build()).getTxId();
+
   }
 
   @Override

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSImageTestUtil.java

@@ -523,11 +523,11 @@ public static void assertNNHasRollbackCheckpoints(MiniDFSCluster cluster,
       int nnIdx, List<Integer> txids) {
 
     for (File nameDir : getNameNodeCurrentDirs(cluster, nnIdx)) {
-      LOG.info("examining name dir with files: " +
+      LOG.info("examining name dir with files: {}",
                    Joiner.on(",").join(nameDir.listFiles()));
       // Should have fsimage_N for the three checkpoints
-      LOG.info("Examining storage dir " + nameDir + " with contents: "
-                   + StringUtils.join(nameDir.listFiles(), ", "));
+      LOG.info("Examining storage dir {} with contents: {}", nameDir,
+                   StringUtils.join(nameDir.listFiles(), ", "));
       for (long checkpointTxId : txids) {
         File image = new File(nameDir,
             NNStorage.getRollbackImageFileName(checkpointTxId));

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestBootstrapStandby.java

@@ -41,6 +41,7 @@
 import org.apache.hadoop.hdfs.server.namenode.FSImage;
 import org.apache.hadoop.hdfs.server.namenode.NameNodeLayoutVersion;
 import org.apache.hadoop.hdfs.server.protocol.NamespaceInfo;
+import org.apache.hadoop.test.LambdaTestUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.apache.hadoop.conf.Configuration;
@@ -216,11 +217,8 @@ public void testRollingUpgradeBootstrapStandby() throws Exception {
 
     // Start rolling upgrade
     fs.rollingUpgrade(RollingUpgradeAction.PREPARE);
-    RollingUpgradeInfo info = fs.rollingUpgrade(RollingUpgradeAction.QUERY);
-    while (!info.createdRollbackImages()) {
-      Thread.sleep(1000);
-      info = fs.rollingUpgrade(RollingUpgradeAction.QUERY);
-    }
+    LambdaTestUtils.await(60000, 1000, () ->
+       fs.rollingUpgrade(RollingUpgradeAction.QUERY).createdRollbackImages());
     // After the rollback image is created the standby is not needed
     cluster.shutdownNameNode(1);
     removeStandbyNameDirs();
@@ -270,10 +268,11 @@ public void testRollingUpgradeBootstrapStandby() throws Exception {
     restartNameNodesFromIndex(1, "-rollingUpgrade", "started");
 
     for (int i = 1; i < maxNNCount; i++) {
+      NameNode nn = cluster.getNameNode(i);
       assertTrue("NameNodes should all have the rollback FSImage",
-          cluster.getNameNode(i).getFSImage().hasRollbackFSImage());
+          nn.getFSImage().hasRollbackFSImage());
       assertTrue("NameNodes should all be inRollingUpgrade",
-          cluster.getNameNode(i).getNamesystem().isRollingUpgrade());
+          nn.getNamesystem().isRollingUpgrade());
     }
 
     // Cleanup standby dirs