Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18053 )
Change subject: [bootstrap] Speedup tablet bootstrap ...................................................................... Patch Set 15: (4 comments) http://gerrit.cloudera.org:8080/#/c/18053/15/src/kudu/tserver/ts_tablet_manager-test.cc File src/kudu/tserver/ts_tablet_manager-test.cc: http://gerrit.cloudera.org:8080/#/c/18053/15/src/kudu/tserver/ts_tablet_manager-test.cc@137 PS15, Line 137: tablet_manager_->WaitForAllBootstrapsToFinish() What if WaitForAllBootstrapsToFinish() returns non-OK status: should the error be ignored intentionally or this needs to be wrapped into RETURN_NOT_OK? If the former, please add a comment to explain why. 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: max_open_threads = fs_manager_->GetDataRootDirs().size(); > In fact I want to seperate each thread pool creating code block. Ah, I see: sounds good to me then. http://gerrit.cloudera.org:8080/#/c/18053/4/src/kudu/tserver/ts_tablet_manager.cc@494 PS4, Line 494: ED_LOG_TIMING(INFO, Sub > Almost OK, but not for reference in Substitute(), an error is threw: Ah, interesting, thanks. Probably, some extra care should be taken for SubstituteArg, but it's a separate issue not related to this patch. http://gerrit.cloudera.org:8080/#/c/18053/15/src/kudu/tserver/ts_tablet_manager.cc File src/kudu/tserver/ts_tablet_manager.cc: http://gerrit.cloudera.org:8080/#/c/18053/15/src/kudu/tserver/ts_tablet_manager.cc@499 PS15, Line 499: metas.resize(tablet_ids.size()); nit: since 'tablet_ids' is already populated when 'metas' is constructed, instead of this the constructor for 'metas' could be updated to avoid re-allocation (see https://en.cppreference.com/w/cpp/container/vector/vector for details): vector<scoped_refptr<TabletMetadata>> metas(tablets_ids.size()); -- 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: 15 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: Thu, 02 Dec 2021 17:23:04 +0000 Gerrit-HasComments: Yes
