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 11: (7 comments) http://gerrit.cloudera.org:8080/#/c/14743/11//COMMIT_MSG Commit Message: PS11: Please rerun your benchmarks, experimenting with different values for --fs_thread_count_per_data_dir. Possibly with different hardware, if you have access to spinning disks, SSDs, and NVMes. http://gerrit.cloudera.org:8080/#/c/14743/11/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: http://gerrit.cloudera.org:8080/#/c/14743/11/src/kudu/fs/log_block_manager.cc@1505 PS11, Line 1505: struct LogBlockContainerLoadResult : public RefCountedThreadSafe<LogBlockContainerLoadResult> { Why does this need shared ownership? http://gerrit.cloudera.org:8080/#/c/14743/11/src/kudu/fs/log_block_manager.cc@1511 PS11, Line 1511: std::vector<LogBlockContainerRefPtr> dead_containers; Don't need std:: prefixes here and below. http://gerrit.cloudera.org:8080/#/c/14743/11/src/kudu/fs/log_block_manager.cc@2077 PS11, Line 2077: dir_result->dead_containers.insert( Why not just std::move()? dir_result->dead_containers = std::move(container_result->dead_containers); http://gerrit.cloudera.org:8080/#/c/14743/11/src/kudu/fs/log_block_manager.cc@2094 PS11, Line 2094: dir_result Since this is going to be written to, pass it by (unretained) pointer. http://gerrit.cloudera.org:8080/#/c/14743/11/src/kudu/fs/log_block_manager.cc@2495 PS11, Line 2495: results->push_back(result); Use emplace_back. http://gerrit.cloudera.org:8080/#/c/14743/11/src/kudu/fs/log_block_manager.cc@2514 PS11, Line 2514: // Open the container asynchronously. Nit: "Load the container's records asynchronously." -- 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: 11 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: Thu, 05 Dec 2019 04:45:57 +0000 Gerrit-HasComments: Yes
