Mike Percy has posted comments on this change. Change subject: KUDU-921. tablet copy: Make the StartTabletCopy() RPC async ......................................................................
Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/5045/2/src/kudu/tserver/ts_tablet_manager.cc File src/kudu/tserver/ts_tablet_manager.cc: Line 320: > spurious \n ? Done PS2, Line 436: success_callback > I am curious why we needed a different callback for success ? One callback I reworked this Line 438: this->LogAndTombstone( > the old comment on line 450-452 seems to be not respected here -- if we fai Reworked this and fixed this issue PS2, Line 444: open_tablet_pool_ > Interesting, just thinking out loud here about possible outcomes of using t Moved to its own thread pool in order to have a zero-size queue on the pool. PS2, Line 446: TOMBSTONE_NOT_OK > There is nothing to tombstone at this point right ? We could as well leave It's a moot point now but StartAsync() could have failed if the thread pool was stopped or full, so we should tombstone in that case. http://gerrit.cloudera.org:8080/#/c/5045/2/src/kudu/tserver/ts_tablet_manager.h File src/kudu/tserver/ts_tablet_manager.h: PS2, Line 328: tablet_copy_sessions_ > Good idea, we could eventually connect this to metrics or tools to show the That is a good idea. I ended up peeling this out because it made this patch more complicated. I plan to put this back in in a more lightweight way, just to allow for cancellation and status notification, as well as metrics. However, I don't think those things are as important as this basic functionality or some other things I want to work on. -- 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: 2 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-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
