Yingchun Lai 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 12: (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_ Run some more benchmarks, but, I don't have machine with 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 { > Why does this need shared ownership? Done http://gerrit.cloudera.org:8080/#/c/14743/11/src/kudu/fs/log_block_manager.cc@1511 PS11, Line 1511: vector<LogBlockContainerRefPtr> dead_containers; > Don't need std:: prefixes here and below. Done 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()? Here I aim to _append_ multiple container_results' dead_containers to dir_result's, not _assign_. http://gerrit.cloudera.org:8080/#/c/14743/11/src/kudu/fs/log_block_manager.cc@2094 PS11, Line 2094: > Since this is going to be written to, pass it by (unretained) pointer. Done http://gerrit.cloudera.org:8080/#/c/14743/11/src/kudu/fs/log_block_manager.cc@2495 PS11, Line 2495: results->emplace_back(new internal::LogBlockContainerLoadResult()); > Use emplace_back. Done http://gerrit.cloudera.org:8080/#/c/14743/11/src/kudu/fs/log_block_manager.cc@2514 PS11, Line 2514: // Load the container's records asynchronously. > Nit: "Load the container's records asynchronously." Done -- 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: 12 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 12:39:22 +0000 Gerrit-HasComments: Yes
