Todd Lipcon has posted comments on this change.

Change subject: log block manager: reopen container metadata writers at the OS 
level
......................................................................


Patch Set 1:

(2 comments)

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

Line 2201:   if (file_cache_) {
semi-unrelated question: now that we've had the file cache for a while and it 
seems stable, can we start just assuming it's always on and reduce some of 
these branches? Especially considering we don't test the "no cache" case, seems 
like cruft we can remove


http://gerrit.cloudera.org:8080/#/c/6825/2/src/kudu/util/pb_util.h
File src/kudu/util/pb_util.h:

PS2, Line 280: Init
maybe best to rename this to OpenNew and the one below OpenForAppend or 
OpenExisting? Despite the nice comments, I found the names a bit confusing.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6029d499422f5a2db036d796c7e208f2af71c394
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: David Ribeiro Alves <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to