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 5: (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@524 PS1, Line 524: ConsensusResponsePB* response, > I think only one thread would end up in here, so I removed the lock. I looked into this too and I think I disagree with your assessment. - UpdateProxy is called asynchronously on the Raft thread pool with a token that allows for concurrent execution. This means there could be multiple UpdateProxy calls racing with one another (if Peer::SendNextRequest was called and failed with a network error before the first UpdateProxy finished). - There's also StartElection, which runs on the Raft thread pool at any time and tries to access the RpcPeerProxy. So we would need a lock here. Though replacing the proxy outright is what's making this hard. I wonder if just going down KUDU-75 would be easier? http://gerrit.cloudera.org:8080/#/c/14420/5/src/kudu/consensus/consensus_peers.cc File src/kudu/consensus/consensus_peers.cc: http://gerrit.cloudera.org:8080/#/c/14420/5/src/kudu/consensus/consensus_peers.cc@476 PS5, Line 476: p->proxy_->UpdateProxy(p->messenger_); WARN_NOT_OK if this fails. -- 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: 5 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: Fri, 18 Oct 2019 20:07:36 +0000 Gerrit-HasComments: Yes
