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