Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18053 )
Change subject: [bootstrap] Speedup tablet bootstrap ...................................................................... Patch Set 19: (6 comments) http://gerrit.cloudera.org:8080/#/c/18053/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18053/8//COMMIT_MSG@18 PS8, Line 18: --num_tablets_to_open_simultaneously from default 1 to 8 > Can you show the actual numbers? Did you use the same num_tablets_to_open_s Commit message updated. Yes I tested both cases, but before this patch, --num_tablets_to_open_simultaneously is not used for _table metadata load_ procedure, the test is meanless. 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@826 PS4, Line 826: if (b.term() == a.term() && > I don't think this makes the code clearer. Done http://gerrit.cloudera.org:8080/#/c/18053/8/src/kudu/tserver/ts_tablet_manager-test.cc File src/kudu/tserver/ts_tablet_manager-test.cc: http://gerrit.cloudera.org:8080/#/c/18053/8/src/kudu/tserver/ts_tablet_manager-test.cc@416 PS8, Line 416: const int64_t kTabletCount = FLAGS_startup_benchmark_tablet_count_for_testing; > Why 8 threads specifically? I‘ve removed this thread pool, the pool is meanless here. http://gerrit.cloudera.org:8080/#/c/18053/8/src/kudu/tserver/ts_tablet_manager-test.cc@436 PS8, Line 436: FLAGS_minloglevel = 2; > Why do we skip opening the tablet? Shouldn't we measure the time it takes t Because this patch and this benchmark is only for tablet _metadata_ load procedure, the whole tablet bootstrap include tablet metadata load, open, replay wals, consensus, etc. The other procedures will cost much time, but I don't care for this benchmark itself, so I want to skip them. http://gerrit.cloudera.org:8080/#/c/18053/8/src/kudu/tserver/ts_tablet_manager-test.cc@437 PS8, Line 437: } > Why are we injecting latency in a benchmark? I'm going to simulate a bad disk IO latency, to prove this patch's optimization works indeed. 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: for (int i = 0; i < tablet_ids.s > nit: since 'tablet_ids' is already populated when 'metas' is constructed, i Done -- 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: 19 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: Fri, 03 Dec 2021 07:58:58 +0000 Gerrit-HasComments: Yes
