Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14743 )
Change subject: KUDU-3001 Multi-thread to load containers in a data directory ...................................................................... Patch Set 13: (3 comments) few nits http://gerrit.cloudera.org:8080/#/c/14743/13//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14743/13//COMMIT_MSG@16 PS13, Line 16: exsiting existing http://gerrit.cloudera.org:8080/#/c/14743/13/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: http://gerrit.cloudera.org:8080/#/c/14743/13/src/kudu/fs/log_block_manager.cc@1977 PS13, Line 1977: if (PREDICT_FALSE(!(s).ok())) { \ : if (!(s).IsDiskFailure()) { \ : return s; \ : } \ : LOG(ERROR) << Substitute("Not using report from $0: $1", \ : (d)->dir(), (s).ToString()); \ : } nit: this macro is naturally used in expressions like ... RETURN_ON_NON_DISK_FAILURE(dir, s); ... The trailing semicolon is superfluous, but it would look strange otherwise. Maybe, use the standard do { ... the code of the macro goes here ... } while (false) pattern for this macro? http://gerrit.cloudera.org:8080/#/c/14743/13/src/kudu/fs/log_block_manager.cc@2065 PS13, Line 2065: if (PREDICT_FALSE(!s.ok())) continue; style nit: please use scope braces for if (...) -- To view, visit http://gerrit.cloudera.org:8080/14743 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0721ee4a5a6824db146ba0658e60eec25dd0c65c Gerrit-Change-Number: 14743 Gerrit-PatchSet: 13 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Reviewer: wangning <[email protected]> Gerrit-Comment-Date: Sat, 07 Dec 2019 04:37:16 +0000 Gerrit-HasComments: Yes
