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 <laiyingc...@apache.org> Gerrit-Reviewer: Abhishek Chennaka <achenn...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <ale...@apache.org> Gerrit-Reviewer: Ashwani Raina <ara...@cloudera.com> Gerrit-Reviewer: Attila Bukor <abu...@apache.org> Gerrit-Reviewer: KeDeng <kdeng...@gmail.com> 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-Reviewer: Yuqi Du <shenxingwuy...@gmail.com> Gerrit-Comment-Date: Tue, 04 Apr 2023 10:55:33 +0000 Gerrit-HasComments: Yes
