Todd Lipcon has posted comments on this change. Change subject: KUDU-1325: more crash avoidance during remote bootstrap and tablet deletion ......................................................................
Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/2265/3/src/kudu/consensus/log.h File src/kudu/consensus/log.h: Line 156: // this function cannot return new reader references, but existing reader can you be more specific here? "will return NULL" or somesuch? Line 351: std::shared_ptr<LogReader> reader_; worth saying 'NULL after the log is closed' http://gerrit.cloudera.org:8080/#/c/2265/3/src/kudu/util/make_shared.h File src/kudu/util/make_shared.h: Line 38: // but forward defined as a struct (ext/alloc_traits.h). Clang complains nit: forward declared Line 39: // about this when -Wmismatched-tags is set, which gcc doesn't support if gcc doesn't support it, could you use #pragma clang instead, and so gcc would just ignore it? ignoring -Wpragmas seems like it might cause trouble for someone later who has included this header and then uses an invalid pragma and confused why it's not doing anything or giving a warning. -- To view, visit http://gerrit.cloudera.org:8080/2265 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6a01c9e6886bd8bf0e286fc3980babc512056286 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
