Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18732 )
Change subject: [Tools] Copy tablets from a remote server in batch ...................................................................... Patch Set 7: (5 comments) http://gerrit.cloudera.org:8080/#/c/18732/7/src/kudu/tablet/tablet_replica.h File src/kudu/tablet/tablet_replica.h: http://gerrit.cloudera.org:8080/#/c/18732/7/src/kudu/tablet/tablet_replica.h@111 PS7, Line 111: TabletReplica(); Why change this? http://gerrit.cloudera.org:8080/#/c/18732/7/src/kudu/tools/tool_action_local_replica.cc File src/kudu/tools/tool_action_local_replica.cc: http://gerrit.cloudera.org:8080/#/c/18732/7/src/kudu/tools/tool_action_local_replica.cc@272 PS7, Line 272: switch (copy_type) { There is only two types, and seems impossible to add the 3rd one, so I think use if..else.. is OK. if (copy_type == TabletCopyClient::FROM_REMOTE) { ... } else { CHECK_EQ(copy_type, TabletCopyClient::FROM_LOCAL); ... } http://gerrit.cloudera.org:8080/#/c/18732/7/src/kudu/tools/tool_action_local_replica.cc@281 PS7, Line 281: nullptr fake_replica can be used to check the copy progess, you can use it as in local mode. Then only client.Start have different styles, FetchAll and Finish are the same, the code can be reused too. http://gerrit.cloudera.org:8080/#/c/18732/7/src/kudu/tools/tool_action_local_replica.cc@565 PS7, Line 565: FLAGS_num_threads nit: this paramter can be omitted, and you can use this flag in the new added function. http://gerrit.cloudera.org:8080/#/c/18732/7/src/kudu/tserver/tablet_copy_client.h File src/kudu/tserver/tablet_copy_client.h: http://gerrit.cloudera.org:8080/#/c/18732/7/src/kudu/tserver/tablet_copy_client.h@135 PS7, Line 135: enum CopyType { You can define it in tool_action_local_replica.cc only, avoid to take effect on other modules. -- To view, visit http://gerrit.cloudera.org:8080/18732 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664 Gerrit-Change-Number: 18732 Gerrit-PatchSet: 7 Gerrit-Owner: Wang Xixu <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Fri, 22 Jul 2022 10:09:51 +0000 Gerrit-HasComments: Yes
