You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Yifan Zhang (Code Review)" <ge...@cloudera.org> on 2019/07/27 09:57:55 UTC
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Yifan Zhang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13937
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
[ksck] Filter tables and tablets in KsckCluster
The ksck tool executed slowly if there are too many tables in a cluster,
move table_name_filters and tablet_id_filters to KsckCluster and only check
filtered cluster will speed up the execution of ksck.
Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_checksum.cc
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/rebalancer.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_replica_util.cc
8 files changed, 78 insertions(+), 49 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/13937/1
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang <ch...@163.com>
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/13937 )
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
Patch Set 8:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13937/7/src/kudu/tools/ksck.h
File src/kudu/tools/ksck.h:
http://gerrit.cloudera.org:8080/#/c/13937/7/src/kudu/tools/ksck.h@452
PS7, Line 452: // Setters for filtering the tables/tablets to be checked.
: // Equivalent to the same functions in class 'Ksck'.
: void set_table_filters(std::vector<std::string> table_names) {
: table_filters_ = std::move(table_names);
: }
:
: // See above.
: void set_tablet_id_filters(std::vector<std::string> tablet_ids) {
: tablet_id_filters_ = std::move(tablet_ids);
> Rather than duplicating all of this, could you just refer to the equivalent
Done
http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck_checksum.cc
File src/kudu/tools/ksck_checksum.cc:
http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck_checksum.cc@502
PS5, Line 502: msg += " Filter:";
> Yeah I'm not sure I understand either; it seems like num_replicas_tmp shoul
I tested several cases and finally got the following results:
If there are tablets in a cluster and all tablets filtered out, the result is 'No tablet replicas found' error.
If there are table(s) but no tablet in a cluster(i.e. all range partitions have been dropped), the result is 'OK'.
The comment 'Warn if the table has tablets, but no replicas.' is confusing, is it possible 'the table has tablets but no replicas'?
However in this patch, the 'cluster_' here is not complete, it only includes unfiltered tables/tablets, so in both cases the checksum result is 'No tablet replicas found' error. I think it's not necessary to distinguish between the two cases.
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 8
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Mon, 05 Aug 2019 04:18:24 +0000
Gerrit-HasComments: Yes
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/13937 )
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 5
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Thu, 01 Aug 2019 09:41:11 +0000
Gerrit-HasComments: No
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13937 )
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
Patch Set 12: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 12
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Tue, 13 Aug 2019 18:46:09 +0000
Gerrit-HasComments: No
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13937
to look at the new patch set (#4).
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
[ksck] Filter tables and tablets in KsckCluster
The ksck tool executes slowly if there are too many tables in a cluster,
move table_name_filters and tablet_id_filters to KsckCluster and only check
filtered cluster would speed up the execution of ksck.
Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_checksum.cc
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/rebalancer.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_replica_util.cc
8 files changed, 78 insertions(+), 52 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/13937/4
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 4
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/13937 )
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
Patch Set 10:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/13937/9/src/kudu/tools/ksck.h
File src/kudu/tools/ksck.h:
http://gerrit.cloudera.org:8080/#/c/13937/9/src/kudu/tools/ksck.h@471
PS9, Line 471: int filtered_tables_count() const {
: return filtered_tables_count_;
: }
:
: int filtered_tablets_count() const {
: return filtered_tablets_count_;
: }
> Value types (i.e. int, double, etc.) should be returned by value rather tha
Done
http://gerrit.cloudera.org:8080/#/c/13937/9/src/kudu/tools/ksck.h@489
PS9, Line 489: int filtered_tables_count_;
: int filtered_tablets_count_;
> Nit: 'int' is fine too.
Done
http://gerrit.cloudera.org:8080/#/c/13937/9/src/kudu/tools/ksck_checksum.cc
File src/kudu/tools/ksck_checksum.cc:
http://gerrit.cloudera.org:8080/#/c/13937/9/src/kudu/tools/ksck_checksum.cc@491
PS9, Line 491: &&
> && n
Done
http://gerrit.cloudera.org:8080/#/c/13937/9/src/kudu/tools/ksck_remote.cc
File src/kudu/tools/ksck_remote.cc:
http://gerrit.cloudera.org:8080/#/c/13937/9/src/kudu/tools/ksck_remote.cc@526
PS9, Line 526: filtered_tables_count_ = 0;
> Should probably also initialize these new variables to 0 in the constructor
Done
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 10
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Wed, 07 Aug 2019 08:09:26 +0000
Gerrit-HasComments: Yes
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13937
to look at the new patch set (#2).
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
[ksck] Filter tables and tablets in KsckCluster
The ksck tool executed slowly if there are too many tables in a cluster,
move table_name_filters and tablet_id_filters to KsckCluster and only check
filtered cluster will speed up the execution of ksck.
Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_checksum.cc
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/rebalancer.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_replica_util.cc
8 files changed, 78 insertions(+), 50 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/13937/2
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13937
to look at the new patch set (#6).
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
[ksck] Filter tables and tablets in KsckCluster
The ksck tool executes slowly if there are too many tables in a cluster,
move table_name_filters and tablet_id_filters to KsckCluster and only check
filtered tables and tablets would speed up the execution of ksck tool
with '--tables=<tables>' or '--tablets=<tablets>' args.
Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_checksum.cc
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/rebalancer.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_replica_util.cc
8 files changed, 80 insertions(+), 39 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/13937/6
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 6
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/13937 )
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
Patch Set 11:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13937/11/src/kudu/tools/ksck-test.cc
File src/kudu/tools/ksck-test.cc:
http://gerrit.cloudera.org:8080/#/c/13937/11/src/kudu/tools/ksck-test.cc@1244
PS11, Line 1244: cluster_->set_table_filters({"xyz"});
> Table filter and tablet filter are now performed in KsckCluster class, not
Yes.
http://gerrit.cloudera.org:8080/#/c/13937/11/src/kudu/tools/ksck_remote.cc
File src/kudu/tools/ksck_remote.cc:
http://gerrit.cloudera.org:8080/#/c/13937/11/src/kudu/tools/ksck_remote.cc@580
PS11, Line 580: Status RemoteKsckCluster::RetrieveTabletsList(const shared_ptr<KsckTable>& table) {
> The logic for filtered_tablets_count_ here is incorrect because RetrieveTab
RemoteKsckCluster is used by CLI tool, but MockKsckCluster is used in unit tests.
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 11
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Tue, 13 Aug 2019 06:28:17 +0000
Gerrit-HasComments: Yes
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13937
to look at the new patch set (#12).
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
[ksck] Filter tables and tablets in KsckCluster
The ksck tool executes slowly if there are too many tables in a cluster
even if we just want to check some specific tables and tablets.
Filtering tables and tablets in KsckCluster and only tracking specified
tables and tablets would speed up the execution of ksck tool with
'--tables=<tables>' and '--tablets=<tablets>' args.
Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_checksum.cc
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/rebalancer.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_replica_util.cc
8 files changed, 122 insertions(+), 40 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/13937/12
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 12
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13937
to look at the new patch set (#8).
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
[ksck] Filter tables and tablets in KsckCluster
The ksck tool executes slowly if there are too many tables in a cluster,
move table_name_filters and tablet_id_filters to KsckCluster and only check
filtered tables and tablets would speed up the execution of ksck tool
with '--tables=<tables>' or '--tablets=<tablets>' args.
Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_checksum.cc
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/rebalancer.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_replica_util.cc
8 files changed, 73 insertions(+), 38 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/13937/8
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 8
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13937
to look at the new patch set (#5).
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
[ksck] Filter tables and tablets in KsckCluster
The ksck tool executes slowly if there are too many tables in a cluster,
move table_name_filters and tablet_id_filters to KsckCluster and only check
filtered cluster would speed up the execution of ksck.
Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_checksum.cc
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/rebalancer.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_replica_util.cc
8 files changed, 78 insertions(+), 52 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/13937/5
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 5
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13937 )
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
[ksck] Filter tables and tablets in KsckCluster
The ksck tool executes slowly if there are too many tables in a cluster
even if we just want to check some specific tables and tablets.
Filtering tables and tablets in KsckCluster and only tracking specified
tables and tablets would speed up the execution of ksck tool with
'--tables=<tables>' and '--tablets=<tablets>' args.
Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Reviewed-on: http://gerrit.cloudera.org:8080/13937
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_checksum.cc
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/rebalancer.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_replica_util.cc
8 files changed, 122 insertions(+), 40 deletions(-)
Approvals:
Kudu Jenkins: Verified
Adar Dembo: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 13
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13937
to look at the new patch set (#11).
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
[ksck] Filter tables and tablets in KsckCluster
The ksck tool executes slowly if there are too many tables in a cluster
even if we just want to check some specific tables and tablets.
Filtering tables and tablets in KsckCluster and only tracking specified
tables and tablets would speed up the execution of ksck tool with
'--tables=<tables>' and '--tablets=<tablets>' args.
Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_checksum.cc
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/rebalancer.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_replica_util.cc
8 files changed, 120 insertions(+), 40 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/13937/11
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 11
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/13937 )
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck_checksum.cc
File src/kudu/tools/ksck_checksum.cc:
http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck_checksum.cc@502
PS5, Line 502: // The table may have no tablets if all range partitions have been dropped.
> Why should this be an error? If there were tablets but the user filtered them all out, shouldn't this be Status::OK?
Maybe the error could tell users they set wrong filters.
> Would it be difficult to add tracking of the list of filtered tables/tablets in KsckCluster, and then use it to differentiate between the two cases here?
Well, if we do this we actually track all tables and tablets, which was same as before. But the idea of this patch is to track less tables and tablets in KsckCluster to accelerate the execution of ksck tool.
However, in ordered to differentiate between the two cases here, I added two variables named 'filtered_tables_count' and 'filtered_tablets_count' to KsckCluster. Besides I added some unit tests to coverage different cases, including the cluster has no table, the cluster has tables but all tables filtered out, the cluster has a table with no tablet, the cluster has a table with tablets but all tablets filtered out.
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 9
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Tue, 06 Aug 2019 11:48:00 +0000
Gerrit-HasComments: Yes
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/13937 )
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
Patch Set 12:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13937/11/src/kudu/tools/ksck_remote.cc
File src/kudu/tools/ksck_remote.cc:
http://gerrit.cloudera.org:8080/#/c/13937/11/src/kudu/tools/ksck_remote.cc@525
PS11, Line 525: int tables_count = 0;
> Nit: no real reason for this to be unsigned; could you just make it regular
Done
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 12
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Tue, 13 Aug 2019 10:14:23 +0000
Gerrit-HasComments: Yes
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13937 )
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
Patch Set 11:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13937/11/src/kudu/tools/ksck_remote.cc
File src/kudu/tools/ksck_remote.cc:
http://gerrit.cloudera.org:8080/#/c/13937/11/src/kudu/tools/ksck_remote.cc@525
PS11, Line 525: uint32_t tables_count = 0;
Nit: no real reason for this to be unsigned; could you just make it regular int?
http://gerrit.cloudera.org:8080/#/c/13937/11/src/kudu/tools/ksck_remote.cc@580
PS11, Line 580: Status RemoteKsckCluster::RetrieveTabletsList(const shared_ptr<KsckTable>& table) {
The logic for filtered_tablets_count_ here is incorrect because RetrieveTabletsList is called _for each table_. Maybe you meant to initialize filtered_tablets_count_ to 0 in RetrieveAllTablets?
Also curious why this wasn't caught by a unit test. Do we never exercise the parallelism here?
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 11
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Tue, 13 Aug 2019 05:25:30 +0000
Gerrit-HasComments: Yes
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13937
to look at the new patch set (#7).
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
[ksck] Filter tables and tablets in KsckCluster
The ksck tool executes slowly if there are too many tables in a cluster,
move table_name_filters and tablet_id_filters to KsckCluster and only check
filtered tables and tablets would speed up the execution of ksck tool
with '--tables=<tables>' or '--tablets=<tablets>' args.
Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_checksum.cc
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/rebalancer.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_replica_util.cc
8 files changed, 80 insertions(+), 38 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/13937/7
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 7
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13937 )
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
Patch Set 8:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck_checksum.cc
File src/kudu/tools/ksck_checksum.cc:
http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck_checksum.cc@502
PS5, Line 502: msg += " Filter:";
> If there are tablets in a cluster and all tablets filtered out, the result is 'No tablet replicas found' error.
Why should this be an error? If there were tablets but the user filtered them all out, shouldn't this be Status::OK?
> If there are table(s) but no tablet in a cluster(i.e. all range partitions have been dropped), the result is 'OK'.
Agreed.
> The comment 'Warn if the table has tablets, but no replicas.' is confusing, is it possible 'the table has tablets but no replicas'?
No, that doesn't seem possible.
> However in this patch, the 'cluster_' here is not complete, it only includes unfiltered tables/tablets, so in both cases the checksum result is 'No tablet replicas found' error. I think it's not necessary to distinguish between the two cases.
Would it be difficult to add tracking of the list of filtered tables/tablets in KsckCluster, and then use it to differentiate between the two cases here?
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 8
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Mon, 05 Aug 2019 22:29:55 +0000
Gerrit-HasComments: Yes
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13937
to look at the new patch set (#3).
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
[ksck] Filter tables and tablets in KsckCluster
The ksck tool executes slowly if there are too many tables in a cluster,
move table_name_filters and tablet_id_filters to KsckCluster and only check
filtered cluster would speed up the execution of ksck.
Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_checksum.cc
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/rebalancer.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_replica_util.cc
8 files changed, 76 insertions(+), 50 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/13937/3
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/13937 )
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
Patch Set 11:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13937/11/src/kudu/tools/ksck-test.cc
File src/kudu/tools/ksck-test.cc:
http://gerrit.cloudera.org:8080/#/c/13937/11/src/kudu/tools/ksck-test.cc@1244
PS11, Line 1244: cluster_->set_table_filters({"xyz"});
Table filter and tablet filter are now performed in KsckCluster class, not Ksck class. And unit tests are testing MockKsckCluster, but RemoteKsckCluster class which is used by CLI tool hasn't been tested, right?
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 11
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Tue, 13 Aug 2019 06:03:37 +0000
Gerrit-HasComments: Yes
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13937
to look at the new patch set (#9).
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
[ksck] Filter tables and tablets in KsckCluster
The ksck tool executes slowly if there are too many tables in a cluster
even if we just want to check some specific tables and tablets.
Filtering tables and tablets in KsckCluster and only tracking specified
tables and tablets would speed up the execution of ksck tool with
'--tables=<tables>' and '--tablets=<tablets>' args.
Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_checksum.cc
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/rebalancer.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_replica_util.cc
8 files changed, 119 insertions(+), 39 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/13937/9
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 9
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/13937 )
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
Patch Set 5: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 5
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Thu, 01 Aug 2019 09:41:27 +0000
Gerrit-HasComments: No
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/13937 )
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13937/3/src/kudu/tools/ksck_remote.cc
File src/kudu/tools/ksck_remote.cc:
http://gerrit.cloudera.org:8080/#/c/13937/3/src/kudu/tools/ksck_remote.cc@558
PS3, Line 558: tables_count - tables_.size(), tables_count));
> When some tables have been filtered, the log here is not correct, because '
Done
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 5
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Thu, 01 Aug 2019 09:33:26 +0000
Gerrit-HasComments: Yes
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has removed a vote on this change.
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
Removed Verified+1 by Yingchun Lai <40...@qq.com>
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 5
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/13937 )
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
Patch Set 3:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/13937/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13937/2//COMMIT_MSG@9
PS2, Line 9: executes
> Nit: executes
Done
http://gerrit.cloudera.org:8080/#/c/13937/2/src/kudu/tools/ksck-test.cc
File src/kudu/tools/ksck-test.cc:
http://gerrit.cloudera.org:8080/#/c/13937/2/src/kudu/tools/ksck-test.cc@201
PS2, Line 201: for (auto it = tables_.begin(); it != tables_.end();) {
: if (!MatchesAnyPattern(tabl
> for (auto& it = tables_.begin(); it != tables_.end(); ) {
Done
http://gerrit.cloudera.org:8080/#/c/13937/2/src/kudu/tools/ksck-test.cc@213
PS2, Line 213: sh
> '& '
Done
http://gerrit.cloudera.org:8080/#/c/13937/2/src/kudu/tools/ksck-test.cc@215
PS2, Line 215: if (!MatchesAnyPattern(tablet_id_filters_, (*it)->id())) {
: it = tablets.erase(it);
> for (auto& it = tablets.begin(); it != tablets.end(); ) {
Done
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Mon, 29 Jul 2019 03:52:05 +0000
Gerrit-HasComments: Yes
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13937 )
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
Patch Set 7:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13937/7/src/kudu/tools/ksck.h
File src/kudu/tools/ksck.h:
http://gerrit.cloudera.org:8080/#/c/13937/7/src/kudu/tools/ksck.h@452
PS7, Line 452: // Setters for filtering the tables/tablets to be checked.
: //
: // Filter strings are glob-style patterns. For example, 'Foo*' matches
: // all tables whose name begins with 'Foo'.
: //
: // If tables is not empty, checks only the named tables.
: // If tablets is not empty, checks only the specified tablet IDs.
: // If both are specified, takes the intersection.
: // If both are empty (unset), all tables and tablets are checked.
Rather than duplicating all of this, could you just refer to the equivalent Ksck functionality?
Same for set_tablet_id_filters().
http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck_checksum.cc
File src/kudu/tools/ksck_checksum.cc:
http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck_checksum.cc@502
PS5, Line 502: msg += " Filter:";
> It's for passing the test`TestNonMatchingTabletIdFilter`.
Yeah I'm not sure I understand either; it seems like num_replicas_tmp should have included both filtered and unfiltered replicas for the "all range partitions were dropped" case to be represented correctly.
In any case, it'd be good to achieve the semantics described by the comment. Maybe we could do that by tracking the number of replicas we've filtered out for each table? Then we could do the following check here:
if (num_tablets > 0 && (num_replicas_tmp + num_replicas_filtered) == 0) {
<no tablet replicas error>
}
Also could you add a unit test to provide coverage for this case? Both "all range partitions dropped" and "all but one range partitions dropped and that last range partition filtered out".
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 7
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Fri, 02 Aug 2019 17:03:59 +0000
Gerrit-HasComments: Yes
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/13937 )
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
Patch Set 6:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck-test.cc
File src/kudu/tools/ksck-test.cc:
http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck-test.cc@54
PS5, Line 54: #include "kudu/util/test_util.h"
> Not sorted correctly.
Done
http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck-test.cc@200
PS5, Line 200: virtual Status RetrieveTablesList() override {
> I don't understand why these changes were needed. Are there any calls to se
Since table_filters and tablet_id_filters has been moved to KsckCluster, then we got a filtered `tables_` in RetrieveTablesList()/RetrieveTabletsList(), and it's also needed to filter tables/tablets in MockKsckMaster for tests.
set_table_filters/set_tablet_id_filters called in tests for testing matching filters.
http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck.h
File src/kudu/tools/ksck.h:
http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck.h@452
PS5, Line 452: // Setters for filtering the tables/tablets to be checked.
> I think having this API on the Ksck class is nicer; that's typically the cl
Okay, this API and filters in KsckChecksumOptions reserved, though they have no use now.
http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck.cc
File src/kudu/tools/ksck.cc:
http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck.cc@591
PS5, Line 591: if (table->tablets().empty()) {
> Do we actually need to make a copy of this list of tablets? Or could we jus
Done
http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck_checksum.cc
File src/kudu/tools/ksck_checksum.cc:
http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck_checksum.cc@502
PS5, Line 502: msg += " Filter:";
> Can you help me understand the motivation for this change?
It's for passing the test`TestNonMatchingTabletIdFilter`.
Actually I also don't understand the meaning of original comments here, it seems all tablets counted in `num_tablets` and only filted replicas counted in `num_replicas_tmp`.
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 6
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Fri, 02 Aug 2019 07:45:32 +0000
Gerrit-HasComments: Yes
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/13937 )
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13937/3/src/kudu/tools/ksck_remote.cc
File src/kudu/tools/ksck_remote.cc:
http://gerrit.cloudera.org:8080/#/c/13937/3/src/kudu/tools/ksck_remote.cc@558
PS3, Line 558: table_names.size() - tables_.size(), table_names.size()));
When some tables have been filtered, the log here is not correct, because 'table_names' include all tables.
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Thu, 01 Aug 2019 07:51:08 +0000
Gerrit-HasComments: Yes
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13937
to look at the new patch set (#10).
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
[ksck] Filter tables and tablets in KsckCluster
The ksck tool executes slowly if there are too many tables in a cluster
even if we just want to check some specific tables and tablets.
Filtering tables and tablets in KsckCluster and only tracking specified
tables and tablets would speed up the execution of ksck tool with
'--tables=<tables>' and '--tablets=<tablets>' args.
Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_checksum.cc
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/rebalancer.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_replica_util.cc
8 files changed, 120 insertions(+), 40 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/13937/10
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 10
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13937 )
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
Patch Set 9:
(4 comments)
The TSAN test failure looks real: a data race in the new code in KsckCluster.
http://gerrit.cloudera.org:8080/#/c/13937/9/src/kudu/tools/ksck.h
File src/kudu/tools/ksck.h:
http://gerrit.cloudera.org:8080/#/c/13937/9/src/kudu/tools/ksck.h@471
PS9, Line 471: const int32_t& filtered_tables_count() const {
: return filtered_tables_count_;
: }
:
: const int32_t& filtered_tablets_count() const {
: return filtered_tablets_count_;
: }
Value types (i.e. int, double, etc.) should be returned by value rather than by cref.
http://gerrit.cloudera.org:8080/#/c/13937/9/src/kudu/tools/ksck.h@489
PS9, Line 489: int32_t filtered_tables_count_;
: int32_t filtered_tablets_count_;
Nit: 'int' is fine too.
http://gerrit.cloudera.org:8080/#/c/13937/9/src/kudu/tools/ksck_checksum.cc
File src/kudu/tools/ksck_checksum.cc:
http://gerrit.cloudera.org:8080/#/c/13937/9/src/kudu/tools/ksck_checksum.cc@491
PS9, Line 491: &&n
&& n
http://gerrit.cloudera.org:8080/#/c/13937/9/src/kudu/tools/ksck_remote.cc
File src/kudu/tools/ksck_remote.cc:
http://gerrit.cloudera.org:8080/#/c/13937/9/src/kudu/tools/ksck_remote.cc@526
PS9, Line 526: filtered_tables_count_ = 0;
Should probably also initialize these new variables to 0 in the constructor so that they aren't garbage before RetrieveTablesList or RetrieveTabletsList are called.
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 9
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Tue, 06 Aug 2019 17:37:50 +0000
Gerrit-HasComments: Yes
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13937 )
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
Patch Set 5:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck-test.cc
File src/kudu/tools/ksck-test.cc:
http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck-test.cc@54
PS5, Line 54: #include "kudu/tools/tool_action_common.h"
Not sorted correctly.
http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck-test.cc@200
PS5, Line 200: virtual Status RetrieveTablesList() override {
I don't understand why these changes were needed. Are there any calls to set_table_filters or set_tablet_filters here?
http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck.h
File src/kudu/tools/ksck.h:
http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck.h@452
PS5, Line 452: // Setters for filtering the tables/tablets to be checked.
I think having this API on the Ksck class is nicer; that's typically the class that we'd expect people to interact with (and set options on).
If you also need something equivalent in KsckCluster for the plumbing, that's fine, but let's keep the Ksck piece and reserve that for user interaction.
http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck.cc
File src/kudu/tools/ksck.cc:
http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck.cc@591
PS5, Line 591: vector<shared_ptr<KsckTablet>> tablets(table->tablets());
Do we actually need to make a copy of this list of tablets? Or could we just operate on table->tablets() directly?
http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck_checksum.cc
File src/kudu/tools/ksck_checksum.cc:
http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck_checksum.cc@502
PS5, Line 502: if (num_tablets == 0 || num_replicas_tmp == 0) {
Can you help me understand the motivation for this change?
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 5
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Thu, 01 Aug 2019 23:30:19 +0000
Gerrit-HasComments: Yes
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13937 )
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13937/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13937/2//COMMIT_MSG@9
PS2, Line 9: executed
Nit: executes
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Sun, 28 Jul 2019 22:07:36 +0000
Gerrit-HasComments: Yes
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/13937 )
Change subject: [ksck] Filter tables and tablets in KsckCluster
......................................................................
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/13937/2/src/kudu/tools/ksck-test.cc
File src/kudu/tools/ksck-test.cc:
http://gerrit.cloudera.org:8080/#/c/13937/2/src/kudu/tools/ksck-test.cc@201
PS2, Line 201: auto it = tables_.begin();
: while (it != tables_.end()) {
for (auto& it = tables_.begin(); it != tables_.end(); ) {
http://gerrit.cloudera.org:8080/#/c/13937/2/src/kudu/tools/ksck-test.cc@213
PS2, Line 213: &
'& '
http://gerrit.cloudera.org:8080/#/c/13937/2/src/kudu/tools/ksck-test.cc@215
PS2, Line 215: auto it = tablets.begin();
: while (it != tablets.end()) {
for (auto& it = tablets.begin(); it != tablets.end(); ) {
http://gerrit.cloudera.org:8080/#/c/13937/2/src/kudu/tools/ksck_remote.cc
File src/kudu/tools/ksck_remote.cc:
http://gerrit.cloudera.org:8080/#/c/13937/2/src/kudu/tools/ksck_remote.cc@591
PS2, Line 591: table
tablet
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Sat, 27 Jul 2019 16:39:44 +0000
Gerrit-HasComments: Yes