Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17845 )

Change subject: [fs] Refactor fs module
......................................................................


Patch Set 3: Code-Review+2

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/17845/1/src/kudu/fs/fs_report.cc@42
PS1, Line 42: MERGE_ENTRIE
> I'd like to rename it to 'MERGE_ENTRIES_FROM', 'ENTRIES' is implicit in mac
If you are OK with the smell of this macro relying on the naming of 'entries' 
field in the corresponding structures, why not to go further and rely on the 
naming of the 'other' parameter in the corresponding MergeFrom() methods?  
Dropping the 'other' macro parameter and making it implicit as well (as it is 
for 'entries') would yield a macro without any parameters:

MERGE_ENTRIES_FROM_OTHER()

:)

It's just a nit anyways, so feel free keep it as is.


http://gerrit.cloudera.org:8080/#/c/17845/1/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:

http://gerrit.cloudera.org:8080/#/c/17845/1/src/kudu/server/server_base.cc@584
PS1, Line 584:     LOG(INFO) << "This appears to be a new deployment of Kudu; 
creating new FS layout";
             :     is_first_run_ = true;
> Emmm, in fact this is a small fix. Some one removed a corrupt .metadata fil
Yep, it seems the case you mentioned is rather commanding the usage of the 
Status::Corruption() instead of Status::NotFound() status.

I agree -- adding a warning into the exact site looks better.



--
To view, visit http://gerrit.cloudera.org:8080/17845
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa45637494b36cc1f4f00affdf995a25eb6231bc
Gerrit-Change-Number: 17845
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <[email protected]>
Gerrit-Comment-Date: Wed, 15 Sep 2021 17:37:02 +0000
Gerrit-HasComments: Yes

Reply via email to