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/08/03 11:28:19 UTC
[kudu-CR] [tools] Add --fault tolerant to 'kudu table copy/scan'
Yingchun Lai has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18813
Change subject: [tools] Add --fault_tolerant to 'kudu table copy/scan'
......................................................................
[tools] Add --fault_tolerant to 'kudu table copy/scan'
Add --fault_tolerant to 'kudu table copy/scan' to make
the results are returned in primary key order and make
the scanner fault-tolerant. This patch also adds some
other flags missing in 'kudu table copy'
Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0
---
M src/kudu/client/scan_configuration.cc
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/tool_action_table.cc
3 files changed, 20 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/13/18813/1
--
To view, visit http://gerrit.cloudera.org:8080/18813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0
Gerrit-Change-Number: 18813
Gerrit-PatchSet: 1
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [tools] Add --fault tolerant to some CLI tools
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18813 )
Change subject: [tools] Add --fault_tolerant to some CLI tools
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18813/6/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/18813/6/src/kudu/tools/kudu-tool-test.cc@5055
PS6, Line 5055: TEST_F(ToolTest, TableScanFaultTolerant) {
> nit: Maybe we should test if the tool can recover from tablet failures or r
IMO it's not necessary, it's just a flag for an existing feature, not a new feature.
--
To view, visit http://gerrit.cloudera.org:8080/18813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0
Gerrit-Change-Number: 18813
Gerrit-PatchSet: 6
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 02 Sep 2022 17:17:35 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] Add --fault tolerant to 'kudu table copy/scan'
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18813 )
Change subject: [tools] Add --fault_tolerant to 'kudu table copy/scan'
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/table_scanner.cc@676
PS1, Line 676: // TODO(yingchun): push down this judgement to ScanConfiguration::SetFaultTolerant
Should we just keep this check inside TableScanner?
--
To view, visit http://gerrit.cloudera.org:8080/18813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0
Gerrit-Change-Number: 18813
Gerrit-PatchSet: 1
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Mon, 29 Aug 2022 06:53:48 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] Add --fault tolerant to 'kudu table copy/scan'
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yifan Zhang, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18813
to look at the new patch set (#2).
Change subject: [tools] Add --fault_tolerant to 'kudu table copy/scan'
......................................................................
[tools] Add --fault_tolerant to 'kudu table copy/scan'
Add --fault_tolerant to 'kudu table copy/scan' to make
the results returned in primary key order and make the
scanner fault-tolerant. This patch also adds some other
flags missing in 'kudu table copy'
Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0
---
M src/kudu/client/scan_configuration.cc
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/tool_action_table.cc
3 files changed, 20 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/13/18813/2
--
To view, visit http://gerrit.cloudera.org:8080/18813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0
Gerrit-Change-Number: 18813
Gerrit-PatchSet: 2
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [tools] Add --fault tolerant to 'kudu table copy/scan'
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18813 )
Change subject: [tools] Add --fault_tolerant to 'kudu table copy/scan'
......................................................................
Patch Set 3:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/18813/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18813/1//COMMIT_MSG@10
PS1, Line 10: ret
> drop 'are'
Done
http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/table_scanner.cc@102
PS1, Line 102: t another tablet ser
> in primary key order for a single tablet ?
Done
http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/table_scanner.cc@103
PS1, Line 103: cans typically have lower throughput than non fault-tolerant scans, "
: "but the results are returned in primary ke
> I guess this part should come first in the description. The fact that the
Done
http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/table_scanner.cc@676
PS1, Line 676: // TODO(yingchun): push down this judgement to ScanConfiguration::SetFaultTolerant
> Should we just keep this check inside TableScanner?
No, I think it should be validated it in ScanConfiguration internally, then everywhere call ScanConfiguration::SetFaultTolerant can validate it. SetFaultTolerant may overwrite the read mode which is set by user explicitly, that may change what the user's intention. And the behavior of calling SetFaultTolerant first, then calling SetReadMode maybe undefined.
http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/table_scanner.cc@676
PS1, Line 676: // TODO(yingchun): push down this judgement to ScanConfiguration::SetFaultTolerant
> I'm not sure I understand how ScanConfiguration() could have such a respons
All ScanConfiguration::Set* has a Status return value, so it is able to return error status when attempt to set invalid options. Now that it return Status, it maybe non-ok status.
http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/table_scanner.cc@678
PS1, Line 678: conflicts w
> conflicts
Done
http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
PS1:
> If adding new flags for the kudu CLI tool, please update corresponding test
Done
--
To view, visit http://gerrit.cloudera.org:8080/18813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0
Gerrit-Change-Number: 18813
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 31 Aug 2022 12:56:48 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] Add --fault tolerant to some CLI tools
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18813 )
Change subject: [tools] Add --fault_tolerant to some CLI tools
......................................................................
Patch Set 6: Verified+1
(1 comment)
Unrelated build fluke (RELEASE): it might be something related to workspace pollution (due to the upgrade to googletest 1.12.1, propably?)
http://gerrit.cloudera.org:8080/#/c/18813/6/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/18813/6/src/kudu/tools/kudu-tool-test.cc@5055
PS6, Line 5055: TEST_F(ToolTest, TableScanFaultTolerant) {
> IMO it's not necessary, it's just a flag for an existing feature, not a new
Yep, I guess that would be an exercise on its own :)
--
To view, visit http://gerrit.cloudera.org:8080/18813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0
Gerrit-Change-Number: 18813
Gerrit-PatchSet: 6
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Sat, 03 Sep 2022 04:41:23 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] Add --fault tolerant to some CLI tools
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yifan Zhang, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18813
to look at the new patch set (#6).
Change subject: [tools] Add --fault_tolerant to some CLI tools
......................................................................
[tools] Add --fault_tolerant to some CLI tools
Add --fault_tolerant to 'kudu table copy/scan' and
'kudu perf table_scan' to make the scanner
fault-tolerant and the results returned in primary
key order per-tablet. This patch also adds some other
flags missing in 'kudu table copy'.
Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0
---
M src/kudu/client/scan_configuration.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/tool_action_perf.cc
M src/kudu/tools/tool_action_table.cc
5 files changed, 97 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/13/18813/6
--
To view, visit http://gerrit.cloudera.org:8080/18813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0
Gerrit-Change-Number: 18813
Gerrit-PatchSet: 6
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [tools] Add --fault tolerant to some CLI tools
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yifan Zhang, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18813
to look at the new patch set (#5).
Change subject: [tools] Add --fault_tolerant to some CLI tools
......................................................................
[tools] Add --fault_tolerant to some CLI tools
Add --fault_tolerant to 'kudu table copy/scan' and
'kudu perf table_scan' to make the scanner
fault-tolerant and the results returned in primary
key order per-tablet. This patch also adds some other
flags missing in 'kudu table copy'.
Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0
---
M src/kudu/client/scan_configuration.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/tool_action_perf.cc
M src/kudu/tools/tool_action_table.cc
5 files changed, 90 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/13/18813/5
--
To view, visit http://gerrit.cloudera.org:8080/18813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0
Gerrit-Change-Number: 18813
Gerrit-PatchSet: 5
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [tools] Add --fault tolerant to 'kudu table copy/scan'
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18813 )
Change subject: [tools] Add --fault_tolerant to 'kudu table copy/scan'
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
PS1:
> I don't see those tests updated in PS4 -- am I missing something?
In https://gerrit.cloudera.org/c/18813/4/src/kudu/tools/kudu-tool-test.cc#3697
--
To view, visit http://gerrit.cloudera.org:8080/18813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0
Gerrit-Change-Number: 18813
Gerrit-PatchSet: 4
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 31 Aug 2022 17:17:20 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] Add --fault tolerant to some CLI tools
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18813 )
Change subject: [tools] Add --fault_tolerant to some CLI tools
......................................................................
Patch Set 6: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18813/6/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/18813/6/src/kudu/tools/kudu-tool-test.cc@5055
PS6, Line 5055: TEST_F(ToolTest, TableScanFaultTolerant) {
nit: Maybe we should test if the tool can recover from tablet failures or return in primary key order per tablet?
--
To view, visit http://gerrit.cloudera.org:8080/18813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0
Gerrit-Change-Number: 18813
Gerrit-PatchSet: 6
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 02 Sep 2022 13:01:39 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] Add --fault tolerant to 'kudu table copy/scan'
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18813 )
Change subject: [tools] Add --fault_tolerant to 'kudu table copy/scan'
......................................................................
Patch Set 1:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/18813/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18813/1//COMMIT_MSG@10
PS1, Line 10: are
drop 'are'
http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/table_scanner.cc@102
PS1, Line 102: in primary key order
in primary key order for a single tablet ?
http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/table_scanner.cc@103
PS1, Line 103: This also means the scanner can recover if the server it "
: "is scanning fails in the middle of a scan.
I guess this part should come first in the description. The fact that the per-tablet results come in primary key order is just an extra feature, right?
http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/table_scanner.cc@676
PS1, Line 676: // TODO(yingchun): push down this judgement to ScanConfiguration::SetFaultTolerant
I'm not sure I understand how ScanConfiguration() could have such a responsibility -- it has only the mode that it given to be set.
http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/table_scanner.cc@678
PS1, Line 678: is conflict
conflicts
http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
PS1:
If adding new flags for the kudu CLI tool, please update corresponding tests.
--
To view, visit http://gerrit.cloudera.org:8080/18813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0
Gerrit-Change-Number: 18813
Gerrit-PatchSet: 1
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Mon, 29 Aug 2022 19:49:54 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] Add --fault tolerant to 'kudu table copy/scan'
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yifan Zhang, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18813
to look at the new patch set (#3).
Change subject: [tools] Add --fault_tolerant to 'kudu table copy/scan'
......................................................................
[tools] Add --fault_tolerant to 'kudu table copy/scan'
Add --fault_tolerant to 'kudu table copy/scan' to make
the results returned in primary key order and make the
scanner fault-tolerant. This patch also adds some other
flags missing in 'kudu table copy'
Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0
---
M src/kudu/client/scan_configuration.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/tool_action_table.cc
4 files changed, 42 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/13/18813/3
--
To view, visit http://gerrit.cloudera.org:8080/18813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0
Gerrit-Change-Number: 18813
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [tools] Add --fault tolerant to 'kudu table copy/scan'
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18813 )
Change subject: [tools] Add --fault_tolerant to 'kudu table copy/scan'
......................................................................
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/18813/4/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/18813/4/src/kudu/tools/kudu-tool-test.cc@3696
PS4, Line 3696: cluster_->master()->bound_rpc_addr().ToString(), kTableName),
Could you also add test coverage for the `kudu table copy` tool since the new flags have been added there as well?
http://gerrit.cloudera.org:8080/#/c/18813/4/src/kudu/tools/kudu-tool-test.cc@3710
PS4, Line 3710:
I'm not sure this exactly the test coverage assumed: as far as I can see, this patch reads and applies FLAGS_fault_tolerant for `kudu table scan` sub-command, not for `kudu perf table_scan`.
With that, could you also update the `kudu perf table_scan` to report --fault_tolerant as a supported flag?
And add a test for `kudu table scan` using the --fault_tolerant flag?
http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/table_scanner.cc@676
PS1, Line 676: // TODO(yingchun): push down this judgement to ScanConfiguration::SetFaultTolerant
> No, I think it should be validated it in ScanConfiguration internally, then
OK, that sounds better to me: basically the idea is to check whether the explicit setting of the read mode for a scanner and calling SetFaultTolerant() contradict each other.
http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
PS1:
> In https://gerrit.cloudera.org/c/18813/4/src/kudu/tools/kudu-tool-test.cc#3
Ah, right, but that's only for the 'perf' sub-command as of now?
Could you please also add coverage for the rest of sub-commands updated in this patch? Thanks!
--
To view, visit http://gerrit.cloudera.org:8080/18813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0
Gerrit-Change-Number: 18813
Gerrit-PatchSet: 1
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 31 Aug 2022 19:13:49 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] Add --fault tolerant to 'kudu table copy/scan'
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18813 )
Change subject: [tools] Add --fault_tolerant to 'kudu table copy/scan'
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
PS1:
> Done
I don't see those tests updated in PS4 -- am I missing something?
--
To view, visit http://gerrit.cloudera.org:8080/18813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0
Gerrit-Change-Number: 18813
Gerrit-PatchSet: 1
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 31 Aug 2022 17:05:06 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] Add --fault tolerant to some CLI tools
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18813 )
Change subject: [tools] Add --fault_tolerant to some CLI tools
......................................................................
[tools] Add --fault_tolerant to some CLI tools
Add --fault_tolerant to 'kudu table copy/scan' and
'kudu perf table_scan' to make the scanner
fault-tolerant and the results returned in primary
key order per-tablet. This patch also adds some other
flags missing in 'kudu table copy'.
Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0
Reviewed-on: http://gerrit.cloudera.org:8080/18813
Reviewed-by: Yifan Zhang <ch...@163.com>
Tested-by: Alexey Serbin <al...@apache.org>
Reviewed-by: Alexey Serbin <al...@apache.org>
---
M src/kudu/client/scan_configuration.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/tool_action_perf.cc
M src/kudu/tools/tool_action_table.cc
5 files changed, 97 insertions(+), 2 deletions(-)
Approvals:
Yifan Zhang: Looks good to me, but someone else must approve
Alexey Serbin: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/18813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0
Gerrit-Change-Number: 18813
Gerrit-PatchSet: 7
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [tools] Add --fault tolerant to some CLI tools
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18813 )
Change subject: [tools] Add --fault_tolerant to some CLI tools
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/18813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0
Gerrit-Change-Number: 18813
Gerrit-PatchSet: 6
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Sat, 03 Sep 2022 04:41:31 +0000
Gerrit-HasComments: No
[kudu-CR] [tools] Add --fault tolerant to some CLI tools
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.
Change subject: [tools] Add --fault_tolerant to some CLI tools
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/18813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0
Gerrit-Change-Number: 18813
Gerrit-PatchSet: 6
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [tools] Add --fault tolerant to some CLI tools
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18813 )
Change subject: [tools] Add --fault_tolerant to some CLI tools
......................................................................
Patch Set 6:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/18813/4/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/18813/4/src/kudu/tools/kudu-tool-test.cc@3696
PS4, Line 3696:
> Could you also add test coverage for the `kudu table copy` tool since the n
Done
http://gerrit.cloudera.org:8080/#/c/18813/4/src/kudu/tools/kudu-tool-test.cc@3710
PS4, Line 3710: --fault_tolerant
> I'm not sure this exactly the test coverage assumed: as far as I can see, t
You're right, I miss it.
Done
http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
PS1:
> Ah, right, but that's only for the 'perf' sub-command as of now?
Done
--
To view, visit http://gerrit.cloudera.org:8080/18813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0
Gerrit-Change-Number: 18813
Gerrit-PatchSet: 6
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 02 Sep 2022 09:17:35 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] Add --fault tolerant to 'kudu table copy/scan'
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yifan Zhang, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18813
to look at the new patch set (#4).
Change subject: [tools] Add --fault_tolerant to 'kudu table copy/scan'
......................................................................
[tools] Add --fault_tolerant to 'kudu table copy/scan'
Add --fault_tolerant to 'kudu table copy/scan' to make
the results returned in primary key order and make the
scanner fault-tolerant. This patch also adds some other
flags missing in 'kudu table copy'
Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0
---
M src/kudu/client/scan_configuration.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/tool_action_table.cc
4 files changed, 44 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/13/18813/4
--
To view, visit http://gerrit.cloudera.org:8080/18813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0
Gerrit-Change-Number: 18813
Gerrit-PatchSet: 4
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>