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/06/08 20:58:49 UTC
[kudu-CR] [tools] ksck: fix some output spacing and printing of GetFlags warnings
Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10667
Change subject: [tools] ksck: fix some output spacing and printing of GetFlags warnings
......................................................................
[tools] ksck: fix some output spacing and printing of GetFlags warnings
Change-Id: I5a7772a2d25d934ce42b27fbb81a5711cb4f12cd
---
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck_results.cc
2 files changed, 8 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/67/10667/1
--
To view, visit http://gerrit.cloudera.org:8080/10667
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5a7772a2d25d934ce42b27fbb81a5711cb4f12cd
Gerrit-Change-Number: 10667
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
[kudu-CR] [tools] ksck: fix some output spacing and printing of GetFlags warnings
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10667 )
Change subject: [tools] ksck: fix some output spacing and printing of GetFlags warnings
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10667/1/src/kudu/tools/ksck.cc
File src/kudu/tools/ksck.cc:
http://gerrit.cloudera.org:8080/#/c/10667/1/src/kudu/tools/ksck.cc@222
PS1, Line 222: s = master->FetchUnusualFlags();
: if (!s.ok()) {
> Gotcha, thanks for explaining. Would be helpful in the commit message for t
Done
--
To view, visit http://gerrit.cloudera.org:8080/10667
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a7772a2d25d934ce42b27fbb81a5711cb4f12cd
Gerrit-Change-Number: 10667
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 11 Jun 2018 17:24:02 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] ksck: fix some output spacing and printing of GetFlags warnings
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has removed a vote on this change.
Change subject: [tools] ksck: fix some output spacing and printing of GetFlags warnings
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/10667
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I5a7772a2d25d934ce42b27fbb81a5711cb4f12cd
Gerrit-Change-Number: 10667
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [tools] ksck: fix some output spacing and printing of GetFlags warnings
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10667 )
Change subject: [tools] ksck: fix some output spacing and printing of GetFlags warnings
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10667/1/src/kudu/tools/ksck.cc
File src/kudu/tools/ksck.cc:
http://gerrit.cloudera.org:8080/#/c/10667/1/src/kudu/tools/ksck.cc@222
PS1, Line 222: s = master->FetchUnusualFlags();
: if (!s.ok()) {
> Before, the printing was conditioned on the success of FetchUnusualFlags bu
Gotcha, thanks for explaining. Would be helpful in the commit message for those who don't have the immediate context.
--
To view, visit http://gerrit.cloudera.org:8080/10667
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a7772a2d25d934ce42b27fbb81a5711cb4f12cd
Gerrit-Change-Number: 10667
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 11 Jun 2018 17:20:07 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] ksck: fix some output spacing and printing of GetFlags warnings
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10667 )
Change subject: [tools] ksck: fix some output spacing and printing of GetFlags warnings
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10667/1/src/kudu/tools/ksck.cc
File src/kudu/tools/ksck.cc:
http://gerrit.cloudera.org:8080/#/c/10667/1/src/kudu/tools/ksck.cc@222
PS1, Line 222: s = master->FetchUnusualFlags();
: if (!s.ok()) {
> Might be missing something, but does this change do anything? Same below.
Before, the printing was conditioned on the success of FetchUnusualFlags but that status was not set into s, so the ToString of the Status chain on L202 was printed instead. This was usually Status::OK() so an empty line was printed whenever GetFlags failed.
--
To view, visit http://gerrit.cloudera.org:8080/10667
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a7772a2d25d934ce42b27fbb81a5711cb4f12cd
Gerrit-Change-Number: 10667
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 11 Jun 2018 17:17:07 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] ksck: fix some output spacing and printing of GetFlags warnings
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10667 )
Change subject: [tools] ksck: fix some output spacing and printing of GetFlags warnings
......................................................................
[tools] ksck: fix some output spacing and printing of GetFlags warnings
The result of the call to GetFlags was not being saved, resulting
in a different warning being printed than the reason GetFlags failed,
when it did fail. The different warning was usually from a successful
previous RPC and so was, pretty confusingly, a blank line.
Change-Id: I5a7772a2d25d934ce42b27fbb81a5711cb4f12cd
Reviewed-on: http://gerrit.cloudera.org:8080/10667
Tested-by: Will Berkeley <wd...@gmail.com>
Reviewed-by: Will Berkeley <wd...@gmail.com>
---
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck_results.cc
2 files changed, 8 insertions(+), 3 deletions(-)
Approvals:
Will Berkeley: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/10667
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5a7772a2d25d934ce42b27fbb81a5711cb4f12cd
Gerrit-Change-Number: 10667
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [tools] ksck: fix some output spacing and printing of GetFlags warnings
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10667 )
Change subject: [tools] ksck: fix some output spacing and printing of GetFlags warnings
......................................................................
Patch Set 1: Verified+1
Unrealted flake in TSAN TlsSocket test.
--
To view, visit http://gerrit.cloudera.org:8080/10667
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a7772a2d25d934ce42b27fbb81a5711cb4f12cd
Gerrit-Change-Number: 10667
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 08 Jun 2018 21:38:03 +0000
Gerrit-HasComments: No
[kudu-CR] [tools] ksck: fix some output spacing and printing of GetFlags warnings
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10667 )
Change subject: [tools] ksck: fix some output spacing and printing of GetFlags warnings
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10667/1/src/kudu/tools/ksck.cc
File src/kudu/tools/ksck.cc:
http://gerrit.cloudera.org:8080/#/c/10667/1/src/kudu/tools/ksck.cc@222
PS1, Line 222: s = master->FetchUnusualFlags();
: if (!s.ok()) {
Might be missing something, but does this change do anything? Same below.
--
To view, visit http://gerrit.cloudera.org:8080/10667
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a7772a2d25d934ce42b27fbb81a5711cb4f12cd
Gerrit-Change-Number: 10667
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Sat, 09 Jun 2018 00:08:38 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] ksck: fix some output spacing and printing of GetFlags warnings
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10667 )
Change subject: [tools] ksck: fix some output spacing and printing of GetFlags warnings
......................................................................
Patch Set 2: Code-Review+2
Forwarding awong's +2 (only change was to commit msg)
--
To view, visit http://gerrit.cloudera.org:8080/10667
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a7772a2d25d934ce42b27fbb81a5711cb4f12cd
Gerrit-Change-Number: 10667
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 11 Jun 2018 17:56:34 +0000
Gerrit-HasComments: No
[kudu-CR] [tools] ksck: fix some output spacing and printing of GetFlags warnings
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10667
to look at the new patch set (#2).
Change subject: [tools] ksck: fix some output spacing and printing of GetFlags warnings
......................................................................
[tools] ksck: fix some output spacing and printing of GetFlags warnings
The result of the call to GetFlags was not being saved, resulting
in a different warning being printed than the reason GetFlags failed,
when it did fail. The different warning was usually from a successful
previous RPC and so was, pretty confusingly, a blank line.
Change-Id: I5a7772a2d25d934ce42b27fbb81a5711cb4f12cd
---
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck_results.cc
2 files changed, 8 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/67/10667/2
--
To view, visit http://gerrit.cloudera.org:8080/10667
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5a7772a2d25d934ce42b27fbb81a5711cb4f12cd
Gerrit-Change-Number: 10667
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [tools] ksck: fix some output spacing and printing of GetFlags warnings
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10667 )
Change subject: [tools] ksck: fix some output spacing and printing of GetFlags warnings
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/10667
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a7772a2d25d934ce42b27fbb81a5711cb4f12cd
Gerrit-Change-Number: 10667
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 11 Jun 2018 17:56:07 +0000
Gerrit-HasComments: No
[kudu-CR] [tools] ksck: fix some output spacing and printing of GetFlags warnings
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has removed a vote on this change.
Change subject: [tools] ksck: fix some output spacing and printing of GetFlags warnings
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/10667
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I5a7772a2d25d934ce42b27fbb81a5711cb4f12cd
Gerrit-Change-Number: 10667
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins