Alexey Serbin has posted comments on this change. Change subject: consensus: Save previous last-logged OpId across tablet copies ......................................................................
Patch Set 2: (5 comments) Added some after-thought nits: they are about making sure my understanding was correct. http://gerrit.cloudera.org:8080/#/c/7888/2//COMMIT_MSG Commit Message: Line 7: consensus: Save previous last-logged OpId across tablet copies nit: as an afterthought, I it would be nice to add some more information on how the system behave prior to this patch in the particular case which is now exercised by the test. Is that something like 'IllegalState: must be running to vote when last-logged opid is not known' or it was something more cryptic? Would it make sense? http://gerrit.cloudera.org:8080/#/c/7888/2/src/kudu/integration-tests/raft_consensus-itest.cc File src/kudu/integration-tests/raft_consensus-itest.cc: PS2, Line 2846: TEST_F(RaftConsensusITest, TestTombstonedVoteAfterFailedTabletCopy) Just for my better understanding: did this test fail before the fix? PS2, Line 2849: vector<string> ts_flags = { : // We write 128KB cells in this test, so bump the limit. : "--max_cell_size_bytes=1000000" : }; nit: could you add a comment explaining why it's crucial to write above the default cell size limit? Line 2860: ASSERT_EQ(3, active_tablet_servers.size()); nit: Since the FLAGS_num_replicas might be altered in the command line, would it make sense to add: ASSERT_EQ(3, FLAGS_num_replicas); ? PS2, Line 2893: ASSERT_OK(cluster_->tablet_server_by_uuid(leader_uuid)->Restart()); nit (just wanted to make sure my understanding is correct): would it work if bringing back not the former leader, but the other replica (not the crashed follower, of course)? -- To view, visit http://gerrit.cloudera.org:8080/7888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie886764adbdc6d84406df44d6cdb693d038bcd57 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: David Ribeiro Alves <davidral...@gmail.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-HasComments: Yes
