Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17152 )
Change subject: KUDU-3254 fix bug in meta-cache exposed by KUDU-1802 ...................................................................... Patch Set 6: Code-Review+1 (4 comments) http://gerrit.cloudera.org:8080/#/c/17152/6/src/kudu/client/meta_cache.cc File src/kudu/client/meta_cache.cc: http://gerrit.cloudera.org:8080/#/c/17152/6/src/kudu/client/meta_cache.cc@943 PS6, Line 943: auto* entry = FindOrNull(tablets_by_key, tablet_lower_bound); > Might be a good idea to audit the codebase for other usages of FindOrDie. W I generally disagree that we shouldn't use FindOrDie in production code. It has its place in the codebase for areas where we are provably guaranteed that the element exists. That said, it is definitely important upon reviewing and in testing that we ensure that such a guarantee exists (and isn't reliant on assumptions that can be broken). Well used, FindOrDie has the nice property of telling readers that such a guarantee does exist, similar to our usages of other *OrDie methods. http://gerrit.cloudera.org:8080/#/c/17152/6/src/kudu/client/meta_cache.cc@955 PS6, Line 955: nit: replace with spaces http://gerrit.cloudera.org:8080/#/c/17152/6/src/kudu/client/scan_token-test.cc File src/kudu/client/scan_token-test.cc: http://gerrit.cloudera.org:8080/#/c/17152/6/src/kudu/client/scan_token-test.cc@950 PS6, Line 950: nit: spacing http://gerrit.cloudera.org:8080/#/c/17152/6/src/kudu/client/scan_token-test.cc@1052 PS6, Line 1052: [[fallthrough]]; nit: maybe use FALLTHROUGH_INTENDED? Or maybe we should deprecate that in favor of [[fallthrough]]. -- To view, visit http://gerrit.cloudera.org:8080/17152 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.13.x Gerrit-MessageType: comment Gerrit-Change-Id: I5b8370290c13b1e496f461ed5bc2e0193bdf4b19 Gerrit-Change-Number: 17152 Gerrit-PatchSet: 6 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Sat, 06 Mar 2021 23:33:13 +0000 Gerrit-HasComments: Yes
