goiri commented on code in PR #6583:
URL: https://github.com/apache/hadoop/pull/6583#discussion_r1501301861
##########
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<Integer> 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);
Review Comment:
OK, I see this pattern already exists.
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestBootstrapStandby.java:
##########
@@ -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()) {
Review Comment:
This is unbounded. Can you use LambdaTestUtils#wait or similar?
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestBootstrapStandby.java:
##########
@@ -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());
Review Comment:
Easier to read if you extract the `nn = cluster.getNameNode(i);`
##########
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<Integer> txids) {
+
+ for (File nameDir : getNameNodeCurrentDirs(cluster, nnIdx)) {
+ LOG.info("examining name dir with files: " +
Review Comment:
Can you use the logger format?
##########
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<Integer> 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);
Review Comment:
An assert in the regular code? Should this be an exception?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
