Mike Percy has posted comments on this change.

Change subject: KUDU-921. tablet copy: Make the StartTabletCopy() RPC async
......................................................................


Patch Set 4:

(4 comments)

> If I'm understanding this correctly, the semantics of the
> StartTabletCopy RPC remain unchanged: remote callers will only
> receive a response when the copy is finished. Is that right?

No: callers will receive a response once the process is started, but will not 
be notified on completion. It would be good to add APIs to get for status / 
completion, as well as enable cancellation, but I think this is more important 
than those other features.

http://gerrit.cloudera.org:8080/#/c/5045/4/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:

PS4, Line 383: nullptr
> Shouldn't need this; the constructor is guaranteed to set it.
Done


PS4, Line 532:   cb = [](const Status&, TabletServerErrorPB::Code) {
             :     LOG(FATAL) << "StartTabletCopy() callback invoked twice";
             :   };
> Isn't 'cb' a local copy? How does this have an effect?
It would have an effect if someone accidentally used one of the macros that 
invokes it, like CALLBACK_RETURN_NOT_OK().


PS4, Line 540: copy_source_uuid
> this is causing a crash because it's a reference to a field in 'req', which
Ah, thanks.


PS4, Line 548: We run this asynchronously.
> No longer true.
Done


-- 
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: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mpe...@apache.org>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Dinesh Bhat <din...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to