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/04/03 20:44:47 UTC

[kudu-CR] [tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310

Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9912


Change subject: [tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310
......................................................................

[tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310

This improves formatting in ksck output and ensures that the tablet ids
appear in the output in verbose mode (KUDU-2271).

It also revamps how results are printed when tablet id filters are
specified, omitting conclusions about entire tables. It's now clear from
ksck's messages whether it is checking individual tablets using
tablet filters or checking whole tables. This addresses KUDU-2310 as
well, since it makes it clear what sort of filters are being applied.
The summary table is unchanged since it's nice whether a table or tablet
filter is applied.

Change-Id: I2371b704a7c2ebb1b5a4822e3b4d852443c200c5
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
3 files changed, 107 insertions(+), 20 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/9912/1
-- 
To view, visit http://gerrit.cloudera.org:8080/9912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2371b704a7c2ebb1b5a4822e3b4d852443c200c5
Gerrit-Change-Number: 9912
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>

[kudu-CR] [tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9912 )

Change subject: [tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310
......................................................................

[tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310

This improves formatting in ksck output and ensures that the tablet ids
appear in the output in verbose mode (KUDU-2271).

It also revamps how results are printed when tablet id filters are
specified, omitting conclusions about entire tables. It's now clear from
ksck's messages whether it is checking individual tablets using
tablet filters or checking whole tables. This addresses KUDU-2310 as
well, since it makes it clear what sort of filters are being applied.
The summary table is unchanged since it's nice whether a table or tablet
filter is applied.

Change-Id: I2371b704a7c2ebb1b5a4822e3b4d852443c200c5
Reviewed-on: http://gerrit.cloudera.org:8080/9912
Reviewed-by: Attila Bukor <ab...@cloudera.com>
Tested-by: Will Berkeley <wd...@gmail.com>
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
3 files changed, 107 insertions(+), 20 deletions(-)

Approvals:
  Attila Bukor: Looks good to me, but someone else must approve
  Will Berkeley: Verified
  Andrew Wong: Looks good to me, approved

-- 
To view, visit http://gerrit.cloudera.org:8080/9912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2371b704a7c2ebb1b5a4822e3b4d852443c200c5
Gerrit-Change-Number: 9912
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@cloudera.com>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] [tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/9912 )

Change subject: [tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310
......................................................................


Patch Set 2: Code-Review+1


-- 
To view, visit http://gerrit.cloudera.org:8080/9912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2371b704a7c2ebb1b5a4822e3b4d852443c200c5
Gerrit-Change-Number: 9912
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 09 Apr 2018 20:49:21 +0000
Gerrit-HasComments: No

[kudu-CR] [tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9912 )

Change subject: [tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310
......................................................................


Patch Set 5: Code-Review+2

Will the IWYU errors continue to break pre-commits to further ksck changes? Could you put this on Alexey's radar?


-- 
To view, visit http://gerrit.cloudera.org:8080/9912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2371b704a7c2ebb1b5a4822e3b4d852443c200c5
Gerrit-Change-Number: 9912
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@cloudera.com>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 10 Apr 2018 20:08:41 +0000
Gerrit-HasComments: No

[kudu-CR] [tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/9912 )

Change subject: [tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310
......................................................................


Patch Set 5: Verified+1

Unrelated flake and incorrect IWYU.


-- 
To view, visit http://gerrit.cloudera.org:8080/9912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2371b704a7c2ebb1b5a4822e3b4d852443c200c5
Gerrit-Change-Number: 9912
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@cloudera.com>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 10 Apr 2018 19:47:12 +0000
Gerrit-HasComments: No

[kudu-CR] [tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has removed Kudu Jenkins from this change.  ( http://gerrit.cloudera.org:8080/9912 )

Change subject: [tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310
......................................................................


Removed reviewer Kudu Jenkins with the following votes:

* Verified-1 by Kudu Jenkins (120)
-- 
To view, visit http://gerrit.cloudera.org:8080/9912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I2371b704a7c2ebb1b5a4822e3b4d852443c200c5
Gerrit-Change-Number: 9912
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@cloudera.com>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] [tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/9912 )

Change subject: [tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310
......................................................................


Patch Set 5:

> Could you put this on Alexey's radar?

Will do, champ.


-- 
To view, visit http://gerrit.cloudera.org:8080/9912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2371b704a7c2ebb1b5a4822e3b4d852443c200c5
Gerrit-Change-Number: 9912
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@cloudera.com>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 10 Apr 2018 20:16:17 +0000
Gerrit-HasComments: No

[kudu-CR] [tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/9912 )

Change subject: [tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310
......................................................................


Patch Set 2: -Verified

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9912/2/src/kudu/tools/ksck.cc
File src/kudu/tools/ksck.cc:

http://gerrit.cloudera.org:8080/#/c/9912/2/src/kudu/tools/ksck.cc@529
PS2, Line 529:  const string tables_or_tablets = tablet_id_filters_.empty() ? "tables" : "tablets";
> could maybe be used throughout this function? Although I guess in all the p
We need different counts, etc in those places, so it's just easier to add the appropriate literal. Maybe for perfect consistency I shoulda branched here, then? But seems harmless so I don't see the point in changing it.



-- 
To view, visit http://gerrit.cloudera.org:8080/9912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2371b704a7c2ebb1b5a4822e3b4d852443c200c5
Gerrit-Change-Number: 9912
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 10 Apr 2018 09:33:02 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has removed a vote on this change.

Change subject: [tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310
......................................................................


Removed Verified-1 by Kudu Jenkins (120)
-- 
To view, visit http://gerrit.cloudera.org:8080/9912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I2371b704a7c2ebb1b5a4822e3b4d852443c200c5
Gerrit-Change-Number: 9912
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/9912 )

Change subject: [tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310
......................................................................


Patch Set 2: Verified+1

The tests had clock unsync'd issues. Becoming too common.


-- 
To view, visit http://gerrit.cloudera.org:8080/9912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2371b704a7c2ebb1b5a4822e3b4d852443c200c5
Gerrit-Change-Number: 9912
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 06 Apr 2018 20:58:10 +0000
Gerrit-HasComments: No

[kudu-CR] [tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9912 )

Change subject: [tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310
......................................................................


Patch Set 2: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9912/2/src/kudu/tools/ksck.cc
File src/kudu/tools/ksck.cc:

http://gerrit.cloudera.org:8080/#/c/9912/2/src/kudu/tools/ksck.cc@529
PS2, Line 529:  const string tables_or_tablets = tablet_id_filters_.empty() ? "tables" : "tablets";
could maybe be used throughout this function? Although I guess in all the places you have to branch anyway, so maybe not.



-- 
To view, visit http://gerrit.cloudera.org:8080/9912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2371b704a7c2ebb1b5a4822e3b4d852443c200c5
Gerrit-Change-Number: 9912
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 09 Apr 2018 19:37:38 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9912 )

Change subject: [tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310
......................................................................


Patch Set 6:

> Patch Set 5:
> 
> > Could you put this on Alexey's radar?
> 
> Will do, champ.

who is champ?


-- 
To view, visit http://gerrit.cloudera.org:8080/9912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2371b704a7c2ebb1b5a4822e3b4d852443c200c5
Gerrit-Change-Number: 9912
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@cloudera.com>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 10 Apr 2018 20:51:52 +0000
Gerrit-HasComments: No

[kudu-CR] [tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/9912 )

Change subject: [tools] ksck improvements [4/n]: KUDU-2271 and KUDU-2310
......................................................................


Patch Set 5: Code-Review+1


-- 
To view, visit http://gerrit.cloudera.org:8080/9912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2371b704a7c2ebb1b5a4822e3b4d852443c200c5
Gerrit-Change-Number: 9912
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 10 Apr 2018 18:47:27 +0000
Gerrit-HasComments: No