You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Wang Xixu (Code Review)" <ge...@cloudera.org> on 2022/07/14 10:45:01 UTC
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Wang Xixu has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18732
Change subject: [Tools] Copy tablets from a remote server in batch
......................................................................
[Tools] Copy tablets from a remote server in batch
The command: kudu local_replica copy_from_remote only supports to copy one
tablet from a remote server once a time. It is not efficient when we need
to copy all tablets from a remote server.
Therefore this MR supports to copy tablets from a remote server in batch.
Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_local_replica.cc
2 files changed, 102 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/18732/1
--
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: newchange
Gerrit-Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
Gerrit-Change-Number: 18732
Gerrit-PatchSet: 1
Gerrit-Owner: Wang Xixu <14...@qq.com>
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18732
to look at the new patch set (#8).
Change subject: [Tools] Copy tablets from a remote server in batch
......................................................................
[Tools] Copy tablets from a remote server in batch
The command: kudu local_replica copy_from_remote only supports to copy one
tablet from a remote server once a time. It is not efficient when we need
to copy all tablets from a remote server.
Therefore this patch supports to copy tablets from a remote server in batch.
Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
---
M src/kudu/tablet/tablet_replica.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/tablet_copy_client.h
4 files changed, 185 insertions(+), 122 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/18732/8
--
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: newpatchset
Gerrit-Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
Gerrit-Change-Number: 18732
Gerrit-PatchSet: 8
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18732
to look at the new patch set (#6).
Change subject: [Tools] Copy tablets from a remote server in batch
......................................................................
[Tools] Copy tablets from a remote server in batch
The command: kudu local_replica copy_from_remote only supports to copy one
tablet from a remote server once a time. It is not efficient when we need
to copy all tablets from a remote server.
Therefore this patch supports to copy tablets from a remote server in batch.
Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_local_replica.cc
2 files changed, 106 insertions(+), 20 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/18732/6
--
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: newpatchset
Gerrit-Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
Gerrit-Change-Number: 18732
Gerrit-PatchSet: 6
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18732
to look at the new patch set (#15).
Change subject: [Tools] Copy tablets from a remote server in batch
......................................................................
[Tools] Copy tablets from a remote server in batch
The command: kudu local_replica copy_from_remote only supports to copy one
tablet from a remote server once a time. It is not efficient when we need
to copy all tablets from a remote server.
Therefore this patch supports to copy tablets from a remote server in batch.
Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
---
M src/kudu/tablet/tablet_replica.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_local_replica.cc
3 files changed, 220 insertions(+), 116 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/18732/15
--
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: newpatchset
Gerrit-Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
Gerrit-Change-Number: 18732
Gerrit-PatchSet: 15
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
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 <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 22 Jul 2022 10:09:51 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18732 )
Change subject: [Tools] Copy tablets from a remote server in batch
......................................................................
[Tools] Copy tablets from a remote server in batch
The command: kudu local_replica copy_from_remote only supports to copy one
tablet from a remote server once a time. It is not efficient when we need
to copy all tablets from a remote server.
Therefore this patch supports to copy tablets from a remote server in batch.
Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
Reviewed-on: http://gerrit.cloudera.org:8080/18732
Tested-by: Kudu Jenkins
Reviewed-by: Yingchun Lai <ac...@gmail.com>
---
M src/kudu/tablet/tablet_replica.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_local_replica.cc
3 files changed, 212 insertions(+), 115 deletions(-)
Approvals:
Kudu Jenkins: Verified
Yingchun Lai: Looks good to me, approved
--
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: merged
Gerrit-Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
Gerrit-Change-Number: 18732
Gerrit-PatchSet: 19
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu 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 10:
(1 comment)
> Patch Set 8:
>
> (1 comment)
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: // peers see the same updates in the same order. In addition to this, this
> not neccessary, you can declare it as a friend function.
Done
--
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: 10
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Mon, 25 Jul 2022 06:08:20 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
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 13:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/18732/13/src/kudu/tablet/tablet_replica.h
File src/kudu/tablet/tablet_replica.h:
http://gerrit.cloudera.org:8080/#/c/18732/13/src/kudu/tablet/tablet_replica.h@26
PS13, Line 26: #include <set>
nit: outdated, remove it
http://gerrit.cloudera.org:8080/#/c/18732/13/src/kudu/tablet/tablet_replica.h@87
PS13, Line 87: TabletCopyer
nit: TabletCopier
http://gerrit.cloudera.org:8080/#/c/18732/13/src/kudu/tools/tablet_copyer.h
File src/kudu/tools/tablet_copyer.h:
http://gerrit.cloudera.org:8080/#/c/18732/13/src/kudu/tools/tablet_copyer.h@1
PS13, Line 1: // Licensed to the Apache Software Foundation (ASF) under one
implement this class in tool_action_local_replica.cc is enough, no need to expose it.
--
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: 13
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Tue, 26 Jul 2022 07:20:32 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18732
to look at the new patch set (#14).
Change subject: [Tools] Copy tablets from a remote server in batch
......................................................................
[Tools] Copy tablets from a remote server in batch
The command: kudu local_replica copy_from_remote only supports to copy one
tablet from a remote server once a time. It is not efficient when we need
to copy all tablets from a remote server.
Therefore this patch supports to copy tablets from a remote server in batch.
Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
---
M src/kudu/tablet/tablet_replica.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_local_replica.cc
3 files changed, 227 insertions(+), 116 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/18732/14
--
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: newpatchset
Gerrit-Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
Gerrit-Change-Number: 18732
Gerrit-PatchSet: 14
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
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 18: Code-Review+2
--
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: 18
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Mon, 01 Aug 2022 06:46:47 +0000
Gerrit-HasComments: No
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18732
to look at the new patch set (#17).
Change subject: [Tools] Copy tablets from a remote server in batch
......................................................................
[Tools] Copy tablets from a remote server in batch
The command: kudu local_replica copy_from_remote only supports to copy one
tablet from a remote server once a time. It is not efficient when we need
to copy all tablets from a remote server.
Therefore this patch supports to copy tablets from a remote server in batch.
Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
---
M src/kudu/tablet/tablet_replica.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_local_replica.cc
3 files changed, 212 insertions(+), 115 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/18732/17
--
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: newpatchset
Gerrit-Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
Gerrit-Change-Number: 18732
Gerrit-PatchSet: 17
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
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 16:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/18732/16/src/kudu/tools/tool_action_local_replica.cc
File src/kudu/tools/tool_action_local_replica.cc:
http://gerrit.cloudera.org:8080/#/c/18732/16/src/kudu/tools/tool_action_local_replica.cc@96
PS16, Line 96: namespace tools {
can be removed?
http://gerrit.cloudera.org:8080/#/c/18732/16/src/kudu/tools/tool_action_local_replica.cc@173
PS16, Line 173: using kudu::tools::TabletCopier;
can be removed?
http://gerrit.cloudera.org:8080/#/c/18732/16/src/kudu/tools/tool_action_local_replica.cc@217
PS16, Line 217: set<string>*
use common object, and you can std::move it in cstor, which is the google code style and used in Kudu
http://gerrit.cloudera.org:8080/#/c/18732/16/src/kudu/tools/tool_action_local_replica.cc@220
PS16, Line 220: HostPort*
same
http://gerrit.cloudera.org:8080/#/c/18732/16/src/kudu/tools/tool_action_local_replica.cc@225
PS16, Line 225: copy_type_ = CopyType::FROM_REMOTE;
you can also init it in cstor init-list
http://gerrit.cloudera.org:8080/#/c/18732/16/src/kudu/tools/tool_action_local_replica.cc@228
PS16, Line 228: std::
can be omitted
http://gerrit.cloudera.org:8080/#/c/18732/16/src/kudu/tools/tool_action_local_replica.cc@328
PS16, Line 328: delete client
how about use unique_ptr?
http://gerrit.cloudera.org:8080/#/c/18732/16/src/kudu/tools/tool_action_local_replica.cc@359
PS16, Line 359: enum
better to use enum class
--
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: 16
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 27 Jul 2022 03:56:53 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu 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 17:
(8 comments)
> Patch Set 16:
>
> (8 comments)
http://gerrit.cloudera.org:8080/#/c/18732/16/src/kudu/tools/tool_action_local_replica.cc
File src/kudu/tools/tool_action_local_replica.cc:
http://gerrit.cloudera.org:8080/#/c/18732/16/src/kudu/tools/tool_action_local_replica.cc@96
PS16, Line 96: namespace rpc {
> can be removed?
Done
http://gerrit.cloudera.org:8080/#/c/18732/16/src/kudu/tools/tool_action_local_replica.cc@173
PS16, Line 173: using kudu::tserver::TSTabletManager;
> can be removed?
Done
http://gerrit.cloudera.org:8080/#/c/18732/16/src/kudu/tools/tool_action_local_replica.cc@217
PS16, Line 217: tablet_id
> use common object, and you can std::move it in cstor, which is the google c
Done
http://gerrit.cloudera.org:8080/#/c/18732/16/src/kudu/tools/tool_action_local_replica.cc@220
PS16, Line 220: source
> same
Done
http://gerrit.cloudera.org:8080/#/c/18732/16/src/kudu/tools/tool_action_local_replica.cc@225
PS16, Line 225: FsManager* dst_fs_manage
> you can also init it in cstor init-list
Done
http://gerrit.cloudera.org:8080/#/c/18732/16/src/kudu/tools/tool_action_local_replica.cc@228
PS16, Line 228: set<s
> can be omitted
Done
http://gerrit.cloudera.org:8080/#/c/18732/16/src/kudu/tools/tool_action_local_replica.cc@328
PS16, Line 328: LOG(INFO)
> how about use unique_ptr?
Done
http://gerrit.cloudera.org:8080/#/c/18732/16/src/kudu/tools/tool_action_local_replica.cc@359
PS16, Line 359: FsMa
> better to use enum class
Done
--
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: 17
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 27 Jul 2022 10:52:02 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
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 6:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/18732/6/src/kudu/tools/tool_action_local_replica.cc
File src/kudu/tools/tool_action_local_replica.cc:
http://gerrit.cloudera.org:8080/#/c/18732/6/src/kudu/tools/tool_action_local_replica.cc@420
PS6, Line 420: Status CopyFromRemote(const RunnerContext& context) {
I found there are some duplicate code both in CopyFromLocal and CopyFromRemote, is it possible to do some refactor to reuse some code?
http://gerrit.cloudera.org:8080/#/c/18732/6/src/kudu/tools/tool_action_local_replica.cc@443
PS6, Line 443: clone_pool
nit: copy tool
http://gerrit.cloudera.org:8080/#/c/18732/6/src/kudu/tools/tool_action_local_replica.cc@444
PS6, Line 444: tool-tablet-copy-pool-pool
nit: tool-tablet-copy-pool
http://gerrit.cloudera.org:8080/#/c/18732/6/src/kudu/tools/tool_action_local_replica.cc@450
PS6, Line 450: clone
nit: copy
http://gerrit.cloudera.org:8080/#/c/18732/6/src/kudu/tools/tool_action_local_replica.cc@457
PS6, Line 457: return client.FetchAll(nullptr);
could you please add an extra thread to check the copy progress, it would be helpful if the copied replica is very large which would cost long time to copy. Similar to CopyFromLocal.
http://gerrit.cloudera.org:8080/#/c/18732/6/src/kudu/tools/tool_action_local_replica.cc@465
PS6, Line 465: s
The error details has been logged in the line above, is it neccesary to collect and log it again?
--
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: 6
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 20 Jul 2022 03:38:02 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu 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 5:
(6 comments)
> Patch Set 3:
>
> (6 comments)
http://gerrit.cloudera.org:8080/#/c/18732/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18732/3//COMMIT_MSG@13
PS3, Line 13: pa
> nit: patch
Done
http://gerrit.cloudera.org:8080/#/c/18732/3/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/18732/3/src/kudu/tools/kudu-tool-test.cc@8151
PS3, Line 8151: NO_FATALS(CreateTableWithFlushedData("table1", mini_cluster_.get(), 3, 1));
: NO_FATALS(CreateTableWithFlushedData("table2", mini_cluster_.get(), 3, 1
> nit: It suggest to explict set the 4th parameter (num_replicas) to 1 to cle
Done
http://gerrit.cloudera.org:8080/#/c/18732/3/src/kudu/tools/kudu-tool-test.cc@8176
PS3, Line 8176: find(target_tablet_ids.begin(), target_tablet_ids.end(), tablet_id)
: != target_tablet_ids.end()
> nit: ContainsKey(target_tablet_ids, tablet_id);
Function: ContainsKey does not support vector.
http://gerrit.cloudera.org:8080/#/c/18732/3/src/kudu/tools/tool_action_local_replica.cc
File src/kudu/tools/tool_action_local_replica.cc:
http://gerrit.cloudera.org:8080/#/c/18732/3/src/kudu/tools/tool_action_local_replica.cc@450
PS3, Line 450: unique_ptr<Thre
> nit: unique_ptr
Done
http://gerrit.cloudera.org:8080/#/c/18732/3/src/kudu/tools/tool_action_local_replica.cc@451
PS3, Line 451: tool-table
> nit: how about keep the same style with CopyFromLocal ? i.e. tool-tablet-c
Done
http://gerrit.cloudera.org:8080/#/c/18732/3/src/kudu/tools/tool_action_local_replica.cc@466
PS3, Line 466: return client.Finis
> Use the same method to refactor code as it in CopyFromLocal, i.e. use AndTh
Done
--
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: 5
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 15 Jul 2022 11:53:32 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
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 3:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/18732/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18732/3//COMMIT_MSG@13
PS3, Line 13: MR
nit: patch
http://gerrit.cloudera.org:8080/#/c/18732/3/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/18732/3/src/kudu/tools/kudu-tool-test.cc@8151
PS3, Line 8151: NO_FATALS(CreateTableWithFlushedData("table1", mini_cluster_.get(), 3));
: NO_FATALS(CreateTableWithFlushedData("table2", mini_cluster_.get(), 3));
nit: It suggest to explict set the 4th parameter (num_replicas) to 1 to clearify the two tservers have different replicas.
http://gerrit.cloudera.org:8080/#/c/18732/3/src/kudu/tools/kudu-tool-test.cc@8176
PS3, Line 8176: find(target_tablet_ids.begin(), target_tablet_ids.end(), tablet_id)
: != target_tablet_ids.end()
nit: ContainsKey(target_tablet_ids, tablet_id);
http://gerrit.cloudera.org:8080/#/c/18732/3/src/kudu/tools/tool_action_local_replica.cc
File src/kudu/tools/tool_action_local_replica.cc:
http://gerrit.cloudera.org:8080/#/c/18732/3/src/kudu/tools/tool_action_local_replica.cc@450
PS3, Line 450: std::unique_ptr
nit: unique_ptr
http://gerrit.cloudera.org:8080/#/c/18732/3/src/kudu/tools/tool_action_local_replica.cc@451
PS3, Line 451: clone-pool
nit: how about keep the same style with CopyFromLocal ? i.e. tool-tablet-copy-pool
http://gerrit.cloudera.org:8080/#/c/18732/3/src/kudu/tools/tool_action_local_replica.cc@466
PS3, Line 466: if (!s.ok()) break;
Use the same method to refactor code as it in CopyFromLocal, i.e. use AndThen.
--
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: 3
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 15 Jul 2022 08:48:03 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18732
to look at the new patch set (#7).
Change subject: [Tools] Copy tablets from a remote server in batch
......................................................................
[Tools] Copy tablets from a remote server in batch
The command: kudu local_replica copy_from_remote only supports to copy one
tablet from a remote server once a time. It is not efficient when we need
to copy all tablets from a remote server.
Therefore this patch supports to copy tablets from a remote server in batch.
Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
---
M src/kudu/tablet/tablet_replica.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/tablet_copy_client.h
4 files changed, 194 insertions(+), 122 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/18732/7
--
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: newpatchset
Gerrit-Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
Gerrit-Change-Number: 18732
Gerrit-PatchSet: 7
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18732
to look at the new patch set (#5).
Change subject: [Tools] Copy tablets from a remote server in batch
......................................................................
[Tools] Copy tablets from a remote server in batch
The command: kudu local_replica copy_from_remote only supports to copy one
tablet from a remote server once a time. It is not efficient when we need
to copy all tablets from a remote server.
Therefore this patch supports to copy tablets from a remote server in batch.
Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_local_replica.cc
2 files changed, 107 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/18732/5
--
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: newpatchset
Gerrit-Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
Gerrit-Change-Number: 18732
Gerrit-PatchSet: 5
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18732
to look at the new patch set (#18).
Change subject: [Tools] Copy tablets from a remote server in batch
......................................................................
[Tools] Copy tablets from a remote server in batch
The command: kudu local_replica copy_from_remote only supports to copy one
tablet from a remote server once a time. It is not efficient when we need
to copy all tablets from a remote server.
Therefore this patch supports to copy tablets from a remote server in batch.
Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
---
M src/kudu/tablet/tablet_replica.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_local_replica.cc
3 files changed, 212 insertions(+), 115 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/18732/18
--
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: newpatchset
Gerrit-Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
Gerrit-Change-Number: 18732
Gerrit-PatchSet: 18
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18732
to look at the new patch set (#9).
Change subject: [Tools] Copy tablets from a remote server in batch
......................................................................
[Tools] Copy tablets from a remote server in batch
The command: kudu local_replica copy_from_remote only supports to copy one
tablet from a remote server once a time. It is not efficient when we need
to copy all tablets from a remote server.
Therefore this patch supports to copy tablets from a remote server in batch.
Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
---
M src/kudu/tablet/tablet_replica.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/tablet_copy_client.h
4 files changed, 194 insertions(+), 121 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/18732/9
--
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: newpatchset
Gerrit-Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
Gerrit-Change-Number: 18732
Gerrit-PatchSet: 9
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18732
to look at the new patch set (#13).
Change subject: [Tools] Copy tablets from a remote server in batch
......................................................................
[Tools] Copy tablets from a remote server in batch
The command: kudu local_replica copy_from_remote only supports to copy one
tablet from a remote server once a time. It is not efficient when we need
to copy all tablets from a remote server.
Therefore this patch supports to copy tablets from a remote server in batch.
Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
---
M src/kudu/tablet/tablet_replica.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-tool-test.cc
A src/kudu/tools/tablet_copyer.cc
A src/kudu/tools/tablet_copyer.h
M src/kudu/tools/tool_action_local_replica.cc
6 files changed, 349 insertions(+), 123 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/18732/13
--
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: newpatchset
Gerrit-Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
Gerrit-Change-Number: 18732
Gerrit-PatchSet: 13
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18732
to look at the new patch set (#3).
Change subject: [Tools] Copy tablets from a remote server in batch
......................................................................
[Tools] Copy tablets from a remote server in batch
The command: kudu local_replica copy_from_remote only supports to copy one
tablet from a remote server once a time. It is not efficient when we need
to copy all tablets from a remote server.
Therefore this MR supports to copy tablets from a remote server in batch.
Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_local_replica.cc
2 files changed, 113 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/18732/3
--
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: newpatchset
Gerrit-Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
Gerrit-Change-Number: 18732
Gerrit-PatchSet: 3
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18732
to look at the new patch set (#11).
Change subject: [Tools] Copy tablets from a remote server in batch
......................................................................
[Tools] Copy tablets from a remote server in batch
The command: kudu local_replica copy_from_remote only supports to copy one
tablet from a remote server once a time. It is not efficient when we need
to copy all tablets from a remote server.
Therefore this patch supports to copy tablets from a remote server in batch.
Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
---
M src/kudu/tablet/tablet_replica.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-tool-test.cc
A src/kudu/tools/tablet_copyer.cc
A src/kudu/tools/tablet_copyer.h
M src/kudu/tools/tool_action_local_replica.cc
6 files changed, 323 insertions(+), 121 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/18732/11
--
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: newpatchset
Gerrit-Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
Gerrit-Change-Number: 18732
Gerrit-PatchSet: 11
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu 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 8:
(4 comments)
> 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?
Construction function: TabletReplica() is private before?we cannot use it directly in function: CopyTablets()
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: TabletCopyClient* client;
> There is only two types, and seems impossible to add the 3rd one, so I thin
Done
http://gerrit.cloudera.org:8080/#/c/18732/7/src/kudu/tools/tool_action_local_replica.cc@281
PS7, Line 281: tCopyCl
> fake_replica can be used to check the copy progess, you can use it as in lo
Done
http://gerrit.cloudera.org:8080/#/c/18732/7/src/kudu/tools/tool_action_local_replica.cc@565
PS7, Line 565:
> nit: this paramter can be omitted, and you can use this flag in the new add
Done
--
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: 8
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Mon, 25 Jul 2022 02:36:18 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
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 8:
(1 comment)
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();
> Construction function: TabletReplica() is private before?we cannot use it d
not neccessary, you can declare it as a friend function.
--
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: 8
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Mon, 25 Jul 2022 03:04:31 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
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 17:
(2 comments)
the failed test is related, have to fix it before mergering.
http://gerrit.cloudera.org:8080/#/c/18732/17/src/kudu/tools/tool_action_local_replica.cc
File src/kudu/tools/tool_action_local_replica.cc:
http://gerrit.cloudera.org:8080/#/c/18732/17/src/kudu/tools/tool_action_local_replica.cc@311
PS17, Line 311: dst_cmeta_manager_, /* messenger */ nullptr,
: &tablet_copy_client_metrics, src_fs_manager_,
: /* tablet_copy_source_metrics */ nullptr));
nit: alignment
http://gerrit.cloudera.org:8080/#/c/18732/17/src/kudu/tools/tool_action_local_replica.cc@630
PS17, Line 630: &dst_fs_manager,
: move(dst_cmeta_manager),
: &src_fs_manager,
: move(src_tablet_ids_set));
nit: alignment
--
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: 17
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Sat, 30 Jul 2022 07:45:00 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu 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 18:
(2 comments)
> Patch Set 18: Verified+1
>
> Build Successful
>
> http://jenkins.kudu.apache.org/job/kudu-gerrit/26194/ : SUCCESS
http://gerrit.cloudera.org:8080/#/c/18732/17/src/kudu/tools/tool_action_local_replica.cc
File src/kudu/tools/tool_action_local_replica.cc:
http://gerrit.cloudera.org:8080/#/c/18732/17/src/kudu/tools/tool_action_local_replica.cc@311
PS17, Line 311: dst_cmeta_manager_, /* messenger */ nullptr,
: &tablet_copy_client_metrics, src_fs_manager_,
: /* tablet_copy_source_metrics */ nullpt
> nit: alignment
Done
http://gerrit.cloudera.org:8080/#/c/18732/17/src/kudu/tools/tool_action_local_replica.cc@630
PS17, Line 630: &dst_fs_manager,
: move(dst_cmeta_manager),
: &src_fs_manager,
: move(src_tablet_ids_set))
> nit: alignment
Done
--
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: 18
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Mon, 01 Aug 2022 06:44:54 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18732
to look at the new patch set (#4).
Change subject: [Tools] Copy tablets from a remote server in batch
......................................................................
[Tools] Copy tablets from a remote server in batch
The command: kudu local_replica copy_from_remote only supports to copy one
tablet from a remote server once a time. It is not efficient when we need
to copy all tablets from a remote server.
Therefore this MR supports to copy tablets from a remote server in batch.
Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_local_replica.cc
2 files changed, 107 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/18732/4
--
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: newpatchset
Gerrit-Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
Gerrit-Change-Number: 18732
Gerrit-PatchSet: 4
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18732
to look at the new patch set (#2).
Change subject: [Tools] Copy tablets from a remote server in batch
......................................................................
[Tools] Copy tablets from a remote server in batch
The command: kudu local_replica copy_from_remote only supports to copy one
tablet from a remote server once a time. It is not efficient when we need
to copy all tablets from a remote server.
Therefore this MR supports to copy tablets from a remote server in batch.
Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_local_replica.cc
2 files changed, 107 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/18732/2
--
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: newpatchset
Gerrit-Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
Gerrit-Change-Number: 18732
Gerrit-PatchSet: 2
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu 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)
> Patch Set 6:
>
> (6 comments)
http://gerrit.cloudera.org:8080/#/c/18732/6/src/kudu/tools/tool_action_local_replica.cc
File src/kudu/tools/tool_action_local_replica.cc:
http://gerrit.cloudera.org:8080/#/c/18732/6/src/kudu/tools/tool_action_local_replica.cc@420
PS6, Line 420: if (tablet_ids.empty()) {
> I found there are some duplicate code both in CopyFromLocal and CopyFromRem
Done
http://gerrit.cloudera.org:8080/#/c/18732/6/src/kudu/tools/tool_action_local_replica.cc@443
PS6, Line 443: cons
> nit: copy tool
Done
http://gerrit.cloudera.org:8080/#/c/18732/6/src/kudu/tools/tool_action_local_replica.cc@444
PS6, Line 444: er->env();
> nit: tool-tablet-copy-pool
Done
http://gerrit.cloudera.org:8080/#/c/18732/6/src/kudu/tools/tool_action_local_replica.cc@450
PS6, Line 450: cmet
> nit: copy
Done
http://gerrit.cloudera.org:8080/#/c/18732/6/src/kudu/tools/tool_action_local_replica.cc@465
PS6, Line 465:
> The error details has been logged in the line above, is it neccesary to col
Done
--
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 <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 21 Jul 2022 10:38:00 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18732
to look at the new patch set (#10).
Change subject: [Tools] Copy tablets from a remote server in batch
......................................................................
[Tools] Copy tablets from a remote server in batch
The command: kudu local_replica copy_from_remote only supports to copy one
tablet from a remote server once a time. It is not efficient when we need
to copy all tablets from a remote server.
Therefore this patch supports to copy tablets from a remote server in batch.
Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
---
M src/kudu/tablet/tablet_replica.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/tablet_copy_client.h
4 files changed, 195 insertions(+), 121 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/18732/10
--
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: newpatchset
Gerrit-Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
Gerrit-Change-Number: 18732
Gerrit-PatchSet: 10
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18732
to look at the new patch set (#12).
Change subject: [Tools] Copy tablets from a remote server in batch
......................................................................
[Tools] Copy tablets from a remote server in batch
The command: kudu local_replica copy_from_remote only supports to copy one
tablet from a remote server once a time. It is not efficient when we need
to copy all tablets from a remote server.
Therefore this patch supports to copy tablets from a remote server in batch.
Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
---
M src/kudu/tablet/tablet_replica.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-tool-test.cc
A src/kudu/tools/tablet_copyer.cc
A src/kudu/tools/tablet_copyer.h
M src/kudu/tools/tool_action_local_replica.cc
6 files changed, 349 insertions(+), 123 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/18732/12
--
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: newpatchset
Gerrit-Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
Gerrit-Change-Number: 18732
Gerrit-PatchSet: 12
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18732
to look at the new patch set (#16).
Change subject: [Tools] Copy tablets from a remote server in batch
......................................................................
[Tools] Copy tablets from a remote server in batch
The command: kudu local_replica copy_from_remote only supports to copy one
tablet from a remote server once a time. It is not efficient when we need
to copy all tablets from a remote server.
Therefore this patch supports to copy tablets from a remote server in batch.
Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
---
M src/kudu/tablet/tablet_replica.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_local_replica.cc
3 files changed, 220 insertions(+), 115 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/18732/16
--
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: newpatchset
Gerrit-Change-Id: Ib598142883b8ab958625a4f04648d58ea95f3664
Gerrit-Change-Number: 18732
Gerrit-PatchSet: 16
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Copy tablets from a remote server in batch
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu 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 16:
(3 comments)
> Patch Set 13:
>
> (3 comments)
http://gerrit.cloudera.org:8080/#/c/18732/13/src/kudu/tablet/tablet_replica.h
File src/kudu/tablet/tablet_replica.h:
http://gerrit.cloudera.org:8080/#/c/18732/13/src/kudu/tablet/tablet_replica.h@26
PS13, Line 26: #include <string>
> nit: outdated, remove it
Done
http://gerrit.cloudera.org:8080/#/c/18732/13/src/kudu/tablet/tablet_replica.h@87
PS13, Line 87: amespace too
> nit: TabletCopier
Done
http://gerrit.cloudera.org:8080/#/c/18732/13/src/kudu/tools/tablet_copyer.h
File src/kudu/tools/tablet_copyer.h:
http://gerrit.cloudera.org:8080/#/c/18732/13/src/kudu/tools/tablet_copyer.h@1
PS13, Line 1:
> implement this class in tool_action_local_replica.cc is enough, no need to
Done
--
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: 16
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 27 Jul 2022 01:50:43 +0000
Gerrit-HasComments: Yes