Mike Percy has posted comments on this change. Change subject: KUDU-921. tablet copy: Make the StartTabletCopy() RPC async ......................................................................
Patch Set 12: (5 comments) http://gerrit.cloudera.org:8080/#/c/5045/12/src/kudu/integration-tests/tablet_copy_client_session-itest.cc File src/kudu/integration-tests/tablet_copy_client_session-itest.cc: PS12, Line 145: kBlank, : kTombstoned, : k > a description of these two scenarios would be useful. In paricular I'm not Done PS12, Line 150: // Initialize state. : TabletDataState delete_type = TabletDataState::TABLET_DATA_UNKNOWN; : switch (scenario) { : case kBlank: : break; : case kTombstoned: : delete_type = TabletDataState::TABLET_DATA_TOMBSTONED; : break; : default: : LOG(FATAL) << "Unsupported Scenario"; : } : if (delete_type != TabletDataState::TABLET_DATA_UNKNOWN) { : NO_FATALS(DeleteTabletWithRetries(ts1, tablet_id, delete_type, boost::none, kTimeout)); : } > instead of all of this, couldn't you just do: Yeah, I had originally intended to put more scenarios in there. I'll simplify this. http://gerrit.cloudera.org:8080/#/c/5045/12/src/kudu/tserver/ts_tablet_manager.cc File src/kudu/tserver/ts_tablet_manager.cc: Line 323: Status _s = (expr); /* NOLINT */ \ > why not do 'const Status& _s' here? then lint wouldn't complain, right? oh... good idea :) Line 373: virtual void Cancel() { > hrm, is this overriding a Runnable method? surprised to see it virtual but Removed virtual and renamed to DisableCallback() Line 820: // TODO(mpercy): Cancel all outstanding tablet copy tasks. > hrm, is this an important todo? sounds important, so maybe there should be Filed KUDU-1795. -- To view, visit http://gerrit.cloudera.org:8080/5045 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I95c63f2bfd67624844447862efbdba9cb3676112 Gerrit-PatchSet: 12 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: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
