Adar Dembo 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 10: (3 comments) http://gerrit.cloudera.org:8080/#/c/14743/10/src/kudu/fs/data_dirs.h File src/kudu/fs/data_dirs.h: http://gerrit.cloudera.org:8080/#/c/14743/10/src/kudu/fs/data_dirs.h@225 PS10, Line 225: std::vector<scoped_refptr<LoadResult>> load_results_; This doesn't seem like state that the DataDir should manage, given how transient it is (i.e. only used at startup). And constraining it to the LBM ought to remove the need for inheritance in the LoadResult definition too. http://gerrit.cloudera.org:8080/#/c/14743/10/src/kudu/fs/data_dirs.cc File src/kudu/fs/data_dirs.cc: http://gerrit.cloudera.org:8080/#/c/14743/10/src/kudu/fs/data_dirs.cc@104 PS10, Line 104: DEFINE_uint64(fs_thread_count_per_data_dir, 5, Could you run some benchmarks to determine an optimal value here, at least for a spinning disk? SSDs and NVMes should use higher values, but until we we can detect that kind of hardware, we should use the lowest common denominator. http://gerrit.cloudera.org:8080/#/c/14743/10/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: http://gerrit.cloudera.org:8080/#/c/14743/10/src/kudu/fs/log_block_manager.cc@2483 PS10, Line 2483: s = LogBlockContainer::Open(this, dir, &result->report, container_name, &container); Don't we want to parallelize this? It's at least two seeks: one for the data file and one for the metadata file. And I believe LBM startup time is dominated by seeks, not by reads. -- 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: 10 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Reviewer: wangning <[email protected]> Gerrit-Comment-Date: Mon, 02 Dec 2019 19:49:08 +0000 Gerrit-HasComments: Yes
