Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/17871 )
Change subject: KUDU-3318 [LBM] Runtime compact log container metadata ...................................................................... Patch Set 9: (4 comments) http://gerrit.cloudera.org:8080/#/c/17871/9/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: http://gerrit.cloudera.org:8080/#/c/17871/9/src/kudu/fs/log_block_manager.cc@202 PS9, Line 202: F > nit: align this with the "(" Done http://gerrit.cloudera.org:8080/#/c/17871/9/src/kudu/fs/log_block_manager.cc@495 PS9, Line 495: enum class ProcessRecordType { : kReadOnly, // Read records only. : kReadAndUpdate, // Read records and update container's statistic. : }; > nit: can you add a comment mentioning why this is used, so users can better Thanks, done http://gerrit.cloudera.org:8080/#/c/17871/9/src/kudu/fs/log_block_manager.cc@629 PS9, Line 629: if (static_cast<double>(live_blocks()) / total_blocks() >= : FLAGS_log_container_live_metadata_before_compact_ratio) { : return false; : } > Considering rewriting using multiplication to avoid the costlier division Done http://gerrit.cloudera.org:8080/#/c/17871/9/src/kudu/fs/log_block_manager.cc@2647 PS9, Line 2647: std::lock_guard<simple_spinlock> l(lock_); > Now that 'all_containers_by_name_' isn't referenced here, we don't need thi Done -- To view, visit http://gerrit.cloudera.org:8080/17871 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I72bdf73fa73403918c01dfa9d45530fad47ecaf6 Gerrit-Change-Number: 17871 Gerrit-PatchSet: 9 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Fri, 29 Oct 2021 15:50:33 +0000 Gerrit-HasComments: Yes
