You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Yingchun Lai (Code Review)" <ge...@cloudera.org> on 2022/04/01 12:54:56 UTC

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Yingchun Lai has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18374


Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................

[tool] Add tool to copy replica from local filesystem

Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
---
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica.h
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/tablet_copy-test-base.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/tablet_copy_service-test.cc
M src/kudu/tserver/tablet_copy_service.cc
M src/kudu/tserver/tablet_copy_service.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_copy_source_session.h
13 files changed, 702 insertions(+), 207 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/18374/1
-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 1
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18374

to look at the new patch set (#13).

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................

[tool] Add tool to copy replica from local filesystem

Add tool to copy replica from local filesystem, it would
be faster than copy from remote peers, it can be use when
add more disk drivers and make data rebalanced faster between
disk drivers, or can be used when migrate data from the only
disk driver to another.
This tool will also make data more dense than data on old
data directories, that means we can save much disk space
and speedup server bootstrap.
We can use the tool like:
kudu local_replica copy_from_local <tablet_id> [-src_fs_wal_dir=<dir>] [-src_fs_metadata_dir=<dir>] [-src_fs_data_dirs=<dirs>] [-dst_fs_wal_dir=<dir>] [-dst_fs_metadata_dir=<dir>] [-dst_fs_data_dirs=<dirs>]

There are some tips to use this tool:
- Using --src_* and --dst_* prefixes to clarify what directories
  are operating on
- The server on the source filesystem must stop before coping
  replicas. Because the tool is standalone, it can't anchor
  replica's data blocks or log segments.
- This tool will not delete data in the source filesystem, you
  should delete data manully if you want, by using
  kudu local_replica delete ...

Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
---
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/master/master_runner.cc
M src/kudu/mini-cluster/internal_mini_cluster.h
M src/kudu/tablet/tablet_replica.cc
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/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
M src/kudu/tserver/tablet_copy-test-base.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/tablet_copy_service.cc
M src/kudu/tserver/tablet_copy_service.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_copy_source_session.h
M src/kudu/tserver/ts_tablet_manager.cc
19 files changed, 1,042 insertions(+), 243 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/18374/13
-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 13
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18374

to look at the new patch set (#9).

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................

[tool] Add tool to copy replica from local filesystem

Add tool to copy replica from local filesystem, it would
be faster than copy from remote peers, it can be use when
add more disk drivers and make data rebalanced faster between
disk drivers, or can be used when migrate data from the only
disk driver to another.
This tool will also make data more dense than data on old
data directories, that means we can save much disk space
and speedup server bootstrap.
We can use the tool like:
kudu local_replica copy_from_local <tablet_id> [-src_fs_wal_dir=<dir>] [-src_fs_metadata_dir=<dir>] [-src_fs_data_dirs=<dirs>] [-dst_fs_wal_dir=<dir>] [-dst_fs_metadata_dir=<dir>] [-dst_fs_data_dirs=<dirs>]

There are some tips to use this tool:
- Using --src_* and --dst_* prefixes to clarify what directories
  are operating on
- The server on the source filesystem must stop before coping
  replicas. Because the tool is standalone, it can't anchor
  replica's data blocks or log segments.
- This tool will not delete data in the source filesystem, you
  should delete data manully if you want, by using
  kudu local_replica delete ...

Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
---
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/master/master_runner.cc
M src/kudu/tablet/tablet_replica.cc
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-test-base.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/tablet_copy_service.cc
M src/kudu/tserver/tablet_copy_service.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_copy_source_session.h
M src/kudu/tserver/ts_tablet_manager.cc
16 files changed, 899 insertions(+), 229 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/18374/9
-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 9
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18374 )

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................


Patch Set 13:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/18374/12/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/18374/12/src/kudu/tools/kudu-tool-test.cc@7986
PS12, Line 7986:   for (int i = 0; i < mini_cluster_->num_tablet_servers(); ++i) {
> warning: 'emplace_back' is called inside a loop; consider pre-allocating th
Done


http://gerrit.cloudera.org:8080/#/c/18374/12/src/kudu/tools/kudu-tool-test.cc@7989
PS12, Line 7989:   auto fs_manager = mini_cluster_->mini_tablet_server(kCopyTserverIndex)->server()->fs_manager();
> warning: the variable 'src_fs_uuid' is copy-constructed from a const refere
Done


http://gerrit.cloudera.org:8080/#/c/18374/12/src/kudu/tserver/mini_tablet_server.cc
File src/kudu/tserver/mini_tablet_server.cc:

http://gerrit.cloudera.org:8080/#/c/18374/12/src/kudu/tserver/mini_tablet_server.cc@87
PS12, Line 87:     for (int dir = 0; dir < num_data_dirs; dir++) {
> warning: 'emplace_back' is called inside a loop; consider pre-allocating th
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 13
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 13 Apr 2022 10:15:40 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18374

to look at the new patch set (#16).

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................

[tool] Add tool to copy replica from local filesystem

Add tool to copy replica from local filesystem, it would
be faster than copy from remote peers, it can be use when
add more disk drivers and make data rebalanced faster between
disk drivers, or can be used when migrate data from the only
disk driver to another.
This tool will also make data more dense than data on old
data directories, that means we can save much disk space
and speedup server bootstrap.
We can use the tool like:
kudu local_replica copy_from_local <tablet_id> [-src_fs_wal_dir=<dir>] [-src_fs_metadata_dir=<dir>] [-src_fs_data_dirs=<dirs>] [-dst_fs_wal_dir=<dir>] [-dst_fs_metadata_dir=<dir>] [-dst_fs_data_dirs=<dirs>]

There are some tips to use this tool:
- Using --src_* and --dst_* prefixes to clarify what directories
  are operating on
- The server on the source filesystem must stop before coping
  replicas. Because the tool is standalone, it can't anchor
  replica's data blocks or log segments.
- This tool will not delete data in the source filesystem, you
  should delete data manully if you want, by using
  kudu local_replica delete ...

Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
---
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/master/master_runner.cc
M src/kudu/mini-cluster/internal_mini_cluster.h
M src/kudu/tablet/tablet_replica.cc
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/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
M src/kudu/tserver/tablet_copy-test-base.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/tablet_copy_service.cc
M src/kudu/tserver/tablet_copy_service.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_copy_source_session.h
M src/kudu/tserver/ts_tablet_manager.cc
19 files changed, 1,040 insertions(+), 243 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/18374/16
-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 16
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18374

to look at the new patch set (#5).

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................

[tool] Add tool to copy replica from local filesystem

Add tool to copy replica from local filesystem, it would
be faster than copy from remote peers, it can be use when
add more disk drivers and make data rebalanced faster between
disk drivers, or can be used when migrate data from the only
disk driver to another.
This tool will also make data more dense than data on old
data directories, that means we can save much disk space
and speedup server bootstrap.
We can use the tool like:
kudu local_replica copy_from_local <tablet_id> [-src_fs_wal_dir=<dir>] [-src_fs_metadata_dir=<dir>] [-src_fs_data_dirs=<dirs>] [-dst_fs_wal_dir=<dir>] [-dst_fs_metadata_dir=<dir>] [-dst_fs_data_dirs=<dirs>]

There are some tips to use this tool:
- Using --src_* and --dst_* prefixes to clarify what directories
  are operating on
- The server on the source filesystem must stop before coping
  replicas. Because the tool is standalone, it can't anchor
  replica's data blocks or log segments.
- This tool will not delete data in the source filesystem, you
  should delete data manully if you want, by using
  kudu local_replica delete ...

Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
---
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/master/master_runner.cc
M src/kudu/tablet/tablet_replica.cc
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-test-base.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/tablet_copy_service-test.cc
M src/kudu/tserver/tablet_copy_service.cc
M src/kudu/tserver/tablet_copy_service.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_copy_source_session.h
M src/kudu/tserver/ts_tablet_manager.cc
17 files changed, 910 insertions(+), 247 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/18374/5
-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 5
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18374

to look at the new patch set (#14).

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................

[tool] Add tool to copy replica from local filesystem

Add tool to copy replica from local filesystem, it would
be faster than copy from remote peers, it can be use when
add more disk drivers and make data rebalanced faster between
disk drivers, or can be used when migrate data from the only
disk driver to another.
This tool will also make data more dense than data on old
data directories, that means we can save much disk space
and speedup server bootstrap.
We can use the tool like:
kudu local_replica copy_from_local <tablet_id> [-src_fs_wal_dir=<dir>] [-src_fs_metadata_dir=<dir>] [-src_fs_data_dirs=<dirs>] [-dst_fs_wal_dir=<dir>] [-dst_fs_metadata_dir=<dir>] [-dst_fs_data_dirs=<dirs>]

There are some tips to use this tool:
- Using --src_* and --dst_* prefixes to clarify what directories
  are operating on
- The server on the source filesystem must stop before coping
  replicas. Because the tool is standalone, it can't anchor
  replica's data blocks or log segments.
- This tool will not delete data in the source filesystem, you
  should delete data manully if you want, by using
  kudu local_replica delete ...

Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
---
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/master/master_runner.cc
M src/kudu/mini-cluster/internal_mini_cluster.h
M src/kudu/tablet/tablet_replica.cc
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/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
M src/kudu/tserver/tablet_copy-test-base.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/tablet_copy_service.cc
M src/kudu/tserver/tablet_copy_service.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_copy_source_session.h
M src/kudu/tserver/ts_tablet_manager.cc
19 files changed, 1,042 insertions(+), 243 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/18374/14
-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 14
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18374 )

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................

[tool] Add tool to copy replica from local filesystem

Add tool to copy replica from local filesystem, it would
be faster than copy from remote peers, it can be use when
add more disk drivers and make data rebalanced faster between
disk drivers, or can be used when migrate data from the only
disk driver to another.
This tool will also make data more dense than data on old
data directories, that means we can save much disk space
and speedup server bootstrap.
We can use the tool like:
kudu local_replica copy_from_local <tablet_id> [-src_fs_wal_dir=<dir>] [-src_fs_metadata_dir=<dir>] [-src_fs_data_dirs=<dirs>] [-dst_fs_wal_dir=<dir>] [-dst_fs_metadata_dir=<dir>] [-dst_fs_data_dirs=<dirs>]

There are some tips to use this tool:
- Using --src_* and --dst_* prefixes to clarify what directories
  are operating on
- The server on the source filesystem must stop before coping
  replicas. Because the tool is standalone, it can't anchor
  replica's data blocks or log segments.
- This tool will not delete data in the source filesystem, you
  should delete data manully if you want, by using
  kudu local_replica delete ...

Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Reviewed-on: http://gerrit.cloudera.org:8080/18374
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/master/master_runner.cc
M src/kudu/mini-cluster/internal_mini_cluster.h
M src/kudu/tablet/tablet_replica.cc
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/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
M src/kudu/tserver/tablet_copy-test-base.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/tablet_copy_service.cc
M src/kudu/tserver/tablet_copy_service.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_copy_source_session.h
M src/kudu/tserver/ts_tablet_manager.cc
19 files changed, 1,040 insertions(+), 243 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Andrew Wong: Looks good to me, approved

-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 17
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18374 )

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................


Patch Set 11:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/18374/7//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18374/7//COMMIT_MSG@21
PS7, Line 21: - Using --src_* and --dst_* prefixes to clarify what directories
> Thanks for clarifying.
Yes, the fs uuid must be the same. We can use 'kudu fs format --uuid=xxx' to format the new fs layout.
Test added.


http://gerrit.cloudera.org:8080/#/c/18374/10/src/kudu/tserver/tablet_copy_client.cc
File src/kudu/tserver/tablet_copy_client.cc:

http://gerrit.cloudera.org:8080/#/c/18374/10/src/kudu/tserver/tablet_copy_client.cc@1096
PS10, Line 1096: 
> nit: why not set this to something like "local replica <uuid>"?
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 11
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 13 Apr 2022 09:10:28 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18374 )

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................


Patch Set 3:

(17 comments)

Thank you for the patch!

I did took a quick look, adding initial feedback.

http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG@7
PS3, Line 7: tool
It would be great to provide an example of the usage for the tool.

That could clarify on a few questions below:

1. What to do with the original source replica after it's copied?
2. What's is the behavior of the tool when starting it under a running tablet server?


http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG@9
PS3, Line 9: would
is


http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG@10
PS3, Line 10: copy
copying


http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG@10
PS3, Line 10: use
used


http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG@11
PS3, Line 11: disk drivers
data directories


http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG@11
PS3, Line 11: rebalance
rebalancing


http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG@11
PS3, Line 11: add
adding


http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG@12
PS3, Line 12: disk driver
data directory


http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG@12
PS3, Line 12: disk drivers
them


http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tablet/tablet_replica.h
File src/kudu/tablet/tablet_replica.h:

http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tablet/tablet_replica.h@115
PS3, Line 115:   TabletReplica();
If this is a constructor for a limited audience, consider moving this into the private section and add corresponding 'friends' declarations.


http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tserver/tablet_copy_client.h
File src/kudu/tserver/tablet_copy_client.h:

PS3: 
Instead of introducing Mode and doing , did you


http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tserver/tablet_copy_client.h@316
PS3, Line 316: client
nit: to be inline with tablet_copy_source_metrics_ and dst_fs_manager_, should this be named tablet_copy_dst_metrics_ instead?


http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tserver/tablet_copy_client.cc
File src/kudu/tserver/tablet_copy_client.cc:

http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tserver/tablet_copy_client.cc@880
PS3, Line 880:  if (mode_ == TabletCopyMode::REMOTE) {
             :     RETURN_NOT_OK_PREPEND(DownloadFile(data_id, writer.get()),
             :                           Substitute("Unable to download WAL segment with seq. number $0",
             :                                      wal_segment_seqno));
             :   } else {
             :     CHECK(mode_ == TabletCopyMode::LOCAL);
             :     RETURN_NOT_OK_PREPEND(CopyFile(data_id, writer.get()),
             :                           Substitute("Unable to copy WAL segment with seq. number $0",
             :                                      wal_segment_seqno));
             :   }
nit: maybe, change this if/else to switch?


http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tserver/tablet_copy_client.cc@972
PS3, Line 972:   if (mode_ == TabletCopyMode::REMOTE) {
             :     RETURN_NOT_OK_PREPEND(DownloadFile(data_id, block.get()),
             :                           Substitute("Unable to download block $0", old_block_id.ToString()));
             :   } else {
             :     CHECK(mode_ == TabletCopyMode::LOCAL);
             :     RETURN_NOT_OK_PREPEND(CopyFile(data_id, block.get()),
             :                           Substitute("Unable to copy block $0", old_block_id.ToString()));
             :   }
nit: maybe, change this if/else to switch?


http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tserver/tablet_copy_client.cc@1067
PS3, Line 1067:     done = (offset + chunk_size) == total_data_length;
nit: maybe, move this one line below, and then there is no need to compute (offset + chunk_size) since the offset is updated one line below.


http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tserver/tablet_copy_service.h
File src/kudu/tserver/tablet_copy_service.h:

PS3: 
Could you separate changes on these tablet_copy_service.{h,cc} into its own changelist?  That way it's much easier to track the relevant changes and review the code.


http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tserver/tablet_copy_source_session.h
File src/kudu/tserver/tablet_copy_source_session.h:

PS3: 
Instead of introducing the TabletCopyMode enum, did you consider an option to separate a base class and have two derived classes for local and remote tablet copying sessions?



-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 01 Apr 2022 19:18:39 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18374 )

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG@11
PS3, Line 11: anced fas
> rebalancing
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 4
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Sat, 02 Apr 2022 07:31:15 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18374

to look at the new patch set (#6).

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................

[tool] Add tool to copy replica from local filesystem

Add tool to copy replica from local filesystem, it would
be faster than copy from remote peers, it can be use when
add more disk drivers and make data rebalanced faster between
disk drivers, or can be used when migrate data from the only
disk driver to another.
This tool will also make data more dense than data on old
data directories, that means we can save much disk space
and speedup server bootstrap.
We can use the tool like:
kudu local_replica copy_from_local <tablet_id> [-src_fs_wal_dir=<dir>] [-src_fs_metadata_dir=<dir>] [-src_fs_data_dirs=<dirs>] [-dst_fs_wal_dir=<dir>] [-dst_fs_metadata_dir=<dir>] [-dst_fs_data_dirs=<dirs>]

There are some tips to use this tool:
- Using --src_* and --dst_* prefixes to clarify what directories
  are operating on
- The server on the source filesystem must stop before coping
  replicas. Because the tool is standalone, it can't anchor
  replica's data blocks or log segments.
- This tool will not delete data in the source filesystem, you
  should delete data manully if you want, by using
  kudu local_replica delete ...

Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
---
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/master/master_runner.cc
M src/kudu/tablet/tablet_replica.cc
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-test-base.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/tablet_copy_service-test.cc
M src/kudu/tserver/tablet_copy_service.cc
M src/kudu/tserver/tablet_copy_service.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_copy_source_session.h
M src/kudu/tserver/ts_tablet_manager.cc
17 files changed, 911 insertions(+), 247 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/18374/6
-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 6
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18374

to look at the new patch set (#3).

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................

[tool] Add tool to copy replica from local filesystem

Add tool to copy replica from local filesystem, it would
be faster than copy from remote peers, it can be use when
add more disk drivers and make fast data rebalance between
disk drivers, or migrate data from the only disk driver to
another.
This tool will also make data more dense than data on old
data directories.

Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
---
M src/kudu/tablet/tablet_replica.cc
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-test-base.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/tablet_copy_service-test.cc
M src/kudu/tserver/tablet_copy_service.cc
M src/kudu/tserver/tablet_copy_service.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_copy_source_session.h
14 files changed, 809 insertions(+), 226 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/18374/3
-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/18374 )

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................


Patch Set 7:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/18374/7//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18374/7//COMMIT_MSG@21
PS7, Line 21: - Using --src_* and --dst_* prefixes to clarify what directories
So just checking my understanding, the idea here is that a user should stand up a new Kudu tserver at the same machine, but mounting a different set of directories. Once set up, the user runs this tool to copy tablets from one local tserver to the other, and once complete, starts up the new tserver?

I also wonder if you considered an online process for rebalancing data (eg through a maintenance manager thread, or a tweak in compaction prioritization or somesuch).


http://gerrit.cloudera.org:8080/#/c/18374/7/src/kudu/tools/tool_action_local_replica.cc
File src/kudu/tools/tool_action_local_replica.cc:

http://gerrit.cloudera.org:8080/#/c/18374/7/src/kudu/tools/tool_action_local_replica.cc@482
PS7, Line 482: LOG(INFO) << fake_replica->last_status();
I'm not sure how much value this logging adds, over just doing it in the tool's main thread. Presumably, fake_replica->last_status() will be OK for the duration of the tool, right? And we seems like we do have some logging throughout the client's life cycle. Alternatively, maybe consider instantiating the tablet_copy_client_metrics and outputting them?


http://gerrit.cloudera.org:8080/#/c/18374/7/src/kudu/tserver/tablet_copy_client-test.cc
File src/kudu/tserver/tablet_copy_client-test.cc:

http://gerrit.cloudera.org:8080/#/c/18374/7/src/kudu/tserver/tablet_copy_client-test.cc@20
PS7, Line 20: 
nit: can drop the extra line


http://gerrit.cloudera.org:8080/#/c/18374/7/src/kudu/tserver/tablet_copy_client.h
File src/kudu/tserver/tablet_copy_client.h:

http://gerrit.cloudera.org:8080/#/c/18374/7/src/kudu/tserver/tablet_copy_client.h@197
PS7, Line 197:                         const boost::optional<std::string>& copy_peer_uuid = boost::none);
nit: why not rely on an empty string? Or pass in the local fs UUID?

It's not too important, but it is a bit awkward as is, since this can be valid, invalid ("(unknown uuid)"), or none


http://gerrit.cloudera.org:8080/#/c/18374/7/src/kudu/tserver/tablet_copy_client.cc
File src/kudu/tserver/tablet_copy_client.cc:

http://gerrit.cloudera.org:8080/#/c/18374/7/src/kudu/tserver/tablet_copy_client.cc@352
PS7, Line 352: CHECK_OK
Why not RETURN_NOT_OK? Won't the tool exit early anyway?


http://gerrit.cloudera.org:8080/#/c/18374/7/src/kudu/tserver/tablet_copy_client.cc@1055
PS7, Line 1055: pb_util::SecureShortDebugString(session_->tablet_superblock()));
nit: the superblock might be huge, and it might be difficult to figure out what the state actually is. Consider using `TabletDataState_Name(data_state)`?


http://gerrit.cloudera.org:8080/#/c/18374/7/src/kudu/tserver/tablet_copy_client.cc@1097
PS7, Line 1097: CHECK_OK
nit: RETURN_NOT_OK? I think this can fail if there's no space, so it'd be nice to fail gracefully


http://gerrit.cloudera.org:8080/#/c/18374/7/src/kudu/tserver/tablet_copy_source_session.h
File src/kudu/tserver/tablet_copy_source_session.h:

http://gerrit.cloudera.org:8080/#/c/18374/7/src/kudu/tserver/tablet_copy_source_session.h@243
PS7, Line 243:   std::string session_id_;
             :   std::string requestor_uuid_;
nit: these can both be const



-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 7
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Mon, 11 Apr 2022 19:05:37 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18374 )

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................


