Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11554 )
Change subject: [tools] KUDU-2179: Have ksck not use a single snapshot for all tablets ...................................................................... Patch Set 9: (6 comments) http://gerrit.cloudera.org:8080/#/c/11554/10/src/kudu/tools/ksck-test.cc File src/kudu/tools/ksck-test.cc: http://gerrit.cloudera.org:8080/#/c/11554/10/src/kudu/tools/ksck-test.cc@21 PS10, Line 21: #include <atomic> > Unused? Or is it? The 'timestamp_' member is an atomic. Also IWYU isn't complaining. http://gerrit.cloudera.org:8080/#/c/11554/10/src/kudu/tools/ksck-test.cc@1588 PS10, Line 1588: new_uuid, : std::make_shared<MockKsckTabletServer>(new_uuid)); > nit: add a space Done http://gerrit.cloudera.org:8080/#/c/11554/7/src/kudu/tools/ksck_checksum.cc File src/kudu/tools/ksck_checksum.cc: http://gerrit.cloudera.org:8080/#/c/11554/7/src/kudu/tools/ksck_checksum.cc@298 PS7, Line 298: aut > Ah, thanks for doing that. I don't think I'm a fan of that; it's verbose an Done http://gerrit.cloudera.org:8080/#/c/11554/9/src/kudu/tools/ksck_checksum.cc File src/kudu/tools/ksck_checksum.cc: http://gerrit.cloudera.org:8080/#/c/11554/9/src/kudu/tools/ksck_checksum.cc@353 PS9, Line 353: : > nit: spacing Done http://gerrit.cloudera.org:8080/#/c/11554/9/src/kudu/tools/ksck_checksum.cc@354 PS9, Line 354: DCHECK_GE(*slots_open, 0); : DCHECK_LE(*slots_open, opts_.scan_concurrency); > These are redundant now that we're actually using the values we already DCH They're technically not redundant but they are probably unnecessary. http://gerrit.cloudera.org:8080/#/c/11554/9/src/kudu/tools/ksck_remote-test.cc File src/kudu/tools/ksck_remote-test.cc: http://gerrit.cloudera.org:8080/#/c/11554/9/src/kudu/tools/ksck_remote-test.cc@416 PS9, Line 416: TestChecksumSnapshotLastingLongerThanAHM > Did you try to run this with --stress-cpu-threads=8 and many iterations usi 1000 times, no failures, DEBUG http://dist-test.cloudera.org/job?job_id=wdberkeley.1538696623.61201 -- To view, visit http://gerrit.cloudera.org:8080/11554 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iff0905c2099e6f56ed1cb651611918acbaf75476 Gerrit-Change-Number: 11554 Gerrit-PatchSet: 9 Gerrit-Owner: Will Berkeley <[email protected]> Gerrit-Reviewer: Adar Dembo <[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: Thu, 04 Oct 2018 23:52:48 +0000 Gerrit-HasComments: Yes
