Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8276 )
Change subject: once: report init_succeeded instead of initted ...................................................................... Patch Set 1: (3 comments) Having read through the patch and having understood the problem, I agree with your solution. Looks like you have some tidy/IWYU issues to deal with. http://gerrit.cloudera.org:8080/#/c/8276/1/src/kudu/fs/fs-test-util.h File src/kudu/fs/fs-test-util.h: http://gerrit.cloudera.org:8080/#/c/8276/1/src/kudu/fs/fs-test-util.h@94 PS1, Line 94: const uint64_t corrupt_offset, uint8_t flip_bit, BlockId* out_id) { The name CorruptBlock suggests that it corrupts an existing block, but the function actually creates a new corrupt block. Perhaps rename it to reflect that? Also, could you doc it? http://gerrit.cloudera.org:8080/#/c/8276/1/src/kudu/tserver/tablet_server-test.cc File src/kudu/tserver/tablet_server-test.cc: http://gerrit.cloudera.org:8080/#/c/8276/1/src/kudu/tserver/tablet_server-test.cc@1240 PS1, Line 1240: req.set_batch_size_bytes(0); // so it won't return data right away Why is this important? http://gerrit.cloudera.org:8080/#/c/8276/1/src/kudu/util/once.h File src/kudu/util/once.h: http://gerrit.cloudera.org:8080/#/c/8276/1/src/kudu/util/once.h@52 PS1, Line 52: // More versatile version of GoogleOnceDynamic, including the following: Would you mind rewriting the block comment to describe the once semantics a little more cohesively? -- To view, visit http://gerrit.cloudera.org:8080/8276 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7ac7131144392d673e0a72a1ba9920bcf9fd991c Gerrit-Change-Number: 8276 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Mon, 16 Oct 2017 22:11:56 +0000 Gerrit-HasComments: Yes
