Dinesh Bhat has posted comments on this change. Change subject: KUDU-1785. Fix potential crash in TabletCopySourceSession ......................................................................
Patch Set 4: Code-Review+1 (4 comments) http://gerrit.cloudera.org:8080/#/c/5363/4/src/kudu/integration-tests/tablet_copy_client_session-itest.cc File src/kudu/integration-tests/tablet_copy_client_session-itest.cc: Line 44: int num_tablets); Nit: num_tablets defaults to 1 ? Line 46: const MonoDelta kTimeout = MonoDelta::FromSeconds(30); Nit: Do we want to move the timeout to individual tests since future tests may have different timeout requirements ? PS4, Line 121: WaitUntilTabletRunning Do we want to wait for tablet copy or just wait till we get a Status::IllegalState("Tablet is still bootstrapping") ? If it's the former we might want to update the comment L89 which says we expect simple failure out of this test. Or perhaps restarting the source tserver while some lambda threads continuously attempting StartTabletCopy might give us more predictable results ? PS4, Line 122: DeleteTablet Why is this necessary for testing this fix ? -- To view, visit http://gerrit.cloudera.org:8080/5363 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6f3ec35885dbf1a81c23ac10b1c9556dfddbd4b7 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dinesh Bhat <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
