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 2017/08/17 20:01:18 UTC
[kudu-CR] KUDU-2101 Include a table summary at the bottom
Will Berkeley has uploaded a new change for review.
http://gerrit.cloudera.org:8080/7707
Change subject: KUDU-2101 Include a table summary at the bottom
......................................................................
KUDU-2101 Include a table summary at the bottom
This add a table summary to the bottom of ksck. For each table, the
table contains a status and information about the total number of
tablets and the number of healthy, under-replicated, and unhealthy
tablets. A tablet with consensus mismatch is counted as unhealthy, to
make the table easier for less experienced users to understand.
See ksck-test for sample output.
Change-Id: I1913352e3a1f91b4bb07e2f5001c8cc94d5155d4
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
3 files changed, 163 insertions(+), 34 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/7707/1
--
To view, visit http://gerrit.cloudera.org:8080/7707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1913352e3a1f91b4bb07e2f5001c8cc94d5155d4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
[kudu-CR] KUDU-2101 Include a table summary at the bottom
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7707
to look at the new patch set (#2).
Change subject: KUDU-2101 Include a table summary at the bottom
......................................................................
KUDU-2101 Include a table summary at the bottom
This add a table summary to the bottom of ksck. For each table, the
table contains a status and information about the total number of
tablets and the number of healthy, under-replicated, and unhealthy
tablets. A tablet with consensus mismatch is counted as unhealthy, to
make the table easier for less experienced users to understand.
See ksck-test for sample output.
Change-Id: I1913352e3a1f91b4bb07e2f5001c8cc94d5155d4
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
3 files changed, 164 insertions(+), 34 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/7707/2
--
To view, visit http://gerrit.cloudera.org:8080/7707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1913352e3a1f91b4bb07e2f5001c8cc94d5155d4
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] KUDU-2101 Include a table summary at the bottom
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.
Change subject: KUDU-2101 Include a table summary at the bottom
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7707/1/src/kudu/tools/ksck.cc
File src/kudu/tools/ksck.cc:
PS1, Line 255: UNHEALTHY
> I find "UNHEALTHY" to be a little confusing. Should we say "unavailable" or
I think unavailable is better but the current unhealthy is better aligned with OK state of being healthy.
Maybe we just need to print out an explanation?
--
To view, visit http://gerrit.cloudera.org:8080/7707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1913352e3a1f91b4bb07e2f5001c8cc94d5155d4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2101 Include a table summary at the bottom
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change.
Change subject: KUDU-2101 Include a table summary at the bottom
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/7707/2/src/kudu/tools/ksck.cc
File src/kudu/tools/ksck.cc:
PS2, Line 235: if (left.TableStatus() == CheckResult::OK && right.TableStatus() == CheckResult::OK) {
: return left.name < right.name;
: }
: if (left.TableStatus() == CheckResult::OK && right.TableStatus() != CheckResult::OK) {
: return true;
: }
: if (left.TableStatus() != CheckResult::OK && right.TableStatus() == CheckResult::OK) {
: return false;
: }
: if (left.TableStatus() != CheckResult::OK && right.TableStatus() != CheckResult::OK) {
: return left.name < right.name;
: }
> slightly simpler option:
Done
PS2, Line 249: col_names
> do you need this variable? I would guess you could just pass the brace list
No, don't need it, but I thought it was a bit more readable. Will change.
PS2, Line 265: row
> same - need the var?
Ditto
--
To view, visit http://gerrit.cloudera.org:8080/7707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1913352e3a1f91b4bb07e2f5001c8cc94d5155d4
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2101 Include a table summary at the bottom
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-2101 Include a table summary at the bottom
......................................................................
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/7707/1/src/kudu/tools/ksck.cc
File src/kudu/tools/ksck.cc:
PS1, Line 222: push_back
emplace_back(std::move(ts))
Line 231: // The table summary is in alphabetical order by name.
is this best or should we put the unhealthy tables at the bottom, so that when you have more than a page full of tables the bad ones stick out?
PS1, Line 239: vector<string> names;
: vector<string> statuses;
: vector<string> total_tablets;
: vector<string> num_healthy;
: vector<string> num_underreplicated;
: vector<string> num_unhealthy;
is using the 'AddRow' interface not fewer lines of code / easier to follow here?
PS1, Line 255: UNHEALTHY
I find "UNHEALTHY" to be a little confusing. Should we say "unavailable" or something to be more precise? I was a little confused when I saw output that said "0 under-replicated but 1 unhealthy". Is under-replicated not unhealthy in some sense?
--
To view, visit http://gerrit.cloudera.org:8080/7707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1913352e3a1f91b4bb07e2f5001c8cc94d5155d4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2101 Include a table summary at the bottom
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change.
Change subject: KUDU-2101 Include a table summary at the bottom
......................................................................
Patch Set 4:
Yes, a machine-readable output format would be nice. Howabout I follow up with a patch for that?
--
To view, visit http://gerrit.cloudera.org:8080/7707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1913352e3a1f91b4bb07e2f5001c8cc94d5155d4
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No
[kudu-CR] KUDU-2101 Include a table summary at the bottom
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-2101 Include a table summary at the bottom
......................................................................
Patch Set 4:
I think a flag for machine-parseable (eg json) output would be nice if we have a need to parse it. Otherwise what's the purpose of the no-summary flag?
--
To view, visit http://gerrit.cloudera.org:8080/7707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1913352e3a1f91b4bb07e2f5001c8cc94d5155d4
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No
[kudu-CR] KUDU-2101 Include a table summary at the bottom
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.
Change subject: KUDU-2101 Include a table summary at the bottom
......................................................................
Patch Set 4: Code-Review+2
> Yes, a machine-readable output format would be nice. Howabout I
> follow up with a patch for that?
> I think a flag for machine-parseable (eg json) output would be nice
> if we have a need to parse it. Otherwise what's the purpose of the
> no-summary flag?
no-sommary flag in case someone relies on the current output format.
> Yes, a machine-readable output format would be nice. Howabout I
> follow up with a patch for that?
Sounds good!
--
To view, visit http://gerrit.cloudera.org:8080/7707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1913352e3a1f91b4bb07e2f5001c8cc94d5155d4
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No
[kudu-CR] KUDU-2101 Include a table summary at the bottom
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has submitted this change and it was merged.
Change subject: KUDU-2101 Include a table summary at the bottom
......................................................................
KUDU-2101 Include a table summary at the bottom
This add a table summary to the bottom of ksck. For each table, the
table contains a status and information about the total number of
tablets and the number of healthy, under-replicated, and unhealthy
tablets. A tablet with consensus mismatch is counted as unhealthy, to
make the table easier for less experienced users to understand.
See ksck-test for sample output.
Change-Id: I1913352e3a1f91b4bb07e2f5001c8cc94d5155d4
Reviewed-on: http://gerrit.cloudera.org:8080/7707
Reviewed-by: Todd Lipcon <to...@apache.org>
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans <jd...@apache.org>
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
3 files changed, 152 insertions(+), 34 deletions(-)
Approvals:
Jean-Daniel Cryans: Looks good to me, approved
Todd Lipcon: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/7707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I1913352e3a1f91b4bb07e2f5001c8cc94d5155d4
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] KUDU-2101 Include a table summary at the bottom
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7707
to look at the new patch set (#4).
Change subject: KUDU-2101 Include a table summary at the bottom
......................................................................
KUDU-2101 Include a table summary at the bottom
This add a table summary to the bottom of ksck. For each table, the
table contains a status and information about the total number of
tablets and the number of healthy, under-replicated, and unhealthy
tablets. A tablet with consensus mismatch is counted as unhealthy, to
make the table easier for less experienced users to understand.
See ksck-test for sample output.
Change-Id: I1913352e3a1f91b4bb07e2f5001c8cc94d5155d4
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
3 files changed, 152 insertions(+), 34 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/7707/4
--
To view, visit http://gerrit.cloudera.org:8080/7707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1913352e3a1f91b4bb07e2f5001c8cc94d5155d4
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] KUDU-2101 Include a table summary at the bottom
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-2101 Include a table summary at the bottom
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7707/3/src/kudu/tools/ksck.cc
File src/kudu/tools/ksck.cc:
PS3, Line 235: ==
On second thought, I think I might have misled you on this, maybe it should be != instead of ==? we want the unhealthy ones to be 'true' so they fall at the end since true > false. But the spirit of using make_pair remains
--
To view, visit http://gerrit.cloudera.org:8080/7707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1913352e3a1f91b4bb07e2f5001c8cc94d5155d4
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2101 Include a table summary at the bottom
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change.
Change subject: KUDU-2101 Include a table summary at the bottom
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7707/3/src/kudu/tools/ksck.cc
File src/kudu/tools/ksck.cc:
PS3, Line 235: ==
> On second thought, I think I might have misled you on this, maybe it should
Whoops.
--
To view, visit http://gerrit.cloudera.org:8080/7707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1913352e3a1f91b4bb07e2f5001c8cc94d5155d4
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2101 Include a table summary at the bottom
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-2101 Include a table summary at the bottom
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/7707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1913352e3a1f91b4bb07e2f5001c8cc94d5155d4
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No
[kudu-CR] KUDU-2101 Include a table summary at the bottom
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.
Change subject: KUDU-2101 Include a table summary at the bottom
......................................................................
Patch Set 4:
Wondering... should we had a flag to disable the summary at the end? Maybe a way to have a less verbose output. Or maybe the reverse, have a flag to not show all the details and only have the summary?
--
To view, visit http://gerrit.cloudera.org:8080/7707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1913352e3a1f91b4bb07e2f5001c8cc94d5155d4
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No
[kudu-CR] KUDU-2101 Include a table summary at the bottom
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change.
Change subject: KUDU-2101 Include a table summary at the bottom
......................................................................
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/7707/1/src/kudu/tools/ksck.cc
File src/kudu/tools/ksck.cc:
PS1, Line 222: push_back
> emplace_back(std::move(ts))
Done
Line 231: // The table summary is in alphabetical order by name.
> is this best or should we put the unhealthy tables at the bottom, so that w
Done
PS1, Line 239: vector<string> names;
: vector<string> statuses;
: vector<string> total_tablets;
: vector<string> num_healthy;
: vector<string> num_underreplicated;
: vector<string> num_unhealthy;
> is using the 'AddRow' interface not fewer lines of code / easier to follow
Done
PS1, Line 255: UNHEALTHY
> I think unavailable is better but the current unhealthy is better aligned w
Unavailable is probably better. An under-replicated tablet is available, but it's not healthy. I think it's straightforward without an explanation to a casual Hadoop admin but I can add some words if you disagree.
PS1, Line 255: UNHEALTHY
> I find "UNHEALTHY" to be a little confusing. Should we say "unavailable" or
Done
--
To view, visit http://gerrit.cloudera.org:8080/7707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1913352e3a1f91b4bb07e2f5001c8cc94d5155d4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2101 Include a table summary at the bottom
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7707
to look at the new patch set (#3).
Change subject: KUDU-2101 Include a table summary at the bottom
......................................................................
KUDU-2101 Include a table summary at the bottom
This add a table summary to the bottom of ksck. For each table, the
table contains a status and information about the total number of
tablets and the number of healthy, under-replicated, and unhealthy
tablets. A tablet with consensus mismatch is counted as unhealthy, to
make the table easier for less experienced users to understand.
See ksck-test for sample output.
Change-Id: I1913352e3a1f91b4bb07e2f5001c8cc94d5155d4
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
3 files changed, 152 insertions(+), 34 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/7707/3
--
To view, visit http://gerrit.cloudera.org:8080/7707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1913352e3a1f91b4bb07e2f5001c8cc94d5155d4
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] KUDU-2101 Include a table summary at the bottom
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-2101 Include a table summary at the bottom
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/7707/2/src/kudu/tools/ksck.cc
File src/kudu/tools/ksck.cc:
PS2, Line 235: if (left.TableStatus() == CheckResult::OK && right.TableStatus() == CheckResult::OK) {
: return left.name < right.name;
: }
: if (left.TableStatus() == CheckResult::OK && right.TableStatus() != CheckResult::OK) {
: return true;
: }
: if (left.TableStatus() != CheckResult::OK && right.TableStatus() == CheckResult::OK) {
: return false;
: }
: if (left.TableStatus() != CheckResult::OK && right.TableStatus() != CheckResult::OK) {
: return left.name < right.name;
: }
slightly simpler option:
return std::make_pair(left.TableStatus() == CheckResult::OK, left.name) <
std::make_pair(right.TableStatus() == CheckResult::OK, right.name);
does that work?
PS2, Line 249: col_names
do you need this variable? I would guess you could just pass the brace list below
PS2, Line 265: row
same - need the var?
--
To view, visit http://gerrit.cloudera.org:8080/7707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1913352e3a1f91b4bb07e2f5001c8cc94d5155d4
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes