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

Reply via email to