Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18053 )
Change subject: [bootstrap] Speedup tablet bootstrap ...................................................................... Patch Set 5: (10 comments) http://gerrit.cloudera.org:8080/#/c/18053/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18053/4//COMMIT_MSG@11 PS4, Line 11: - short circuit return when found an active memory store > nit: wrap these at 80 Done http://gerrit.cloudera.org:8080/#/c/18053/4/src/kudu/fs/fs_manager.cc File src/kudu/fs/fs_manager.cc: http://gerrit.cloudera.org:8080/#/c/18053/4/src/kudu/fs/fs_manager.cc@719 PS4, Line 719: emplace_back(std > nit: could use the move semantics for 'child', moving strings from one vect Done http://gerrit.cloudera.org:8080/#/c/18053/4/src/kudu/tablet/tablet_bootstrap.cc File src/kudu/tablet/tablet_bootstrap.cc: http://gerrit.cloudera.org:8080/#/c/18053/4/src/kudu/tablet/tablet_bootstrap.cc@654 PS4, Line 654: ock > nit: the prior version was grammatically correct and easy to understand Eng Done http://gerrit.cloudera.org:8080/#/c/18053/4/src/kudu/tserver/ts_tablet_manager.cc File src/kudu/tserver/ts_tablet_manager.cc: http://gerrit.cloudera.org:8080/#/c/18053/4/src/kudu/tserver/ts_tablet_manager.cc@438 PS4, Line 438: > nit: an extra line In fact I want to seperate each thread pool creating code block. http://gerrit.cloudera.org:8080/#/c/18053/4/src/kudu/tserver/ts_tablet_manager.cc@489 PS4, Line 489: uccess_loaded_co > What's difference between a fatal and a non-fatal one? Please add a commen Done http://gerrit.cloudera.org:8080/#/c/18053/4/src/kudu/tserver/ts_tablet_manager.cc@494 PS4, Line 494: seen_error) { > nit here and below: for STL atomics, it's not necessary to use load() or ot Almost OK, but not for reference in Substitute(), an error is threw: error: invalid initialization of reference of type ‘const strings::internal::SubstituteArg&’ from expression of type ‘std::atomic<int>’ http://gerrit.cloudera.org:8080/#/c/18053/4/src/kudu/tserver/ts_tablet_manager.cc@511 PS4, Line 511: s = s.CloneAndPrepend(Substitute("could not open tablet metadata: $0", tablet_id)); > Does KLOG_EVERY_N/KLOG_EVERY_N_SECS not work here? they work, I'll use KLOG_EVERY_N_SECS. http://gerrit.cloudera.org:8080/#/c/18053/4/src/kudu/tserver/ts_tablet_manager.cc@519 PS4, Line 519: > Missed '$0'? Done http://gerrit.cloudera.org:8080/#/c/18053/4/src/kudu/tserver/ts_tablet_manager.cc@524 PS4, Line 524: > Using atomics everywhere and then resorting to serialized/locking access lo Good idea! I'll do it like that. http://gerrit.cloudera.org:8080/#/c/18053/4/src/kudu/tserver/ts_tablet_manager.cc@1416 PS4, Line 1416: deleter->Destroy(); > nit: this comment is too vague -- I'm not sure anybody (even you yourself) I'll remove it. -- To view, visit http://gerrit.cloudera.org:8080/18053 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I816417d2d4c24014edb6b2a40c060f29e37ae219 Gerrit-Change-Number: 18053 Gerrit-PatchSet: 5 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Wed, 01 Dec 2021 17:06:35 +0000 Gerrit-HasComments: Yes
