InmemTransport: respect timeout when sending #313
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
|
Hmm - I think the integration test that failed is flaky. If I run it in a loop against master it fails within 10 runs. |
babbageclunk
commented
Mar 19, 2019
api.go
Outdated
| localAddr: localAddr, | ||
| logger: logger, | ||
| logs: logs, | ||
| protocolVersion: protocolVersion, |
There was a problem hiding this comment.
I didn't mean to include this formatting change, the file got gofmt'd when I was adding debugging.
InmemTransport would block indefinitely when trying to make an RPC to a peer who has already shut down. This was causing our tests using it to hang sometimes when stopping raft nodes - guarding the send with a timeout (like there is on the receive) resolves the problem.
babbageclunk
force-pushed
the
inmemtransport-fix
branch
from
efef81e to
834fca2
Compare
babbageclunk
added a commit
to babbageclunk/juju
that referenced
this pull request
Mar 19, 2019
This fixes the intermittent raftflag test hang which was caused by the raft InmemTransport blocking indefinitely when trying to send an RPC and the other end has stopped already. Proposed upstream at hashicorp/raft#313, but using the juju/raft fork until that is merged.
jujubot
added a commit
to juju/juju
that referenced
this pull request
Mar 19, 2019
#9909 ## Description of change Updates hashicorp/raft dependency to use our own fork to respect timeouts when sending RPC requests (at least until our PR is merged upstream). See hashicorp/raft#313 for more detail. ## QA steps Running the raftflag worker test under stress doesn't hang anymore. ## Documentation changes None ## Bug reference None
|
Thanks for fixing this odd timing bug in our testing system! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
InmemTransport would block indefinitely when trying to make an RPC to a peer who has already shut down. This was causing some of Juju's tests for our raft machinery to hang sometimes when stopping raft nodes - see #312 for code that will reproduce the situation.
Guarding the send with a timeout (like there is on the receive) fixes it and seems like the right thing to do here.