[kudu-CR] KUDU-2101 Include a table summary at the bottom
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 Tested-by: Kudu Jenkins Reviewed-by: Jean-Daniel Cryans --- 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 Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2101 Include a table summary at the bottom
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 Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: No
[kudu-CR] KUDU-2101 Include a table summary at the bottom
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 Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: No
[kudu-CR] KUDU-2101 Include a table summary at the bottom
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 Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: No
[kudu-CR] KUDU-2101 Include a table summary at the bottom
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 Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: No
[kudu-CR] KUDU-2101 Include a table summary at the bottom
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 Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: No
[kudu-CR] KUDU-2101 Include a table summary at the bottom
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 Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] KUDU-2101 Include a table summary at the bottom
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 Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2101 Include a table summary at the bottom
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 Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] KUDU-2101 Include a table summary at the bottom
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 Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] KUDU-2101 Include a table summary at the bottom
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 Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2101 Include a table summary at the bottom
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 Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] KUDU-2101 Include a table summary at the bottom
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 names; : vector statuses; : vector total_tablets; : vector num_healthy; : vector num_underreplicated; : vector 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 Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] KUDU-2101 Include a table summary at the bottom
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 Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2101 Include a table summary at the bottom
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 Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-2101 Include a table summary at the bottom
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 names; : vector statuses; : vector total_tablets; : vector num_healthy; : vector num_underreplicated; : vector 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 Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[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