Patch Set 8:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/18374/7//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18374/7//COMMIT_MSG@21
PS7, Line 21: - Using --src_* and --dst_* prefixes to clarify what directories
> a user should stand up a new Kudu tserver at the same machine

No need to start a real server, actually use Kudu CLI to format a new filesystem layout is OK. And after local replica copied, use the new ---src_fs_data_dirs and --dst_fs_wal_dir to start the server. In real usage scenario, we have to copy all replicas, i.e. use the tool to copy replica one by one (we can improve the tool to copy all replicas on a tserver in the future).

Online process for rebalancing data between data dirs is a good idea, we can implement it after 'local tablet copy' supportted(and anchor Logs and Blocks is needed then).


http://gerrit.cloudera.org:8080/#/c/18374/7/src/kudu/tools/tool_action_local_replica.cc
File src/kudu/tools/tool_action_local_replica.cc:

http://gerrit.cloudera.org:8080/#/c/18374/7/src/kudu/tools/tool_action_local_replica.cc@482
PS7, Line 482: LOG(INFO) << fake_replica->last_status();
> I'm not sure how much value this logging adds, over just doing it in the to
The main usage of this log is it willl show the copy progress, for a large size tablet, we can see and estimate how long it cost and will cost. The status is not Status but a std::string, e.g. 'Downloading block $0 ($1/$2)', 'Downloading WAL segment with seq. number $0 ($1/$2)'


