Yingchun Lai 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:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/19610/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19610/1//COMMIT_MSG@12
PS1, Line 12: a l
> a
Done


http://gerrit.cloudera.org:8080/#/c/19610/1//COMMIT_MSG@13
PS1, Line 13: a sequentially written file
> a sequentially written file
Done


http://gerrit.cloudera.org:8080/#/c/19610/1//COMMIT_MSG@14
PS1, Line 14: that's what LogBlockConta
> that's what LogBlockContainer used to be prior to this change
Done


http://gerrit.cloudera.org:8080/#/c/19610/1//COMMIT_MSG@16
PS1, Line 16:
            : Change-Id: Ieb199de95973aaeba76947ad095907272d84ca67
            :
> +1
Sure, it's meaningful. I've updated the code to make the base class 
LogBlockContainer only care of .data file and general metadata logic, and moved 
all .metadata file related code to derived class LogBlockContainerNativeMeta.
You can preview the last result of the whole code in 
https://gerrit.cloudera.org/c/18569/.


http://gerrit.cloudera.org:8080/#/c/19610/1//COMMIT_MSG@19
PS1, Line 19:
> updates
Done


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

http://gerrit.cloudera.org:8080/#/c/19610/1/src/kudu/fs/log_block_manager.cc@210
PS1, Line 210: using kudu::fs::internal::LogWritableBlock;
             : using kudu::pb_util::ReadablePBContainerFile;
             : using kudu::pb_util::WritablePBContainerFile;
             : using std::accumulate;
             : using std::map;
             : using std::optional;
             : using std::set;
             : using std::shared_ptr;
             : using std::string;
             : using std::unique_ptr;
             : using std::unordered_map;
             : using std::unordered_set;
             : using std::vector;
             : using strings::Substitute;
             :
             : namespace kudu {
             :
             : namespace fs {
> nit: move these declarations out of the namespace
Done


http://gerrit.cloudera.org:8080/#/c/19610/1/src/kudu/fs/log_block_manager.cc@2568
PS1, Line 2568:
> nit: opening
Done



--
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 <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Ashwani Raina <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: KeDeng <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <[email protected]>
Gerrit-Reviewer: Yifan Zhang <[email protected]>
Gerrit-Reviewer: Yingchun Lai <[email protected]>
Gerrit-Reviewer: Yuqi Du <[email protected]>
Gerrit-Comment-Date: Sat, 25 Mar 2023 09:16:13 +0000
Gerrit-HasComments: Yes

Reply via email to