joseluisll commented on PR #8659:
URL: https://github.com/apache/hadoop/pull/8659#issuecomment-5291334712

   Thanks for looking at this @Hexiaoqiao
   
   Checkstyle: 45 were real and are fixed. The other 70 are pre-existing 
violations on lines
   the patch moves.
   
   Unit test: TestStandbyCheckpoints#testLastCheckpointTime. I don't think it
   is this patch - the method is byte-identical to trunk, it passes locally on
   trunk, and precommit itself voted +1 unit on PR-8659/3 and -1 on /5 with no
   checkpoint code changed in between.
   
   Looking at it, the active-NN half of that assertion doesn't seem bounded by
   anything: annCheckpointTime1 is nn0's own checkpoint while it is still a
   standby, and annCheckpointTime2 is the image nn1 uploads after nn0 becomes
   active - two different NNs and triggers, so the 3 s gap isn't guaranteed.
   The standby half looks fine. 
   
   I'd rather not touch another test's logic here though.
   


-- 
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]

Reply via email to