Alexey Serbin has posted comments on this change. Change subject: tablet copy: Allow voting from crashed initial tablet copies ......................................................................
Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/7961/3/src/kudu/integration-tests/tablet_copy-itest.cc File src/kudu/integration-tests/tablet_copy-itest.cc: PS3, Line 1155: ASSERT_EVENTUALLY By default, this times out in 30 seconds (which is exactly kTimeout for this test). Would it make sense to set some custom timeout while awaiting for eventual success (i.e. multiple of kTimeout)? PS3, Line 1161: ASSERT_OK What it itest::AddServer() returns other than IllegalState, not-the-leader status? Is there a risk to overlook some other bug in that case if using such this sort of 'blanket' retry? PS3, Line 1187: LOG(INFO) << "Restarting tablet servers. New replica TS UUID: " << new_replica_uuid : << ", tablet data state: " : << tablet::TabletDataState_Name(superblock.tablet_data_state()) : << ", last-logged opid: " << superblock.tombstone_last_logged_opid(); nit: would it make sense to move this after ASSERT_OPID_EQ() and before ASSERT_OK(cluster_->master()->Restart()) ? http://gerrit.cloudera.org:8080/#/c/7961/3/src/kudu/tserver/tablet_copy_client.cc File src/kudu/tserver/tablet_copy_client.cc: PS3, Line 401: RETURN_NOT_OK(meta_->ReplaceSuperBlock(*superblock_)); If not clearing the tombstone_last_logged_opid field in the superblock, would tombstone_last_logged_opid_ be still consistent after call of the DeleteTabletData() below? Or it's not relevant here? -- To view, visit http://gerrit.cloudera.org:8080/7961 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idbc9cecac701c24ebe05759adbbb1fbe2b140506 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
