Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12138 )
Change subject: Support location awareness in READ_CLOSEST for the C++ client ...................................................................... Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/12138/1/src/kudu/client/client-internal.cc File src/kudu/client/client-internal.cc: http://gerrit.cloudera.org:8080/#/c/12138/1/src/kudu/client/client-internal.cc@412 PS1, Line 412: local.push_back(rts); Nit: emplace_back for new code. http://gerrit.cloudera.org:8080/#/c/12138/1/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/12138/1/src/kudu/client/client.h@472 PS1, Line 472: ///< Local replicas are considered the closest, : ///< followed by replicas in the same location as the : ///< client, followed by all other replicas. If there are : ///< multiple closest replicas, one is chosen randomly. Would you consider this to be a breaking change? The answer and justification would be good to add to the commit message. http://gerrit.cloudera.org:8080/#/c/12138/1/src/kudu/client/meta_cache.h File src/kudu/client/meta_cache.h: http://gerrit.cloudera.org:8080/#/c/12138/1/src/kudu/client/meta_cache.h@109 PS1, Line 109: // Return a copy of this tablet server's location, as assigned by the master. The changes here (removal of constness, returning a copy) suggest that the location of a tserver is no longer immutable. That's true to the extent that a location could change from empty (i.e. unassigned) to "some location", but once a location has been assigned, it may not change to some other location, right? As written such a change is legal, but that isn't allowed by the server, right? http://gerrit.cloudera.org:8080/#/c/12138/1/src/kudu/integration-tests/location_assignment-itest.cc File src/kudu/integration-tests/location_assignment-itest.cc: PS1: Could you rename this in a separate commit? It's showing up as a brand new file so it's hard to review just the changes. -- To view, visit http://gerrit.cloudera.org:8080/12138 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2c6bcc7479c5cf2e17cb6e368ca89a1eb7f21713 Gerrit-Change-Number: 12138 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 02 Jan 2019 18:16:05 +0000 Gerrit-HasComments: Yes
