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