You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Will Berkeley (Code Review)" <ge...@cloudera.org> on 2018/02/14 00:03:40 UTC
[kudu-CR] [tools] Fail ksck if fetching consensus state fails
Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9316
Change subject: [tools] Fail ksck if fetching consensus state fails
......................................................................
[tools] Fail ksck if fetching consensus state fails
Commonly, users run ksck as someone other than the kudu superuser.
This means that ksck can't gather consensus state. It still does its
other checks, and will exit with an OK status if there are no
problems with them, just printing some easily-missed warnings at
the top of the output. This patch changes ksck so it fails when
it cannot gather all the information it needs to do all the checks.
Note that if the user specifies consensus=false, they can run ksck
without missing consensus checks causing a failure.
Change-Id: Id3efc9342c3cb3f9652bb8c9789fe20ecf12ff55
---
M src/kudu/tools/ksck.cc
1 file changed, 11 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/9316/1
--
To view, visit http://gerrit.cloudera.org:8080/9316
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id3efc9342c3cb3f9652bb8c9789fe20ecf12ff55
Gerrit-Change-Number: 9316
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
[kudu-CR] [tools] Fail ksck if fetching consensus state fails
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/9316 )
Change subject: [tools] Fail ksck if fetching consensus state fails
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9316/1/src/kudu/tools/ksck.cc
File src/kudu/tools/ksck.cc:
http://gerrit.cloudera.org:8080/#/c/9316/1/src/kudu/tools/ksck.cc@215
PS1, Line 215: s = ts->FetchConsensusState();
: if (!s.ok()) {
: Warn() << Substitute("Errors gathering consensus info for Tablet Server $0: $1",
: ts->ToString(), s.ToString()) << endl;
: }
: }
: return s;
> I'm not sure you want to return status from the FetchConsensusState() here
We do return an error if any FetchConsensusState fails. ConnectToTabletServer is run against every TS off of a thread pool. Each failure will produce the warning above, and additionally increment a bad_servers count. If bad_servers > 0, then we fail ksck with a generic error, so we get the specific errors for each TS that had a failure, plus the overall failure of ksck.
--
To view, visit http://gerrit.cloudera.org:8080/9316
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3efc9342c3cb3f9652bb8c9789fe20ecf12ff55
Gerrit-Change-Number: 9316
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 14 Feb 2018 00:39:34 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] Fail ksck if fetching consensus state fails
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9316 )
Change subject: [tools] Fail ksck if fetching consensus state fails
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9316/1/src/kudu/tools/ksck.cc
File src/kudu/tools/ksck.cc:
http://gerrit.cloudera.org:8080/#/c/9316/1/src/kudu/tools/ksck.cc@215
PS1, Line 215: s = ts->FetchConsensusState();
: if (!s.ok()) {
: Warn() << Substitute("Errors gathering consensus info for Tablet Server $0: $1",
: ts->ToString(), s.ToString()) << endl;
: }
: }
: return s;
I'm not sure you want to return status from the FetchConsensusState() here if it failed.
If you do, then maybe it's worth to be more deterministic and return an error when at least one error happened while fetching the consensus state, not only the last one.
--
To view, visit http://gerrit.cloudera.org:8080/9316
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3efc9342c3cb3f9652bb8c9789fe20ecf12ff55
Gerrit-Change-Number: 9316
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 14 Feb 2018 00:34:27 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] Fail ksck if fetching consensus state fails
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9316 )
Change subject: [tools] Fail ksck if fetching consensus state fails
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9316/1/src/kudu/tools/ksck.cc
File src/kudu/tools/ksck.cc:
http://gerrit.cloudera.org:8080/#/c/9316/1/src/kudu/tools/ksck.cc@215
PS1, Line 215: s = ts->FetchConsensusState();
: if (!s.ok()) {
: Warn() << Substitute("Errors gathering consensus info for Tablet Server $0: $1",
: ts->ToString(), s.ToString()) << endl;
: }
: }
: return s;
> We do return an error if any FetchConsensusState fails. ConnectToTabletServ
Ah, I see. Then it was just my misunderstanding.
SGTM.
--
To view, visit http://gerrit.cloudera.org:8080/9316
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3efc9342c3cb3f9652bb8c9789fe20ecf12ff55
Gerrit-Change-Number: 9316
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 14 Feb 2018 00:58:18 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] Fail ksck if fetching consensus state fails
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9316 )
Change subject: [tools] Fail ksck if fetching consensus state fails
......................................................................
[tools] Fail ksck if fetching consensus state fails
Commonly, users run ksck as someone other than the kudu superuser.
This means that ksck can't gather consensus state. It still does its
other checks, and will exit with an OK status if there are no
problems with them, just printing some easily-missed warnings at
the top of the output. This patch changes ksck so it fails when
it cannot gather all the information it needs to do all the checks.
Note that if the user specifies consensus=false, they can run ksck
without missing consensus checks causing a failure.
Change-Id: Id3efc9342c3cb3f9652bb8c9789fe20ecf12ff55
Reviewed-on: http://gerrit.cloudera.org:8080/9316
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/tools/ksck.cc
1 file changed, 11 insertions(+), 11 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/9316
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Id3efc9342c3cb3f9652bb8c9789fe20ecf12ff55
Gerrit-Change-Number: 9316
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>