Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17871 )

Change subject: KUDU-3318 [LBM] Runtime compact log container metadata
......................................................................


Patch Set 11: Code-Review+2

(4 comments)

Thanks for revving this! Just more nits, otherwise LGTM.

http://gerrit.cloudera.org:8080/#/c/17871/11/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

http://gerrit.cloudera.org:8080/#/c/17871/11/src/kudu/fs/log_block_manager.cc@89
PS11, Line 89: DEFINE_uint64(log_container_metadata_max_size, 0,
             :               "Maximum size (soft) of a log container's 
metadata. Use 0 for "
             :               "no limit.");
Empirically, and for posterity, I'm curious what values have been useful in 
resolving the space issues you've seen in your clusters. I imagine this would 
be directly correlated with the --log_container_max_size configuration, since 
the size of each metadata record is fairly constant, though with some wiggle 
room for deletes.


http://gerrit.cloudera.org:8080/#/c/17871/11/src/kudu/fs/log_block_manager.cc@123
PS11, Line 123: at startup
nit: "..., or at runtime, based on the configuration of 
--log_container_metadata_size_before_compact_ratio."


http://gerrit.cloudera.org:8080/#/c/17871/11/src/kudu/fs/log_block_manager.cc@127
PS11, Line 127: "Desired ratio of block metadata size of 
--log_container_metadata_max_size. "
              :               "If a container's metadata file size exceed 
--log_container_metadata_max_size * "
              :               
"--log_container_metadata_size_before_compact_ratio, the container is going "
              :               "to be compact at runtime."
nit: IMO it isn't clear from this what the behavior is, especially in 
considering log_container_live_metadata_before_compact_ratio.

How about something like,

"Desired portion of --log_container_metadata_max_size container metadata must 
consume before metadata is considered for compaction. If a container's metadata 
file exceeds --log_container_metadata_max_size * 
--log_container_metadata_size_before_compact_ratio, and the number of live 
blocks falls below --log_container_live_metadata_before_compact_ratio, the 
container's metadata will be compacted."


http://gerrit.cloudera.org:8080/#/c/17871/11/src/kudu/fs/log_block_manager.cc@868
PS11, Line 868:   // Check again is necessary, metadata may has been compacted 
very before.
nit: maybe

"Check again, in case the metadata was compacted while we were waiting."



--
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: 11
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: Mon, 21 Mar 2022 22:21:54 +0000
Gerrit-HasComments: Yes

Reply via email to