Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14420 )
Change subject: KUDU-1620: Update consensus peer proxies when a network error occurs ...................................................................... Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/14420/1/src/kudu/consensus/consensus_peers.cc File src/kudu/consensus/consensus_peers.cc: http://gerrit.cloudera.org:8080/#/c/14420/1/src/kudu/consensus/consensus_peers.cc@518 PS1, Line 518: consensus_proxy_(std::move(DCHECK_NOTNULL(consensus_proxy))) { > KUDU-75 says "do DNS resolution inline with calls", I thought it means reso I think KUDU-75 was filed sufficiently long ago and is sufficiently terse that precisely how it should work is open to interpretation. The only requirement is that proxy construction take a HostPort instead of a Sockaddr and that the Proxy be responsible for DNS resolution too (today Proxy callers are responsible). Maybe it always does a DNS resolution (and DNS cache lookup) in each call. Maybe it uses a Sockaddr as a "local cache", refreshing it automatically on network error (or maybe it forces callers to refresh manually). Agree that callback functions need to change if refreshing is automatic. One option is for the Proxy to bind its own callback around the user-provided callback. The Proxy's callback could look for NetworkErrors and either reresolve DNS or, if reresolution doesn't help, send the NetworkError up to the user's callback. http://gerrit.cloudera.org:8080/#/c/14420/1/src/kudu/consensus/consensus_peers.cc@524 PS1, Line 524: const rpc::ResponseCallback& callback) { > I'm not sure if multiple threads end up Peer::ProcessResponse, if not, the OK. Could you try to figure this out? -- To view, visit http://gerrit.cloudera.org:8080/14420 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ied476d302b1d9489a030847b5da74d60d0c1d8e9 Gerrit-Change-Number: 14420 Gerrit-PatchSet: 3 Gerrit-Owner: Yifan Zhang <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Comment-Date: Tue, 15 Oct 2019 20:50:17 +0000 Gerrit-HasComments: Yes
