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 7: (2 comments) http://gerrit.cloudera.org:8080/#/c/14743/5/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: http://gerrit.cloudera.org:8080/#/c/14743/5/src/kudu/fs/log_block_manager.cc@2411 PS5, Line 2411: void LogBlockManager::OpenDataDir(DataDir* dir, > - Yes, especially for SSDs and NVMEs. But for HDDs, it's better to be 1, so Yeah making it a gflag seems like a good idea. Eventually we'd want to autodetect a good value based on the hardware characteristics. As for the second problem, OpenDataDir can create a ThreadPoolToken on the data dir's thread pool, submit LoadRecords using the token, then Wait on the token rather than the pool, which means it'll wait on just those tasks. See the comments in threadpool.h for more details. Hopefully you can implement that without poking too many holes in the DataDir abstraction. http://gerrit.cloudera.org:8080/#/c/14743/5/src/kudu/fs/log_block_manager.cc@2414 PS5, Line 2414: scoped_refptr<internal::ContainerLoadResult> data_dir_result(new internal::ContainerLoadResult()); > I think the best max thread value depends on containers count per data dir, This is where consolidating behind the DataDir thread pool makes it easier to reason about the situation: the only thing the operator needs to figure out is the max threads of the data dir's threadpool, and it should reflect the number of concurrent operations the underlying hardware can process. I'd expect LoadRecords to be I/O bound so not sure the number of CPU cores matters. -- 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: 7 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Thu, 21 Nov 2019 19:52:22 +0000 Gerrit-HasComments: Yes
