Adar Dembo has posted comments on this change.

Change subject: KUDU-1125 (part 1) catalog_manager: try to avoid unnecessarily 
rewriting tablet info
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/6916/3/src/kudu/master/sys_catalog.cc
File src/kudu/master/sys_catalog.cc:

Line 114:   return md.Compare(prev_pb, new_pb);
I presume this is equivalent to prev_pb == new_pb? But you're going through the 
MessageDifferencer for the diff_str!= null case?

Bearing in mind that the diff_str!=null case is very rare (VLOG only), would it 
be faster to use equality? Or are the two approaches equivalent?


Line 692:     VLOG(2) << "Adding tablet " << tablet->tablet_id() << " in 
catalog: "
Any particular reason you want this VLOG but not one for ReqAddTable()?


http://gerrit.cloudera.org:8080/#/c/6916/3/src/kudu/master/sys_catalog.h
File src/kudu/master/sys_catalog.h:

Line 164:   const scoped_refptr<tablet::TabletReplica>& tablet_replica() const {
Since this is only being used in a test, an alternative would be to keep it 
private and add FRIEND_TEST() as needed.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0de7189b8f1dbeea55172929396b73fd92fd62ba
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to