You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Yuqi Du (Code Review)" <ge...@cloudera.org> on 2023/04/04 10:55:33 UTC
[kudu-CR] KUDU-3371 [fs] make LogBlockContainer a base class
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/19610 )
Change subject: KUDU-3371 [fs] make LogBlockContainer a base class
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/19610/3/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:
http://gerrit.cloudera.org:8080/#/c/19610/3/src/kudu/fs/log_block_manager.cc@629
PS3, Line 629: virtual void AfterBlocksDeleted() {}
nit: what about
'CompactIfNeed' or 'DoCleanerWorksIfNeed()' ?
This is a new function, make it a pure virtual function?
http://gerrit.cloudera.org:8080/#/c/19610/3/src/kudu/fs/log_block_manager.cc@743
PS3, Line 743: LogBlockContainerNativeMeta
In semantics, LogBlockContainer is a pair combination of a meta file and a data file.
LogBlockContainerNativeMeta is a LogBlockContainer. So
nit: the name seems not good, because it's not a meta. so NativeLogBlockContainer ?
Furthermore, IMO, another reasonable refactoring, because the data part is not changed. Under LogBlockContainer, only the meta part is a plugin for LogBlock and rocksdb. So how about extracting a LogBlockMeta which defines all all meta's logic actions, LogBlockMeta is part of LogBlockContainer, And we implements several class such as LogBlockMeta and RocksMeta based LogBlockMeta.
--
To view, visit http://gerrit.cloudera.org:8080/19610
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb199de95973aaeba76947ad095907272d84ca67
Gerrit-Change-Number: 19610
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Tue, 04 Apr 2023 10:55:33 +0000
Gerrit-HasComments: Yes