You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Andrew Wong (Code Review)" <ge...@cloudera.org> on 2018/03/07 04:48:53 UTC
[kudu-CR] WIP KUDU-2129 make ksck less scary when copying
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9528
Change subject: WIP KUDU-2129 make ksck less scary when copying
......................................................................
WIP KUDU-2129 make ksck less scary when copying
WIP: There should be a nicer way to test this without the long lines.
Also there are a few changes here, so open to feedback about which
should stay/go, if there are more, etc. Also pondering whether we should
special-case bootstrapping tablets as well.
This patch changes some of the outputs for ksck to be less troubling
when the cluster is recovering. Updates include:
- Report tablets with replicas whose data states are TABLET_DATA_COPYING
as RECOVERING instead of UNDER_REPLICATED.
- Report tables with recovering tablets as RECOVERING instead of
UNDER_REPLICATED.
- Report replicas that aren't running as "not running" instead of "bad
state". Most non-RUNNING states have a time and place during a healthy
cluster's lifespan.
- Don't log the Status error when using the tool, just the message (e.g.
to avoid messages like "Corruption: 1 out of 1 table(s) are bad").
- Report "non-HEALTHY" instead of "bad" in the above log.
Change-Id: Iba0c1f5b5a7083cbef99d3674dfebe1457075ce0
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/tool_action_cluster.cc
4 files changed, 94 insertions(+), 46 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/9528/1
--
To view, visit http://gerrit.cloudera.org:8080/9528
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iba0c1f5b5a7083cbef99d3674dfebe1457075ce0
Gerrit-Change-Number: 9528
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
[kudu-CR] KUDU-2129 make ksck less scary when copying
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/9528 )
Change subject: KUDU-2129 make ksck less scary when copying
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/9528
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iba0c1f5b5a7083cbef99d3674dfebe1457075ce0
Gerrit-Change-Number: 9528
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 08 Mar 2018 22:18:59 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2129 make ksck less scary when copying
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9528 )
Change subject: KUDU-2129 make ksck less scary when copying
......................................................................
Patch Set 2:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/9528/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/9528/1//COMMIT_MSG@25
PS1, Line 25:
> This is the one change I have an issue with because it makes it less clear
Yeah, I think essentially we want to convey the messages:
- The table is good, DON'T WORRY ABOUT IT
- The table is recovering, KEEP AN EYE ON IT
- The table is under-replicated/unavailable, DO SOMETHING ABOUT IT
I'll go with "not healthy" for now, hopefully its proximity with the table summary will be enough to connect some dots.
http://gerrit.cloudera.org:8080/#/c/9528/1/src/kudu/tools/ksck-test.cc
File src/kudu/tools/ksck-test.cc:
http://gerrit.cloudera.org:8080/#/c/9528/1/src/kudu/tools/ksck-test.cc@322
PS1, Line 322:
> Howabout "are not healthy"?
Sure, I wanted to avoid being so direct because this could easily be seen as "the table is unhealthy", but I see what you mean.
http://gerrit.cloudera.org:8080/#/c/9528/1/src/kudu/tools/ksck-test.cc@402
PS1, Line 402: RT_OK(ksck_->ChecksumData(ChecksumOptions()));
: ASSERT_STR_CONTAINS(err_stream_.str(),
: "0/9 replicas remaining (180B from disk, 90 rows summed)");
:
> Are these the long lines you're complaining about? Maybe we can add a helpe
Yeah, I'm going to just reuse the printing code we have in the Ksck class.
This ended up not being as pretty as I'd hoped because TableSummary is private, and I'd rather not make it public for the sake of tests. Alternatively, I could FRIEND_TEST everything.
http://gerrit.cloudera.org:8080/#/c/9528/1/src/kudu/tools/ksck-test.cc@420
PS1, Line 420: RT_STR_CONTAINS(err_stream_.str(), ExpectedKsckTableSummary("test",
: /*healthy_tables=*/ 3,
: /*recovering_tables=*/ 0,
: /*underreplicated_tables=*/ 0,
: /*consensus_mismatch_tables=*/ 0,
: /*unavailable_tables=*/ 0));
: }
> On the other hand I think the consensus matrix literals are kind of a featu
Yeah, I think the cmatrix stuff is fine, this patch doesn't really affect it and I agree it's helpful to have the full output.
http://gerrit.cloudera.org:8080/#/c/9528/1/src/kudu/tools/ksck.h
File src/kudu/tools/ksck.h:
http://gerrit.cloudera.org:8080/#/c/9528/1/src/kudu/tools/ksck.h@461
PS1, Line 461: // The tablet is healthy.
> Can you add a blurb like this for all non-OK statuses? Maybe like
Done
http://gerrit.cloudera.org:8080/#/c/9528/1/src/kudu/tools/ksck.h@489
PS1, Line 489: t TotalTablets() const {
: return healthy_tablets + recovering_tablets + underreplicated_tablets +
: consensus_mismatch_tablets + unavailable_tablets;
: }
:
: //
> Isn't under-replicated worse than recovering?
Ah, missed that above comment.
--
To view, visit http://gerrit.cloudera.org:8080/9528
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iba0c1f5b5a7083cbef99d3674dfebe1457075ce0
Gerrit-Change-Number: 9528
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 07 Mar 2018 21:57:06 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP KUDU-2129 make ksck less scary when copying
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/9528 )
Change subject: WIP KUDU-2129 make ksck less scary when copying
......................................................................
Patch Set 1:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/9528/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/9528/1//COMMIT_MSG@25
PS1, Line 25: Report "non-HEALTHY" instead of "bad" in the above log
This is the one change I have an issue with because it makes it less clear what ksck is saying. It tells you what the thing is not instead of what it is, which makes it harder to know how to respond. Admittedly "bad" isn't great either, since "bad" tables may actually be self-healing. Maybe we need to break down the results further into HEALTHY, RECOVERING, NEEDS_INTERVENTION, or something?
http://gerrit.cloudera.org:8080/#/c/9528/1/src/kudu/tools/ksck-test.cc
File src/kudu/tools/ksck-test.cc:
http://gerrit.cloudera.org:8080/#/c/9528/1/src/kudu/tools/ksck-test.cc@322
PS1, Line 322: with non-HEALTHY statuses
Howabout "are not healthy"?
http://gerrit.cloudera.org:8080/#/c/9528/1/src/kudu/tools/ksck-test.cc@402
PS1, Line 402: "Table Summary\n"
: " Name | Status | Total Tablets | Healthy | Recovering | Under-replicated | Unavailable\n"
: "------+---------+---------------+---------+------------+------------------+-------------\n"
: " test | HEALTHY | 3 | 3 | 0 | 0 | 0");
Are these the long lines you're complaining about? Maybe we can add a helper like ExpectedKsckTableSummary() that takes a container of expected CheckResults or just an expected count of each kind, and returns the table as a string.
http://gerrit.cloudera.org:8080/#/c/9528/1/src/kudu/tools/ksck-test.cc@420
PS1, Line 420: "The consensus matrix is:\n"
: " Config source | Replicas | Current term | Config index | Committed?\n"
: "---------------+------------------+--------------+--------------+------------\n"
: " master | A* B C | | | Yes\n"
: " A | A* B C D | 0 | | Yes\n"
: " B | A* B C | 0 | | Yes\n"
: " C | A* B C | 0 | | Yes");
On the other hand I think the consensus matrix literals are kind of a feature, since as we test for detecting more situations they serve as a reference for how certain situations look in ksck.
Also it'd be harder to write a helper.
http://gerrit.cloudera.org:8080/#/c/9528/1/src/kudu/tools/ksck.h
File src/kudu/tools/ksck.h:
http://gerrit.cloudera.org:8080/#/c/9528/1/src/kudu/tools/ksck.h@461
PS1, Line 461: // There was a discrepancy among the tablets' consensus configs and the master's.
Can you add a blurb like this for all non-OK statuses? Maybe like
// The tablet has less replicas than its table's replication factor.
UNDER_REPLICATED
// The tablet is missing a majority of its replicas and is unavailable for writes. If a majority cannot be brought back online then the tablet requires manual intervention to recover.
UNAVAILABLE
// The tablet is missing replicas but is in the process of self-healing.
RECOVERING
I also have weak preferences for:
1. Renaming OK to HEALTHY.
2. Reordering the enum values to go from better to worse. IIRC it's
HEALTHY/OK
RECOVERING
UNDER_REPLICATED
CONSENSUS_MISMATCH
UNAVAILABLE
http://gerrit.cloudera.org:8080/#/c/9528/1/src/kudu/tools/ksck.h@489
PS1, Line 489: if (recovering_tablets > 0) {
: return CheckResult::RECOVERING;
: }
: if (underreplicated_tablets > 0) {
: return CheckResult::UNDER_REPLICATED;
: }
Isn't under-replicated worse than recovering?
--
To view, visit http://gerrit.cloudera.org:8080/9528
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iba0c1f5b5a7083cbef99d3674dfebe1457075ce0
Gerrit-Change-Number: 9528
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 07 Mar 2018 18:04:32 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2129 make ksck less scary when copying
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9528
to look at the new patch set (#3).
Change subject: KUDU-2129 make ksck less scary when copying
......................................................................
KUDU-2129 make ksck less scary when copying
This patch changes some of the outputs for ksck to be less troubling
when the cluster is recovering. Updates include:
- Report tablets with replicas whose data states are TABLET_DATA_COPYING
as RECOVERING instead of UNDER_REPLICATED.
- Report tables with recovering tablets as RECOVERING instead of
UNDER_REPLICATED.
- Report replicas that aren't running as "not running" instead of "bad
state". Most non-RUNNING states have a time and place during a healthy
cluster's lifespan.
- Don't log the error type with the errors when running the `ksck` tool.
Our messages are generally good enough, and there isn't a perfect
mapping to some errors. E.g. we should take the "Corruption" out of
"Corruption: 1 out of 1 table(s) are bad"
- Report "not healthy" instead of "bad" in logs like the one above
Change-Id: Iba0c1f5b5a7083cbef99d3674dfebe1457075ce0
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/tool_action_cluster.cc
4 files changed, 189 insertions(+), 90 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/9528/3
--
To view, visit http://gerrit.cloudera.org:8080/9528
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iba0c1f5b5a7083cbef99d3674dfebe1457075ce0
Gerrit-Change-Number: 9528
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] KUDU-2129 make ksck less scary when copying
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9528
to look at the new patch set (#2).
Change subject: KUDU-2129 make ksck less scary when copying
......................................................................
KUDU-2129 make ksck less scary when copying
This patch changes some of the outputs for ksck to be less troubling
when the cluster is recovering. Updates include:
- Report tablets with replicas whose data states are TABLET_DATA_COPYING
as RECOVERING instead of UNDER_REPLICATED.
- Report tables with recovering tablets as RECOVERING instead of
UNDER_REPLICATED.
- Report replicas that aren't running as "not running" instead of "bad
state". Most non-RUNNING states have a time and place during a healthy
cluster's lifespan.
- Don't log the error type with the errors when running the `ksck` tool.
Our messages are generally good enough, and there isn't a perfect
mapping to some errors. E.g. we should take the "Corruption" out of
"Corruption: 1 out of 1 table(s) are bad"
- Report "not healthy" instead of "bad" in logs like the one above
Change-Id: Iba0c1f5b5a7083cbef99d3674dfebe1457075ce0
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/tool_action_cluster.cc
4 files changed, 189 insertions(+), 90 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/9528/2
--
To view, visit http://gerrit.cloudera.org:8080/9528
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iba0c1f5b5a7083cbef99d3674dfebe1457075ce0
Gerrit-Change-Number: 9528
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] KUDU-2129 make ksck less scary when copying
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9528 )
Change subject: KUDU-2129 make ksck less scary when copying
......................................................................
KUDU-2129 make ksck less scary when copying
This patch changes some of the outputs for ksck to be less troubling
when the cluster is recovering. Updates include:
- Report tablets with replicas whose data states are TABLET_DATA_COPYING
as RECOVERING instead of UNDER_REPLICATED.
- Report tables with recovering tablets as RECOVERING instead of
UNDER_REPLICATED.
- Report replicas that aren't running as "not running" instead of "bad
state". Most non-RUNNING states have a time and place during a healthy
cluster's lifespan.
- Don't log the error type with the errors when running the `ksck` tool.
Our messages are generally good enough, and there isn't a perfect
mapping to some errors. E.g. we should take the "Corruption" out of
"Corruption: 1 out of 1 table(s) are bad"
- Report "not healthy" instead of "bad" in logs like the one above
Change-Id: Iba0c1f5b5a7083cbef99d3674dfebe1457075ce0
Reviewed-on: http://gerrit.cloudera.org:8080/9528
Tested-by: Kudu Jenkins
Reviewed-by: Will Berkeley <wd...@gmail.com>
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/tool_action_cluster.cc
4 files changed, 189 insertions(+), 90 deletions(-)
Approvals:
Kudu Jenkins: Verified
Will Berkeley: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/9528
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iba0c1f5b5a7083cbef99d3674dfebe1457075ce0
Gerrit-Change-Number: 9528
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>