Mike Percy has posted comments on this change.
Change subject: KUDU-921. tablet copy: Make the StartTabletCopy() RPC async
Patch Set 2:
> spurious \n ?
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.
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-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>