You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Andrew Wong (Code Review)" <ge...@cloudera.org> on 2021/11/02 01:30:54 UTC
[kudu-CR] [LBM] Speed up server bootstrap by using multi-thread to compact containers
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17942 )
Change subject: [LBM] Speed up server bootstrap by using multi-thread to compact containers
......................................................................
Patch Set 5: Code-Review+1
(4 comments)
LGTM for the most part, just a couple nits
http://gerrit.cloudera.org:8080/#/c/17942/2/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:
http://gerrit.cloudera.org:8080/#/c/17942/2/src/kudu/fs/log_block_manager.cc@2964
PS2, Line 2964: // The container was deleted outright.
> In our environments, there are many tserver deployed on a server with just
Thanks for adding a test! I played around with the test and am seeing consistent results. 12s without the patch, 2s with it, using the same arguments.
http://gerrit.cloudera.org:8080/#/c/17942/5/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:
http://gerrit.cloudera.org:8080/#/c/17942/5/src/kudu/fs/log_block_manager.cc@131
PS5, Line 131: Amount of latency in ms to inject before registering "
: "an op with MVCC.
nit: wrong description?
http://gerrit.cloudera.org:8080/#/c/17942/5/src/kudu/fs/log_block_manager.cc@2957
PS5, Line 2957: Load
nit: this displays as
Time spent Load low live block containers: real 0.000s user 0.000s sys 0.000s
instead, how about we make it
"loading block containers with low live blocks" so the capitalization is more correct when displayed
http://gerrit.cloudera.org:8080/#/c/17942/5/src/kudu/fs/log_block_manager.cc@3027
PS5, Line 3027: if (PREDICT_FALSE(FLAGS_log_container_metadata_rewrite_inject_latency_ms > 0)) {
: SleepFor(MonoDelta::FromMilliseconds(FLAGS_log_container_metadata_rewrite_inject_latency_ms));
: }
nit: if you want, you could include util/fault_injection.h to use MAYBE_INJECT_FIXED_LATENCY()
--
To view, visit http://gerrit.cloudera.org:8080/17942
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie48211d9e8c1d74e520fcb04df25c1d681261bb5
Gerrit-Change-Number: 17942
Gerrit-PatchSet: 5
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Tue, 02 Nov 2021 01:30:54 +0000
Gerrit-HasComments: Yes