Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

RPC Timeout/Retries account for blocking requests #8978

Closed
wants to merge 1 commit into from

Conversation

joel0
Copy link
Contributor

@joel0 joel0 commented Oct 17, 2020

I've ported the Nomad fixes from hashicorp/nomad#8921 and hashicorp/nomad#9064.

The only major differences:

  • Consul has a configurable MaxQueryTime and DefaultQueryTime, which is a const in Nomad.
  • agent/consul/rpc.go:ForwardRPC() didn't start the firstCheck timer until after the first call. I did not see any motivation for it to be different than agent/consul/client.go:RPC() and Nomad's client/rpc.go:RPC(), so I moved it to the top of the function.

@joel0
Copy link
Contributor Author

joel0 commented Oct 19, 2020

I see that I broke some tests with this. I'm working on fixing those.

I have pushed a fix that moves the timeout check to avoid retrying past the timeout. The tests appear to be passing on my machine*.

*I could not get make test to work. Using gotestsum -- -p 2 ./agent returned 3132 tests passed and 1 skipped in 91s.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant