Adar Dembo has posted comments on this change.

Change subject: Forward compat for changing TSRegistrationPB
......................................................................


Patch Set 3:

(2 comments)

How about a unit test showing a successful reregistration when something other 
than the addresses has changed?

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

Line 58: // Compares two repeated HostPortPB fields. Returns true if equal, 
false otherwise.
This will return false if the two containers have the same contents, but are 
out of order. Perhaps we should do a set comparison?


http://gerrit.cloudera.org:8080/#/c/4062/3/src/kudu/util/net/net_util.h
File src/kudu/util/net/net_util.h:

Line 56:     return port() == other.port() && host() == other.host();
Any particular reason to compare the port before the host? I'd think the latter 
is more likely to be "different" and so should come first.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I794517d200b1c9ad74b3897b17aa808d68d7924b
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mpe...@apache.org>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to