Dinesh Bhat has posted comments on this change.

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


Patch Set 2:

(5 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 ?


PS2, Line 436: success_callback
I am curious why we needed a different callback for success ? One callback can 
have 
if (status == success ) 
  OpenTablet()
else 
  Tombstone()

?


PS2, Line 444: open_tablet_pool_
Interesting, just thinking out loud here about possible outcomes of using the 
same threadpool as that of OpenTablet(): Given that open_tablet_pool_ contains 
as many max threads as there are tablets to open during server bring up, tablet 
copy could contend with OpenTablet() threads if consensus for these tablets 
starts up in the context of OpenTablet(). i.e, leader could get to know about 
follower before OpenTablet thread finishes and starts a tablet copy session 
(because it is tombstoned or whatever reason) and now there is a possibility 
that there are no threads available to start async tablet copy ? Or am I just 
hallucinating ?


PS2, Line 446: TOMBSTONE_NOT_OK
There is nothing to tombstone at this point right ? We could as well leave the 
tablet as it is since it hasn't changed its state yet.


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 
number of tablets being tablet copied, right ?


-- 
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 <mpe...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <din...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to