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 1: (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: RETURN_NOT_OK(hostport_->ResolveAddresses(&addrs)); You can't call this out of Peer::ProcessResponse because that runs on the reactor thread, which must only do non-blocking operations (and this may block). See the comment at the end of Peer:ProcessResponse for more details. You'll probably need to employ a similar strategy of deferring the work after the reactor thread is done and into another thread pool. I was kind of surprised tests passed with this, but then I saw that there's no call to ThreadRestrictions::AssertWaitAllowed() within the DNS resolution code paths. I'll add one in a separate change. http://gerrit.cloudera.org:8080/#/c/14420/1/src/kudu/consensus/consensus_peers.cc@524 PS1, Line 524: consensus_proxy_.reset(new ConsensusServiceProxy(messenger, addrs[0], hostport_->host())); What are the synchronization requirements around PeerProxy? Could multiple threads end up in here concurrently? -- 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: 1 Gerrit-Owner: Yifan Zhang <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 14 Oct 2019 04:00:06 +0000 Gerrit-HasComments: Yes
