From 4edd305353315f3f826949307a71888b1c8fc92f Mon Sep 17 00:00:00 2001 From: Danny Becker Date: Fri, 23 Feb 2024 15:59:17 -0800 Subject: [PATCH 1/9] BootstrapStandby should download the rollback fsimage --- .../router/RouterNamenodeProtocol.java | 12 +++++++++ .../federation/router/RouterRpcServer.java | 7 +++++ ...amenodeProtocolServerSideTranslatorPB.java | 17 ++++++++++++ .../NamenodeProtocolTranslatorPB.java | 13 +++++++++ .../hadoop/hdfs/server/namenode/FSImage.java | 21 +++++++++++++++ .../server/namenode/NameNodeRpcServer.java | 8 ++++++ .../server/namenode/ha/BootstrapStandby.java | 22 +++++++++++++-- .../server/protocol/NamenodeProtocol.java | 7 +++++ .../src/main/proto/NamenodeProtocol.proto | 14 ++++++++++ .../hdfs/server/namenode/FSImageTestUtil.java | 17 ++++++++++++ .../namenode/ha/TestBootstrapStandby.java | 27 +++++++++++++++++-- 11 files changed, 161 insertions(+), 4 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterNamenodeProtocol.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterNamenodeProtocol.java index a5a047d115cd2..2c7482eefbebd 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterNamenodeProtocol.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterNamenodeProtocol.java @@ -26,6 +26,7 @@ import org.apache.hadoop.hdfs.protocol.HdfsConstants.DatanodeReportType; import org.apache.hadoop.hdfs.security.token.block.ExportedBlockKeys; import org.apache.hadoop.hdfs.server.namenode.CheckpointSignature; +import org.apache.hadoop.hdfs.server.namenode.NNStorage; import org.apache.hadoop.hdfs.server.namenode.NameNode.OperationCategory; import org.apache.hadoop.hdfs.server.protocol.BlocksWithLocations; import org.apache.hadoop.hdfs.server.protocol.DatanodeStorageReport; @@ -114,6 +115,17 @@ public long getMostRecentCheckpointTxId() throws IOException { return rpcServer.invokeAtAvailableNs(method, long.class); } + @Override + public long getMostRecentNameNodeFileTxId(NNStorage.NameNodeFile nnf) + throws IOException { + rpcServer.checkOperation(OperationCategory.READ); + + RemoteMethod method = + new RemoteMethod(NamenodeProtocol.class, "getMostRecentNameNodeFileTxId", + new Class[] {NNStorage.NameNodeFile.class}, nnf); + return rpcServer.invokeAtAvailableNs(method, long.class); + } + @Override public CheckpointSignature rollEditLog() throws IOException { rpcServer.checkOperation(OperationCategory.WRITE, false); diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcServer.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcServer.java index 29aa16ff041e7..ddba5fdba3520 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcServer.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcServer.java @@ -145,6 +145,7 @@ import org.apache.hadoop.hdfs.server.namenode.CheckpointSignature; import org.apache.hadoop.hdfs.server.namenode.LeaseExpiredException; import org.apache.hadoop.hdfs.server.namenode.NameNode.OperationCategory; +import org.apache.hadoop.hdfs.server.namenode.NNStorage; import org.apache.hadoop.hdfs.server.namenode.NotReplicatedYetException; import org.apache.hadoop.hdfs.server.namenode.SafeModeException; import org.apache.hadoop.hdfs.server.protocol.BlocksWithLocations; @@ -1640,6 +1641,12 @@ public long getMostRecentCheckpointTxId() throws IOException { return nnProto.getMostRecentCheckpointTxId(); } + @Override // NamenodeProtocol + public long getMostRecentNameNodeFileTxId(NNStorage.NameNodeFile nnf) + throws IOException { + return nnProto.getMostRecentNameNodeFileTxId(nnf); + } + @Override // NamenodeProtocol public CheckpointSignature rollEditLog() throws IOException { return nnProto.rollEditLog(); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/NamenodeProtocolServerSideTranslatorPB.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/NamenodeProtocolServerSideTranslatorPB.java index f4025366391c1..da2440dfa722a 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/NamenodeProtocolServerSideTranslatorPB.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/NamenodeProtocolServerSideTranslatorPB.java @@ -35,6 +35,8 @@ import org.apache.hadoop.hdfs.protocol.proto.NamenodeProtocolProtos.GetEditLogManifestResponseProto; import org.apache.hadoop.hdfs.protocol.proto.NamenodeProtocolProtos.GetMostRecentCheckpointTxIdRequestProto; import org.apache.hadoop.hdfs.protocol.proto.NamenodeProtocolProtos.GetMostRecentCheckpointTxIdResponseProto; +import org.apache.hadoop.hdfs.protocol.proto.NamenodeProtocolProtos.GetMostRecentNameNodeFileTxIdRequestProto; +import org.apache.hadoop.hdfs.protocol.proto.NamenodeProtocolProtos.GetMostRecentNameNodeFileTxIdResponseProto; import org.apache.hadoop.hdfs.protocol.proto.NamenodeProtocolProtos.GetNextSPSPathRequestProto; import org.apache.hadoop.hdfs.protocol.proto.NamenodeProtocolProtos.GetNextSPSPathResponseProto; import org.apache.hadoop.hdfs.protocol.proto.NamenodeProtocolProtos.GetTransactionIdRequestProto; @@ -51,6 +53,7 @@ import org.apache.hadoop.hdfs.protocol.proto.NamenodeProtocolProtos.StartCheckpointResponseProto; import org.apache.hadoop.hdfs.security.token.block.ExportedBlockKeys; import org.apache.hadoop.hdfs.server.namenode.CheckpointSignature; +import org.apache.hadoop.hdfs.server.namenode.NNStorage; import org.apache.hadoop.hdfs.server.protocol.BlocksWithLocations; import org.apache.hadoop.hdfs.server.protocol.NamenodeCommand; import org.apache.hadoop.hdfs.server.protocol.NamenodeProtocol; @@ -141,6 +144,20 @@ public GetMostRecentCheckpointTxIdResponseProto getMostRecentCheckpointTxId( return GetMostRecentCheckpointTxIdResponseProto.newBuilder().setTxId(txid).build(); } + @Override + public GetMostRecentNameNodeFileTxIdResponseProto getMostRecentNameNodeFileTxId( + RpcController unused, GetMostRecentNameNodeFileTxIdRequestProto request) + throws ServiceException { + long txid; + try { + txid = impl.getMostRecentNameNodeFileTxId( + NNStorage.NameNodeFile.valueOf(request.getNameNodeFile())); + } catch (IOException e) { + throw new ServiceException(e); + } + return GetMostRecentNameNodeFileTxIdResponseProto.newBuilder().setTxId(txid).build(); + } + @Override public RollEditLogResponseProto rollEditLog(RpcController unused, diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/NamenodeProtocolTranslatorPB.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/NamenodeProtocolTranslatorPB.java index 87518aa1e231a..13840ba6f1ec8 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/NamenodeProtocolTranslatorPB.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/NamenodeProtocolTranslatorPB.java @@ -34,6 +34,7 @@ import org.apache.hadoop.hdfs.protocol.proto.NamenodeProtocolProtos.GetBlocksRequestProto; import org.apache.hadoop.hdfs.protocol.proto.NamenodeProtocolProtos.GetEditLogManifestRequestProto; import org.apache.hadoop.hdfs.protocol.proto.NamenodeProtocolProtos.GetMostRecentCheckpointTxIdRequestProto; +import org.apache.hadoop.hdfs.protocol.proto.NamenodeProtocolProtos.GetMostRecentNameNodeFileTxIdRequestProto; import org.apache.hadoop.hdfs.protocol.proto.NamenodeProtocolProtos.GetNextSPSPathRequestProto; import org.apache.hadoop.hdfs.protocol.proto.NamenodeProtocolProtos.GetNextSPSPathResponseProto; import org.apache.hadoop.hdfs.protocol.proto.NamenodeProtocolProtos.GetTransactionIdRequestProto; @@ -46,6 +47,7 @@ import org.apache.hadoop.hdfs.protocol.proto.NamenodeProtocolProtos.StartCheckpointRequestProto; import org.apache.hadoop.hdfs.security.token.block.ExportedBlockKeys; import org.apache.hadoop.hdfs.server.namenode.CheckpointSignature; +import org.apache.hadoop.hdfs.server.namenode.NNStorage; import org.apache.hadoop.hdfs.server.protocol.BlocksWithLocations; import org.apache.hadoop.hdfs.server.protocol.NamenodeCommand; import org.apache.hadoop.hdfs.server.protocol.NamenodeProtocol; @@ -134,6 +136,17 @@ public long getMostRecentCheckpointTxId() throws IOException { GetMostRecentCheckpointTxIdRequestProto.getDefaultInstance()).getTxId()); } + @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); + } + } + @Override public CheckpointSignature rollEditLog() throws IOException { return PBHelper.convert(ipc(() -> rpcProxy.rollEditLog(NULL_CONTROLLER, diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImage.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImage.java index 12f79e8c52e1d..00b5f811c4515 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImage.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImage.java @@ -1562,4 +1562,25 @@ public void updateLastAppliedTxIdFromWritten() { public long getMostRecentCheckpointTxId() { return storage.getMostRecentCheckpointTxId(); } + + /** + * @return the latest txid for the NameNodeFile type, or + * {@link HdfsServerConstants::INVALID_TXID}if there is no FSImage file of the + * type requested. + */ + public long getMostRecentNameNodeFileTxId(NameNodeFile nnf) throws IOException { + final FSImageStorageInspector inspector = + new FSImageTransactionalStorageInspector(EnumSet.of(nnf)); + storage.inspectStorageDirs(inspector); + try { + List images = inspector.getLatestImages(); + if (images != null && !images.isEmpty()) { + return images.get(0).getCheckpointTxId(); + } else { + return HdfsServerConstants.INVALID_TXID; + } + } catch (FileNotFoundException e) { + return HdfsServerConstants.INVALID_TXID; + } + } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java index 3a8357f4bddf4..6a7b8406929c9 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java @@ -1361,6 +1361,14 @@ public long getMostRecentCheckpointTxId() throws IOException { namesystem.checkSuperuserPrivilege(operationName); return namesystem.getFSImage().getMostRecentCheckpointTxId(); } + + @Override // NamenodeProtocol + public long getMostRecentNameNodeFileTxId(NNStorage.NameNodeFile nnf) throws IOException { + checkNNStartup(); + namesystem.checkOperation(OperationCategory.UNCHECKED); + namesystem.checkSuperuserPrivilege(); + return namesystem.getFSImage().getMostRecentNameNodeFileTxId(nnf); + } @Override // NamenodeProtocol public CheckpointSignature rollEditLog() throws IOException { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/BootstrapStandby.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/BootstrapStandby.java index 97af6f6509a47..1401b6f35bc15 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/BootstrapStandby.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/BootstrapStandby.java @@ -248,7 +248,7 @@ private int doRun() throws IOException { } // download the fsimage from active namenode - int download = downloadImage(storage, proxy, proxyInfo); + int download = downloadImage(storage, proxy, proxyInfo, isRollingUpgrade); if (download != 0) { return download; } @@ -351,12 +351,30 @@ private void doUpgrade(NNStorage storage) throws IOException { } } - private int downloadImage(NNStorage storage, NamenodeProtocol proxy, RemoteNameNodeInfo proxyInfo) + private int downloadImage(NNStorage storage, NamenodeProtocol proxy, RemoteNameNodeInfo proxyInfo, + boolean isRollingUpgrade) throws IOException { // Load the newly formatted image, using all of the directories // (including shared edits) final long imageTxId = proxy.getMostRecentCheckpointTxId(); final long curTxId = proxy.getTransactionID(); + final long rollbackTxId = + proxy.getMostRecentNameNodeFileTxId(NameNodeFile.IMAGE_ROLLBACK); + + if (rollbackTxId != HdfsServerConstants.INVALID_TXID || isRollingUpgrade) { + FSImage rollbackImage = new FSImage(conf); + try { + rollbackImage.getStorage().setStorageInfo(storage); + MD5Hash hash = TransferFsImage.downloadImageToStorage( + proxyInfo.getHttpAddress(), rollbackTxId, storage, true, true); + rollbackImage.saveDigestAndRenameCheckpointImage( + NameNodeFile.IMAGE_ROLLBACK, rollbackTxId, hash); + } catch (IOException ioe) { + throw ioe; + } finally { + rollbackImage.close(); + } + } FSImage image = new FSImage(conf); try { image.getStorage().setStorageInfo(storage); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/NamenodeProtocol.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/NamenodeProtocol.java index 03ddc5ef8b1e9..9eaa40ce67b21 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/NamenodeProtocol.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/NamenodeProtocol.java @@ -26,6 +26,7 @@ import org.apache.hadoop.hdfs.protocol.DatanodeInfo; import org.apache.hadoop.hdfs.security.token.block.ExportedBlockKeys; import org.apache.hadoop.hdfs.server.namenode.CheckpointSignature; +import org.apache.hadoop.hdfs.server.namenode.NNStorage; import org.apache.hadoop.hdfs.server.namenode.ha.ReadOnly; import org.apache.hadoop.io.retry.AtMostOnce; import org.apache.hadoop.io.retry.Idempotent; @@ -111,6 +112,12 @@ BlocksWithLocations getBlocks(DatanodeInfo datanode, long size, long @Idempotent public long getMostRecentCheckpointTxId() throws IOException; + /** + * Get the transaction ID of the most recent checkpoint for the given NameNodeFile. + */ + @Idempotent + public long getMostRecentNameNodeFileTxId(NNStorage.NameNodeFile nnf) throws IOException; + /** * Closes the current edit log and opens a new one. The * call fails if the file system is in SafeMode. diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/NamenodeProtocol.proto b/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/NamenodeProtocol.proto index 29a9aa01b68d4..edcbb69f495d1 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/NamenodeProtocol.proto +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/NamenodeProtocol.proto @@ -108,6 +108,14 @@ message GetMostRecentCheckpointTxIdResponseProto{ required uint64 txId = 1; } +message GetMostRecentNameNodeFileTxIdRequestProto { + required string nameNodeFile = 1; +} + +message GetMostRecentNameNodeFileTxIdResponseProto{ + required uint64 txId = 1; +} + /** * registration - Namenode reporting the error * errorCode - error code indicating the error @@ -253,6 +261,12 @@ service NamenodeProtocolService { rpc getMostRecentCheckpointTxId(GetMostRecentCheckpointTxIdRequestProto) returns(GetMostRecentCheckpointTxIdResponseProto); + /** + * Get the transaction ID of the NameNodeFile + */ + rpc getMostRecentNameNodeFileTxId(GetMostRecentNameNodeFileTxIdRequestProto) + returns(GetMostRecentNameNodeFileTxIdResponseProto); + /** * Close the current editlog and open a new one for checkpointing purposes */ diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSImageTestUtil.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSImageTestUtil.java index 0c660bb7f11f0..d03ce79a5d49c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSImageTestUtil.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSImageTestUtil.java @@ -519,6 +519,23 @@ public static void assertNNHasCheckpoints(MiniDFSCluster cluster, } } + public static void assertNNHasRollbackCheckpoints(MiniDFSCluster cluster, + int nnIdx, List txids) { + + for (File nameDir : getNameNodeCurrentDirs(cluster, nnIdx)) { + 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(), ", ")); + for (long checkpointTxId : txids) { + File image = new File(nameDir, + NNStorage.getRollbackImageFileName(checkpointTxId)); + assertTrue("Expected non-empty " + image, image.length() > 0); + } + } + } + public static List getNameNodeCurrentDirs(MiniDFSCluster cluster, int nnIdx) { List nameDirs = Lists.newArrayList(); for (URI u : cluster.getNameDirs(nnIdx)) { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestBootstrapStandby.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestBootstrapStandby.java index f470a90f374c9..77023b439f7d4 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestBootstrapStandby.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestBootstrapStandby.java @@ -36,6 +36,7 @@ import java.util.function.Supplier; import org.apache.hadoop.hdfs.DistributedFileSystem; import org.apache.hadoop.hdfs.protocol.HdfsConstants.RollingUpgradeAction; +import org.apache.hadoop.hdfs.protocol.RollingUpgradeInfo; import org.apache.hadoop.hdfs.server.common.HttpGetFailedException; import org.apache.hadoop.hdfs.server.namenode.FSImage; import org.apache.hadoop.hdfs.server.namenode.NameNodeLayoutVersion; @@ -189,7 +190,8 @@ public void testDownloadingLaterCheckpoint() throws Exception { */ @Test public void testRollingUpgradeBootstrapStandby() throws Exception { - removeStandbyNameDirs(); + // This node is needed to create the rollback fsimage + cluster.restartNameNode(1); int futureVersion = NameNodeLayoutVersion.CURRENT_LAYOUT_VERSION - 1; @@ -208,12 +210,21 @@ public void testRollingUpgradeBootstrapStandby() throws Exception { // BootstrapStandby should fail if the node has a future version // and the cluster isn't in rolling upgrade - bs.setConf(cluster.getConfiguration(1)); + bs.setConf(cluster.getConfiguration(2)); assertEquals("BootstrapStandby should return ERR_CODE_INVALID_VERSION", ERR_CODE_INVALID_VERSION, bs.run(new String[]{"-force"})); // Start rolling upgrade fs.rollingUpgrade(RollingUpgradeAction.PREPARE); + RollingUpgradeInfo info = fs.rollingUpgrade(RollingUpgradeAction.QUERY); + while (!info.createdRollbackImages()) { + Thread.sleep(1000); + info = fs.rollingUpgrade(RollingUpgradeAction.QUERY); + } + // After the rollback image is created the standby is not needed + cluster.shutdownNameNode(1); + removeStandbyNameDirs(); + nn0 = spy(nn0); // Make nn0 think it is a future version @@ -237,6 +248,9 @@ public void testRollingUpgradeBootstrapStandby() throws Exception { long expectedCheckpointTxId = NameNodeAdapter.getNamesystem(nn0) .getFSImage().getMostRecentCheckpointTxId(); + long expectedRollbackTxId = NameNodeAdapter.getNamesystem(nn0) + .getFSImage().getMostRecentNameNodeFileTxId( + NNStorage.NameNodeFile.IMAGE_ROLLBACK); assertEquals(11, expectedCheckpointTxId); for (int i = 1; i < maxNNCount; i++) { @@ -245,6 +259,8 @@ public void testRollingUpgradeBootstrapStandby() throws Exception { bs.run(new String[]{"-force"}); FSImageTestUtil.assertNNHasCheckpoints(cluster, i, ImmutableList.of((int) expectedCheckpointTxId)); + FSImageTestUtil.assertNNHasRollbackCheckpoints(cluster, i, + ImmutableList.of((int) expectedRollbackTxId)); } // Make sure the bootstrap was successful @@ -253,6 +269,13 @@ public void testRollingUpgradeBootstrapStandby() throws Exception { // We should now be able to start the standby successfully restartNameNodesFromIndex(1, "-rollingUpgrade", "started"); + for (int i = 1; i < maxNNCount; i++) { + assertTrue("NameNodes should all have the rollback FSImage", + cluster.getNameNode(i).getFSImage().hasRollbackFSImage()); + assertTrue("NameNodes should all be inRollingUpgrade", + cluster.getNameNode(i).getNamesystem().isRollingUpgrade()); + } + // Cleanup standby dirs for (int i = 1; i < maxNNCount; i++) { cluster.shutdownNameNode(i); From 04a35a21c0727e4cd6e80fbd8f0956a3f284ea68 Mon Sep 17 00:00:00 2001 From: Danny Becker Date: Fri, 23 Feb 2024 16:11:08 -0800 Subject: [PATCH 2/9] Add check for invalid txid --- .../hadoop/hdfs/server/namenode/ha/BootstrapStandby.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/BootstrapStandby.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/BootstrapStandby.java index 1401b6f35bc15..abbc2f24e4f5f 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/BootstrapStandby.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/BootstrapStandby.java @@ -358,10 +358,12 @@ private int downloadImage(NNStorage storage, NamenodeProtocol proxy, RemoteNameN // (including shared edits) final long imageTxId = proxy.getMostRecentCheckpointTxId(); final long curTxId = proxy.getTransactionID(); - final long rollbackTxId = - proxy.getMostRecentNameNodeFileTxId(NameNodeFile.IMAGE_ROLLBACK); - if (rollbackTxId != HdfsServerConstants.INVALID_TXID || isRollingUpgrade) { + if (isRollingUpgrade) { + final long rollbackTxId = + proxy.getMostRecentNameNodeFileTxId(NameNodeFile.IMAGE_ROLLBACK); + assert rollbackTxId != HdfsServerConstants.INVALID_TXID : + "Expected a valid TXID for fsimage_rollback file"; FSImage rollbackImage = new FSImage(conf); try { rollbackImage.getStorage().setStorageInfo(storage); From d7f6943491645ec44fdca6b7fe9de2f643b1d645 Mon Sep 17 00:00:00 2001 From: Danny Becker Date: Mon, 26 Feb 2024 11:54:09 -0800 Subject: [PATCH 3/9] Address comments --- .../protocolPB/NamenodeProtocolTranslatorPB.java | 11 ++++------- .../hdfs/server/namenode/FSImageTestUtil.java | 6 +++--- .../server/namenode/ha/TestBootstrapStandby.java | 13 ++++++------- 3 files changed, 13 insertions(+), 17 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/NamenodeProtocolTranslatorPB.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/NamenodeProtocolTranslatorPB.java index 13840ba6f1ec8..a23d44c746d6a 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/NamenodeProtocolTranslatorPB.java +++ b/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 diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSImageTestUtil.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSImageTestUtil.java index d03ce79a5d49c..14283f76fd429 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSImageTestUtil.java +++ b/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 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)); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestBootstrapStandby.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestBootstrapStandby.java index 77023b439f7d4..35fcb99442408 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestBootstrapStandby.java +++ b/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 From ed0d884f3b6d63cf35ce826511c3637ffac3d5f9 Mon Sep 17 00:00:00 2001 From: Danny Becker Date: Mon, 26 Feb 2024 14:00:46 -0800 Subject: [PATCH 4/9] Add ipc() wrapper --- .../hadoop/hdfs/protocolPB/NamenodeProtocolTranslatorPB.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/NamenodeProtocolTranslatorPB.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/NamenodeProtocolTranslatorPB.java index a23d44c746d6a..40d0395721be4 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/NamenodeProtocolTranslatorPB.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/NamenodeProtocolTranslatorPB.java @@ -138,9 +138,9 @@ public long getMostRecentCheckpointTxId() throws IOException { @Override public long getMostRecentNameNodeFileTxId(NNStorage.NameNodeFile nnf) throws IOException { - return rpcProxy.getMostRecentNameNodeFileTxId(NULL_CONTROLLER, + return ipc(() -> rpcProxy.getMostRecentNameNodeFileTxId(NULL_CONTROLLER, GetMostRecentNameNodeFileTxIdRequestProto.newBuilder() - .setNameNodeFile(nnf.toString()).build()).getTxId(); + .setNameNodeFile(nnf.toString()).build()).getTxId()); } From bf017bf2b5696b8d843da9ed6a17b69ee9af57ea Mon Sep 17 00:00:00 2001 From: Danny Becker Date: Wed, 28 Feb 2024 09:57:22 -0800 Subject: [PATCH 5/9] Fix javadoc --- .../apache/hadoop/hdfs/server/namenode/FSImage.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImage.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImage.java index 00b5f811c4515..0d653ea08e0c3 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImage.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImage.java @@ -1564,11 +1564,17 @@ public long getMostRecentCheckpointTxId() { } /** + * Given a NameNodeFile type, retrieve the latest txid for that file or + * {@link HdfsServerConstants::INVALID_TXID} if the file does not exist + * + * @param nnf The NameNodeFile type to retrieve the latest txid from. * @return the latest txid for the NameNodeFile type, or - * {@link HdfsServerConstants::INVALID_TXID}if there is no FSImage file of the - * type requested. + * {@link HdfsServerConstants::INVALID_TXID} if there is no FSImage file of + * the type requested + * @throws IOException */ - public long getMostRecentNameNodeFileTxId(NameNodeFile nnf) throws IOException { + public long getMostRecentNameNodeFileTxId(NameNodeFile nnf) + throws IOException { final FSImageStorageInspector inspector = new FSImageTransactionalStorageInspector(EnumSet.of(nnf)); storage.inspectStorageDirs(inspector); From 2079744ad58257e7131e9c07a473774ee161f9da Mon Sep 17 00:00:00 2001 From: Danny Becker Date: Wed, 28 Feb 2024 10:00:05 -0800 Subject: [PATCH 6/9] Fix checkstyle --- .../hdfs/server/namenode/ha/TestBootstrapStandby.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestBootstrapStandby.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestBootstrapStandby.java index 35fcb99442408..ffcf72ad9ab07 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestBootstrapStandby.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestBootstrapStandby.java @@ -36,7 +36,6 @@ import java.util.function.Supplier; import org.apache.hadoop.hdfs.DistributedFileSystem; import org.apache.hadoop.hdfs.protocol.HdfsConstants.RollingUpgradeAction; -import org.apache.hadoop.hdfs.protocol.RollingUpgradeInfo; import org.apache.hadoop.hdfs.server.common.HttpGetFailedException; import org.apache.hadoop.hdfs.server.namenode.FSImage; import org.apache.hadoop.hdfs.server.namenode.NameNodeLayoutVersion; @@ -218,7 +217,7 @@ public void testRollingUpgradeBootstrapStandby() throws Exception { // Start rolling upgrade fs.rollingUpgrade(RollingUpgradeAction.PREPARE); LambdaTestUtils.await(60000, 1000, () -> - fs.rollingUpgrade(RollingUpgradeAction.QUERY).createdRollbackImages()); + fs.rollingUpgrade(RollingUpgradeAction.QUERY).createdRollbackImages()); // After the rollback image is created the standby is not needed cluster.shutdownNameNode(1); removeStandbyNameDirs(); @@ -247,8 +246,8 @@ public void testRollingUpgradeBootstrapStandby() throws Exception { long expectedCheckpointTxId = NameNodeAdapter.getNamesystem(nn0) .getFSImage().getMostRecentCheckpointTxId(); long expectedRollbackTxId = NameNodeAdapter.getNamesystem(nn0) - .getFSImage().getMostRecentNameNodeFileTxId( - NNStorage.NameNodeFile.IMAGE_ROLLBACK); + .getFSImage().getMostRecentNameNodeFileTxId( + NNStorage.NameNodeFile.IMAGE_ROLLBACK); assertEquals(11, expectedCheckpointTxId); for (int i = 1; i < maxNNCount; i++) { From 9db198c3ae8a67a561a57b9a629473a35a42196a Mon Sep 17 00:00:00 2001 From: Danny Becker Date: Fri, 1 Mar 2024 13:13:28 -0800 Subject: [PATCH 7/9] Address javadoc and checkstyle --- .../apache/hadoop/hdfs/server/namenode/FSImage.java | 10 +++++----- .../hadoop/hdfs/server/protocol/NamenodeProtocol.java | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImage.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImage.java index 0d653ea08e0c3..5e3e7e43a79b7 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImage.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImage.java @@ -1564,13 +1564,13 @@ public long getMostRecentCheckpointTxId() { } /** - * Given a NameNodeFile type, retrieve the latest txid for that file or - * {@link HdfsServerConstants::INVALID_TXID} if the file does not exist + * Given a NameNodeFile type, retrieve the latest txid for that file or {@link + * HdfsServerConstants::INVALID_TXID} if the file does not exist. * * @param nnf The NameNodeFile type to retrieve the latest txid from. - * @return the latest txid for the NameNodeFile type, or - * {@link HdfsServerConstants::INVALID_TXID} if there is no FSImage file of - * the type requested + * @return the latest txid for the NameNodeFile type, or {@link + * HdfsServerConstants::INVALID_TXID} if there is no FSImage file of the type + * requested. * @throws IOException */ public long getMostRecentNameNodeFileTxId(NameNodeFile nnf) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/NamenodeProtocol.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/NamenodeProtocol.java index 9eaa40ce67b21..b6a60919fb16b 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/NamenodeProtocol.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/NamenodeProtocol.java @@ -110,7 +110,7 @@ BlocksWithLocations getBlocks(DatanodeInfo datanode, long size, long * Get the transaction ID of the most recent checkpoint. */ @Idempotent - public long getMostRecentCheckpointTxId() throws IOException; + long getMostRecentCheckpointTxId() throws IOException; /** * Get the transaction ID of the most recent checkpoint for the given NameNodeFile. From 2cbf8b9f2a3e55b265b8a20edfa008778fae1d24 Mon Sep 17 00:00:00 2001 From: Danny Becker Date: Mon, 4 Mar 2024 16:47:03 -0800 Subject: [PATCH 8/9] Fix javadoc link reference typo --- .../java/org/apache/hadoop/hdfs/server/namenode/FSImage.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImage.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImage.java index 5e3e7e43a79b7..cc79568e1a43c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImage.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImage.java @@ -1565,11 +1565,11 @@ public long getMostRecentCheckpointTxId() { /** * Given a NameNodeFile type, retrieve the latest txid for that file or {@link - * HdfsServerConstants::INVALID_TXID} if the file does not exist. + * HdfsServerConstants#INVALID_TXID} if the file does not exist. * * @param nnf The NameNodeFile type to retrieve the latest txid from. * @return the latest txid for the NameNodeFile type, or {@link - * HdfsServerConstants::INVALID_TXID} if there is no FSImage file of the type + * HdfsServerConstants#INVALID_TXID} if there is no FSImage file of the type * requested. * @throws IOException */ From 3bcfed3fa4a42da4bac346702baed458bcb5b72c Mon Sep 17 00:00:00 2001 From: Danny Becker Date: Mon, 4 Mar 2024 16:48:05 -0800 Subject: [PATCH 9/9] Fix checkstyle --- .../apache/hadoop/hdfs/server/protocol/NamenodeProtocol.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/NamenodeProtocol.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/NamenodeProtocol.java index b6a60919fb16b..a93611d4a1223 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/NamenodeProtocol.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/NamenodeProtocol.java @@ -110,13 +110,13 @@ BlocksWithLocations getBlocks(DatanodeInfo datanode, long size, long * Get the transaction ID of the most recent checkpoint. */ @Idempotent - long getMostRecentCheckpointTxId() throws IOException; + public long getMostRecentCheckpointTxId() throws IOException; /** * Get the transaction ID of the most recent checkpoint for the given NameNodeFile. */ @Idempotent - public long getMostRecentNameNodeFileTxId(NNStorage.NameNodeFile nnf) throws IOException; + long getMostRecentNameNodeFileTxId(NNStorage.NameNodeFile nnf) throws IOException; /** * Closes the current edit log and opens a new one. The