Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19327 )
Change subject: [Tool] KUDU-3318 Compact container metadata file manually ...................................................................... Patch Set 12: (7 comments) http://gerrit.cloudera.org:8080/#/c/19327/8/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: http://gerrit.cloudera.org:8080/#/c/19327/8/src/kudu/fs/log_block_manager.cc@552 PS8, Line 552: static Status ProcessRecord( Describe what does 'total_blocks' mean as well, and it's helpful to mention whether it count dead blocks or not. And it seems this function always return OK, so the return type can be replaced by void? http://gerrit.cloudera.org:8080/#/c/19327/8/src/kudu/fs/log_block_manager.cc@957 PS8, Line 957: Env* env, > Using LogBlockContainerNativeMeta::ProcessRecords() needs to open log block I see,thanks for clarification. http://gerrit.cloudera.org:8080/#/c/19327/8/src/kudu/fs/log_block_manager.cc@981 PS8, Line 981: } Should we update the offset only if no error occurs (EOF or Incomplete) ? http://gerrit.cloudera.org:8080/#/c/19327/8/src/kudu/fs/log_block_manager.cc@1369 PS8, Line 1369: switch (record->op_type()) { nit: remove it if not used. http://gerrit.cloudera.org:8080/#/c/19327/8/src/kudu/fs/log_block_manager.cc@2791 PS8, Line 2791: _block_records.size(); > Add this parameter to compact_metadata tool, then --help will print it. Done http://gerrit.cloudera.org:8080/#/c/19327/8/src/kudu/fs/log_block_manager.cc@2801 PS8, Line 2801: RETURN_NOT_OK(GenerateCompactedFile(env, dir, container_name, records)); > In function GenerateCompactedFile(), there exists a method to delete .compa Done http://gerrit.cloudera.org:8080/#/c/19327/8/src/kudu/fs/log_block_manager.cc@2987 PS8, Line 2987: > RewriteMetadataFile() is only called with log block manager opened. Even we Is there any reasons we must keep the functions as static? I guess you don't want to open the block manager, does the option "skip_block_manager = true" enough? -- To view, visit http://gerrit.cloudera.org:8080/19327 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id61cb6c88d7a093a8448e2497639f5c5c094efa4 Gerrit-Change-Number: 19327 Gerrit-PatchSet: 12 Gerrit-Owner: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Attila Bukor <abu...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang <chinazhangyi...@163.com> Gerrit-Reviewer: Yingchun Lai <laiyingc...@apache.org> Gerrit-Comment-Date: Mon, 26 Feb 2024 04:20:23 +0000 Gerrit-HasComments: Yes