joseluisll commented on code in PR #8717:
URL: https://github.com/apache/hadoop/pull/8717#discussion_r3947055078
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestStandbyCheckpoints.java:
##########
@@ -764,13 +764,29 @@ public void testLastCheckpointTime() throws Exception {
nns[0].getRpcServer().rollEditLog();
HATestUtil.waitForCheckpoint(cluster, 0, ImmutableList.of(23));
+ // The wait above only says the active holds the new image. Every standby
+ // builds its own checkpoint and any of them may be the one that uploaded
+ // it, and the one that did stamps its own lastCheckpointTime only once
+ // doCheckpoint() has returned. So nns[1] can still be reporting the
+ // previous checkpoint here, which reads as an interval of zero. Wait for
+ // its time to move before taking the pair. The active stamps its own
+ // while it receives the upload, inside that same doCheckpoint(), so by
+ // then it has necessarily moved too.
+ GenericTestUtils.waitFor(
+ () -> nns[1].getNamesystem().getStandbyLastCheckpointTime()
+ > snnCheckpointTime1, 100, 30000);
+
long snnCheckpointTime2 =
nns[1].getNamesystem().getStandbyLastCheckpointTime();
long annCheckpointTime2 = nns[0].getNamesystem().getLastCheckpointTime();
Review Comment:
Agreed, added. Confirmed it's saveDigestAndRenameCheckpointImage — the same
method the active runs on receiving the upload — renaming at :1481 and stamping
at :1486, so "has necessarily moved too" wasn't something I could claim.
--
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]