http://gerrit.cloudera.org:8080/#/c/18374/7/src/kudu/tserver/tablet_copy_client-test.cc
File src/kudu/tserver/tablet_copy_client-test.cc:

http://gerrit.cloudera.org:8080/#/c/18374/7/src/kudu/tserver/tablet_copy_client-test.cc@20
PS7, Line 20: #include <cstdint>
> nit: can drop the extra line
Done


http://gerrit.cloudera.org:8080/#/c/18374/7/src/kudu/tserver/tablet_copy_client.h
File src/kudu/tserver/tablet_copy_client.h:

http://gerrit.cloudera.org:8080/#/c/18374/7/src/kudu/tserver/tablet_copy_client.h@197
PS7, Line 197: 
> nit: why not rely on an empty string? Or pass in the local fs UUID?
Done


http://gerrit.cloudera.org:8080/#/c/18374/7/src/kudu/tserver/tablet_copy_client.cc
File src/kudu/tserver/tablet_copy_client.cc:

http://gerrit.cloudera.org:8080/#/c/18374/7/src/kudu/tserver/tablet_copy_client.cc@352
PS7, Line 352: 
> Why not RETURN_NOT_OK? Won't the tool exit early anyway?
Done


http://gerrit.cloudera.org:8080/#/c/18374/7/src/kudu/tserver/tablet_copy_client.cc@1055
PS7, Line 1055: Substitute("Source tablet is not ready itself! state: $0",
> nit: the superblock might be huge, and it might be difficult to figure out 
Improved, also L313.


http://gerrit.cloudera.org:8080/#/c/18374/7/src/kudu/tserver/tablet_copy_client.cc@1097
PS7, Line 1097: 
> nit: RETURN_NOT_OK? I think this can fail if there's no space, so it'd be n
Done


http://gerrit.cloudera.org:8080/#/c/18374/7/src/kudu/tserver/tablet_copy_source_session.h
File src/kudu/tserver/tablet_copy_source_session.h:

http://gerrit.cloudera.org:8080/#/c/18374/7/src/kudu/tserver/tablet_copy_source_session.h@243
PS7, Line 243:   const std::string session_id_;
             :   const std::string requestor_
> nit: these can both be const
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 8
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Tue, 12 Apr 2022 15:16:49 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18374

to look at the new patch set (#8).

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................

[tool] Add tool to copy replica from local filesystem

Add tool to copy replica from local filesystem, it would
be faster than copy from remote peers, it can be use when
add more disk drivers and make data rebalanced faster between
disk drivers, or can be used when migrate data from the only
disk driver to another.
This tool will also make data more dense than data on old
data directories, that means we can save much disk space
and speedup server bootstrap.
We can use the tool like:
kudu local_replica copy_from_local <tablet_id> [-src_fs_wal_dir=<dir>] [-src_fs_metadata_dir=<dir>] [-src_fs_data_dirs=<dirs>] [-dst_fs_wal_dir=<dir>] [-dst_fs_metadata_dir=<dir>] [-dst_fs_data_dirs=<dirs>]

There are some tips to use this tool:
- Using --src_* and --dst_* prefixes to clarify what directories
  are operating on
- The server on the source filesystem must stop before coping
  replicas. Because the tool is standalone, it can't anchor
  replica's data blocks or log segments.
- This tool will not delete data in the source filesystem, you
  should delete data manully if you want, by using
  kudu local_replica delete ...

Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
---
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/master/master_runner.cc
M src/kudu/tablet/tablet_replica.cc
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-test-base.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/tablet_copy_service.cc
M src/kudu/tserver/tablet_copy_service.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_copy_source_session.h
M src/kudu/tserver/ts_tablet_manager.cc
16 files changed, 892 insertions(+), 227 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/18374/8
-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 8
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18374 )

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................


Patch Set 9:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/18374/8/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/18374/8/src/kudu/tools/kudu-tool-test.cc@7886
PS8, Line 7886:   string tablet_id;
> warning: multiple declarations in a single statement reduces readability [r
Done


http://gerrit.cloudera.org:8080/#/c/18374/8/src/kudu/tools/kudu-tool-test.cc@7904
PS8, Line 7904:   }
> warning: multiple declarations in a single statement reduces readability [r
Done


http://gerrit.cloudera.org:8080/#/c/18374/8/src/kudu/tools/kudu-tool-test.cc@7922
PS8, Line 7922: 
> warning: multiple declarations in a single statement reduces readability [r
Done


http://gerrit.cloudera.org:8080/#/c/18374/8/src/kudu/tserver/tablet_copy_client.h
File src/kudu/tserver/tablet_copy_client.h:

http://gerrit.cloudera.org:8080/#/c/18374/8/src/kudu/tserver/tablet_copy_client.h@110
PS8, Line 110:   virtual Status Start(const HostPort& /*copy_source_addr*/,
> warning: parameter 'copy_source_addr' is unused [misc-unused-parameters]
Done


http://gerrit.cloudera.org:8080/#/c/18374/8/src/kudu/tserver/tablet_copy_client.h@111
PS8, Line 111:                        scoped_refptr<tablet::TabletMetadata>* /*meta*/) {
> warning: parameter 'meta' is unused [misc-unused-parameters]
Done


http://gerrit.cloudera.org:8080/#/c/18374/8/src/kudu/tserver/tablet_copy_client.h@116
PS8, Line 116:   virtual Status Start(const std::string& /*tablet_id*/,
> warning: parameter 'tablet_id' is unused [misc-unused-parameters]
Done


http://gerrit.cloudera.org:8080/#/c/18374/8/src/kudu/tserver/tablet_copy_client.h@117
PS8, Line 117:                        scoped_refptr<tablet::TabletMetadata>* /*meta*/) {
> warning: parameter 'meta' is unused [misc-unused-parameters]
Done


http://gerrit.cloudera.org:8080/#/c/18374/8/src/kudu/tserver/tablet_copy_client.h@330
PS8, Line 330:   ~RemoteTabletCopyClient() override;
> warning: annotate this function with 'override' or (rarely) 'final' [modern
Done


http://gerrit.cloudera.org:8080/#/c/18374/8/src/kudu/tserver/tablet_copy_service.cc
File src/kudu/tserver/tablet_copy_service.cc:

http://gerrit.cloudera.org:8080/#/c/18374/8/src/kudu/tserver/tablet_copy_service.cc@269
PS8, Line 269:     RPC_RETURN_NOT_OK(FindSessionUnlocked(session_id, &app_error, &session),
> warning: 2nd function call argument is an uninitialized value [clang-analyz
Done


http://gerrit.cloudera.org:8080/#/c/18374/8/src/kudu/tserver/tablet_copy_service.cc@331
PS8, Line 331:     RPC_RETURN_NOT_OK(DoEndTabletCopySessionUnlocked(req->session_id(), &app_error),
> warning: 2nd function call argument is an uninitialized value [clang-analyz
Done


http://gerrit.cloudera.org:8080/#/c/18374/8/src/kudu/tserver/tablet_copy_source_session.cc
File src/kudu/tserver/tablet_copy_source_session.cc:

http://gerrit.cloudera.org:8080/#/c/18374/8/src/kudu/tserver/tablet_copy_source_session.cc@322
PS8, Line 322:   CHECK(file_info);
> warning: Called C++ object pointer is null [clang-analyzer-core.CallAndMess
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 9
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Tue, 12 Apr 2022 16:34:49 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18374

to look at the new patch set (#10).

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................

[tool] Add tool to copy replica from local filesystem

Add tool to copy replica from local filesystem, it would
be faster than copy from remote peers, it can be use when
add more disk drivers and make data rebalanced faster between
disk drivers, or can be used when migrate data from the only
disk driver to another.
This tool will also make data more dense than data on old
data directories, that means we can save much disk space
and speedup server bootstrap.
We can use the tool like:
kudu local_replica copy_from_local <tablet_id> [-src_fs_wal_dir=<dir>] [-src_fs_metadata_dir=<dir>] [-src_fs_data_dirs=<dirs>] [-dst_fs_wal_dir=<dir>] [-dst_fs_metadata_dir=<dir>] [-dst_fs_data_dirs=<dirs>]

There are some tips to use this tool:
- Using --src_* and --dst_* prefixes to clarify what directories
  are operating on
- The server on the source filesystem must stop before coping
  replicas. Because the tool is standalone, it can't anchor
  replica's data blocks or log segments.
- This tool will not delete data in the source filesystem, you
  should delete data manully if you want, by using
  kudu local_replica delete ...

Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
---
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/master/master_runner.cc
M src/kudu/tablet/tablet_replica.cc
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-test-base.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/tablet_copy_service.cc
M src/kudu/tserver/tablet_copy_service.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_copy_source_session.h
M src/kudu/tserver/ts_tablet_manager.cc
16 files changed, 898 insertions(+), 229 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/18374/10
-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 10
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18374

to look at the new patch set (#7).

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................

[tool] Add tool to copy replica from local filesystem

Add tool to copy replica from local filesystem, it would
be faster than copy from remote peers, it can be use when
add more disk drivers and make data rebalanced faster between
disk drivers, or can be used when migrate data from the only
disk driver to another.
This tool will also make data more dense than data on old
data directories, that means we can save much disk space
and speedup server bootstrap.
We can use the tool like:
kudu local_replica copy_from_local <tablet_id> [-src_fs_wal_dir=<dir>] [-src_fs_metadata_dir=<dir>] [-src_fs_data_dirs=<dirs>] [-dst_fs_wal_dir=<dir>] [-dst_fs_metadata_dir=<dir>] [-dst_fs_data_dirs=<dirs>]

There are some tips to use this tool:
- Using --src_* and --dst_* prefixes to clarify what directories
  are operating on
- The server on the source filesystem must stop before coping
  replicas. Because the tool is standalone, it can't anchor
  replica's data blocks or log segments.
- This tool will not delete data in the source filesystem, you
  should delete data manully if you want, by using
  kudu local_replica delete ...

Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
---
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/master/master_runner.cc
M src/kudu/tablet/tablet_replica.cc
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-test-base.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/tablet_copy_service.cc
M src/kudu/tserver/tablet_copy_service.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_copy_source_session.h
M src/kudu/tserver/ts_tablet_manager.cc
16 files changed, 889 insertions(+), 223 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/18374/7
-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 7
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/18374 )

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................


Patch Set 10:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/18374/7//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18374/7//COMMIT_MSG@21
PS7, Line 21: - Using --src_* and --dst_* prefixes to clarify what directories
> > a user should stand up a new Kudu tserver at the same machine
Thanks for clarifying.

So with the first usage pattern in mind, doesn't destination file system have to have the same UUID? Is that a requirement? Otherwise, once the new tablet server is started, all existing Raft quorums will think the new server is not a part of the quorum.

Regardless, could you add an end-to-end test for the usage pattern you described (copy a few local tablets, then start up the server, and make sure ksck is healthy)

Agreed regarding online process -- definitely trickier to handle races with runtime operations.


http://gerrit.cloudera.org:8080/#/c/18374/10/src/kudu/tserver/tablet_copy_client.cc
File src/kudu/tserver/tablet_copy_client.cc:

http://gerrit.cloudera.org:8080/#/c/18374/10/src/kudu/tserver/tablet_copy_client.cc@1096
PS10, Line 1096: ""
nit: why not set this to something like "local replica <uuid>"?



-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 10
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Tue, 12 Apr 2022 18:30:59 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18374

to look at the new patch set (#15).

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................

[tool] Add tool to copy replica from local filesystem

Add tool to copy replica from local filesystem, it would
be faster than copy from remote peers, it can be use when
add more disk drivers and make data rebalanced faster between
disk drivers, or can be used when migrate data from the only
disk driver to another.
This tool will also make data more dense than data on old
data directories, that means we can save much disk space
and speedup server bootstrap.
We can use the tool like:
kudu local_replica copy_from_local <tablet_id> [-src_fs_wal_dir=<dir>] [-src_fs_metadata_dir=<dir>] [-src_fs_data_dirs=<dirs>] [-dst_fs_wal_dir=<dir>] [-dst_fs_metadata_dir=<dir>] [-dst_fs_data_dirs=<dirs>]

There are some tips to use this tool:
- Using --src_* and --dst_* prefixes to clarify what directories
  are operating on
- The server on the source filesystem must stop before coping
  replicas. Because the tool is standalone, it can't anchor
  replica's data blocks or log segments.
- This tool will not delete data in the source filesystem, you
  should delete data manully if you want, by using
  kudu local_replica delete ...

Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
---
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/master/master_runner.cc
M src/kudu/mini-cluster/internal_mini_cluster.h
M src/kudu/tablet/tablet_replica.cc
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/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
M src/kudu/tserver/tablet_copy-test-base.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/tablet_copy_service.cc
M src/kudu/tserver/tablet_copy_service.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_copy_source_session.h
M src/kudu/tserver/ts_tablet_manager.cc
19 files changed, 1,042 insertions(+), 243 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/18374/15
-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 15
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/18374 )

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................


Patch Set 17:

Thanks for the contribution!


-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 17
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 14 Apr 2022 21:39:02 +0000
Gerrit-HasComments: No

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/18374 )

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................


Patch Set 16: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 16
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 14 Apr 2022 21:38:45 +0000
Gerrit-HasComments: No

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18374

to look at the new patch set (#2).

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................

[tool] Add tool to copy replica from local filesystem

Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
---
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica.h
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/tablet_copy-test-base.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/tablet_copy_service-test.cc
M src/kudu/tserver/tablet_copy_service.cc
M src/kudu/tserver/tablet_copy_service.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_copy_source_session.h
13 files changed, 732 insertions(+), 224 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/18374/2
-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 2
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18374 )

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................


Patch Set 4:

(13 comments)

http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG@7
PS3, Line 7: tool
> It would be great to provide an example of the usage for the tool.
Done


http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG@9
PS3, Line 9: would
> is
Done


http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG@10
PS3, Line 10: use
> used
Done


http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG@10
PS3, Line 10: copy
> copying
Done


http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG@11
PS3, Line 11: add
> adding
Done


http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG@11
PS3, Line 11: disk drivers
> data directories
Done


http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG@12
PS3, Line 12: ta from the
> data directory
Done


http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG@12
PS3, Line 12: disk drivers
> them
Done


http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tablet/tablet_replica.h
File src/kudu/tablet/tablet_replica.h:

http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tablet/tablet_replica.h@115
PS3, Line 115:                 Th
> If this is a constructor for a limited audience, consider moving this into 
Done


http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tserver/tablet_copy_client.h
File src/kudu/tserver/tablet_copy_client.h:

http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tserver/tablet_copy_client.h@316
PS3, Line 316: opy_me
> nit: to be inline with tablet_copy_source_metrics_ and dst_fs_manager_, sho
Done


http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tserver/tablet_copy_client.cc
File src/kudu/tserver/tablet_copy_client.cc:

http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tserver/tablet_copy_client.cc@880
PS3, Line 880:    case TabletCopyMode::REMOTE:
             :       RETURN_NOT_OK_PREPEND(DownloadFile(data_id, writer.get()),
             :                             Substitute("Unable to download WAL segment with seq. number $0",
             :                                        wal_segment_seqno));
             :       break;
             :     case TabletCopyMode::LOCAL:
             :       RETURN_NOT_OK_PREPEND(CopyFile(data_id, writer.get()),
             :                             Substitute("Unable to copy WAL segment with seq. number $0",
             :                                        wal_segment_seqno));
             :    
> nit: maybe, change this if/else to switch?
Done


http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tserver/tablet_copy_client.cc@972
PS3, Line 972:   DataIdPB data_id;
             :   data_id.set_type(DataIdPB::BLOCK);
             :   old_block_id.CopyToPB(data_id.mutable_block_id());
             :   switch (mode_) {
             :     case TabletCopyMode::REMOTE:
             :       RETURN_NOT_OK_PREPEND(DownloadFile(data_id, block.get()),
             :                             Substitute("Unable to download block $0", old_block_id.ToString()));
             :    
> nit: maybe, change this if/else to switch?
Done


http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tserver/tablet_copy_client.cc@1067
PS3, Line 1067:           segment_seqno, offset, client_maxlen, &data,
> nit: maybe, move this one line below, and then there is no need to compute 
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 4
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Sat, 02 Apr 2022 07:30:23 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18374

to look at the new patch set (#4).

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................

[tool] Add tool to copy replica from local filesystem

Add tool to copy replica from local filesystem, it would
be faster than copy from remote peers, it can be use when
add more disk drivers and make data rebalanced faster between
disk drivers, or can be used when migrate data from the only
disk driver to another.
This tool will also make data more dense than data on old
data directories, that means we can save much disk space
and speedup server bootstrap.
We can use the tool like:
kudu local_replica copy_from_local <tablet_id> [-src_fs_wal_dir=<dir>] [-src_fs_metadata_dir=<dir>] [-src_fs_data_dirs=<dirs>] [-dst_fs_wal_dir=<dir>] [-dst_fs_metadata_dir=<dir>] [-dst_fs_data_dirs=<dirs>]

There are some tips to use this tool:
- Using --src_* and --dst_* prefixes to clarify what directories
  are operating on
- The server on the source filesystem must stop before coping
  replicas. Because the tool is standalone, it can't anchor
  replica's data blocks or log segments.
- This tool will not delete data in the source filesystem, you
  should delete data manully if you want, by using
  kudu local_replica delete ...

Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
---
M src/kudu/tablet/tablet_replica.cc
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-test-base.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/tablet_copy_service-test.cc
M src/kudu/tserver/tablet_copy_service.cc
M src/kudu/tserver/tablet_copy_service.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_copy_source_session.h
14 files changed, 826 insertions(+), 230 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/18374/4
-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 4
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18374 )

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tserver/tablet_copy_service.h
File src/kudu/tserver/tablet_copy_service.h:

PS3: 
> Could you separate changes on these tablet_copy_service.{h,cc} into its own
OK, I created another CR to do this work, https://gerrit.cloudera.org/c/18379/


http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tserver/tablet_copy_source_session.h
File src/kudu/tserver/tablet_copy_source_session.h:

PS3: 
> Instead of introducing the TabletCopyMode enum, did you consider an option 
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 6
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Sun, 03 Apr 2022 15:27:44 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18374

to look at the new patch set (#11).

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................

[tool] Add tool to copy replica from local filesystem

Add tool to copy replica from local filesystem, it would
be faster than copy from remote peers, it can be use when
add more disk drivers and make data rebalanced faster between
disk drivers, or can be used when migrate data from the only
disk driver to another.
This tool will also make data more dense than data on old
data directories, that means we can save much disk space
and speedup server bootstrap.
We can use the tool like:
kudu local_replica copy_from_local <tablet_id> [-src_fs_wal_dir=<dir>] [-src_fs_metadata_dir=<dir>] [-src_fs_data_dirs=<dirs>] [-dst_fs_wal_dir=<dir>] [-dst_fs_metadata_dir=<dir>] [-dst_fs_data_dirs=<dirs>]

There are some tips to use this tool:
- Using --src_* and --dst_* prefixes to clarify what directories
  are operating on
- The server on the source filesystem must stop before coping
  replicas. Because the tool is standalone, it can't anchor
  replica's data blocks or log segments.
- This tool will not delete data in the source filesystem, you
  should delete data manully if you want, by using
  kudu local_replica delete ...

Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
---
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/master/master_runner.cc
M src/kudu/mini-cluster/internal_mini_cluster.h
M src/kudu/tablet/tablet_replica.cc
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/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
M src/kudu/tserver/tablet_copy-test-base.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/tablet_copy_service.cc
M src/kudu/tserver/tablet_copy_service.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_copy_source_session.h
M src/kudu/tserver/ts_tablet_manager.cc
19 files changed, 1,039 insertions(+), 243 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/18374/11
-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 11
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [tool] Add tool to copy replica from local filesystem

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18374

to look at the new patch set (#12).

Change subject: [tool] Add tool to copy replica from local filesystem
......................................................................

[tool] Add tool to copy replica from local filesystem

Add tool to copy replica from local filesystem, it would
be faster than copy from remote peers, it can be use when
add more disk drivers and make data rebalanced faster between
disk drivers, or can be used when migrate data from the only
disk driver to another.
This tool will also make data more dense than data on old
data directories, that means we can save much disk space
and speedup server bootstrap.
We can use the tool like:
kudu local_replica copy_from_local <tablet_id> [-src_fs_wal_dir=<dir>] [-src_fs_metadata_dir=<dir>] [-src_fs_data_dirs=<dirs>] [-dst_fs_wal_dir=<dir>] [-dst_fs_metadata_dir=<dir>] [-dst_fs_data_dirs=<dirs>]

There are some tips to use this tool:
- Using --src_* and --dst_* prefixes to clarify what directories
  are operating on
- The server on the source filesystem must stop before coping
  replicas. Because the tool is standalone, it can't anchor
  replica's data blocks or log segments.
- This tool will not delete data in the source filesystem, you
  should delete data manully if you want, by using
  kudu local_replica delete ...

Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
---
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/master/master_runner.cc
M src/kudu/mini-cluster/internal_mini_cluster.h
M src/kudu/tablet/tablet_replica.cc
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/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
M src/kudu/tserver/tablet_copy-test-base.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/tablet_copy_service.cc
M src/kudu/tserver/tablet_copy_service.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_copy_source_session.h
M src/kudu/tserver/ts_tablet_manager.cc
19 files changed, 1,040 insertions(+), 243 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/18374/12
-- 
To view, visit http://gerrit.cloudera.org:8080/18374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95
Gerrit-Change-Number: 18374
Gerrit-PatchSet: 12
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>