Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11498 )
Change subject: [tools] ksck checksums: Add KsckChecksummer class ...................................................................... Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/11498/6/src/kudu/tools/ksck_checksum.cc File src/kudu/tools/ksck_checksum.cc: http://gerrit.cloudera.org:8080/#/c/11498/6/src/kudu/tools/ksck_checksum.cc@205 PS6, Line 205: num_tablets += table->tablets().size(); I'm curious why the number of tablets is incremented regardless of the tablet_id filter. If there is a tablet_id filter which does not match any tablet (but table_name filter matches), the code below will issue a warning about not finding any replicas, as if a table where every partition has been dropped. Is that intended behavior? http://gerrit.cloudera.org:8080/#/c/11498/6/src/kudu/tools/ksck_checksum.cc@227 PS6, Line 227: nit: for better readability, maybe drop this space but prefix 'table_filters=...' and 'tablet_id_filters=...' messages below with spaces. http://gerrit.cloudera.org:8080/#/c/11498/6/src/kudu/tools/ksck_checksum.cc@372 PS6, Line 372: if (queue->BlockingGet(&table_tablet)) What if BlockingGet() returns false? Does it make sense to continue in that case? http://gerrit.cloudera.org:8080/#/c/11498/6/src/kudu/tools/ksck_checksum.cc@388 PS6, Line 388: Status s = CollateChecksumResults(reporter->checksums(), nit: consider adding 'const' specifier -- the code goes further and it's easier to read when you know it's not going to change. -- To view, visit http://gerrit.cloudera.org:8080/11498 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2016936eaa26fd6b499783e7d5d8f404816b37fa Gerrit-Change-Number: 11498 Gerrit-PatchSet: 6 Gerrit-Owner: Will Berkeley <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Tue, 02 Oct 2018 19:57:01 +0000 Gerrit-HasComments: Yes
