-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ability to cancel blocking queries from the API client #8009
Comments
I really like this idea. Something to consider is that the API calls are mostly proxies to the RPC calls, which don't currently have a cancellation mechanism: #7119. |
@tgross I sent a PR for canceling HTTP. I am curious about RPC calls you think it doesn't cover. Even missing some API calls I think this is super valuable because it allows calls to a lot of endpoints with ?waitIndex to be cancel-able. For example, we have code that watches new jobs using waitIndex, but to make it cancelable we have to retry periodically which is surprisingly CPU intensive (JSON decode is slow) and this change is invaluable for us. |
Hi @benbuzbee! The problem I'm referring to is that almost all (maybe even all?) Nomad's HTTP API calls result in an RPC call, and our RPC doesn't have cancellation. So if we make the HTTP API cancellable, that'll "work" inasmuch that your code will drop the request. But that'll result in a cascade of connection errors between server nodes, especially in the common case where the RPC is being forwarded to the leader. That's probably safe to do for |
#8836 has been merged and this will ship in 0.13 |
I'm going to lock this issue because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active issues. |
Nomad version
0.11.x
Issue
The Nomad API (and the SDK in the
api
package) supports blocking queries. However, the SDK does not provide the ability to cancel blocking queries. The GoLang HTTP package used to implement the SDK supports request cancellation:https://golang.org/pkg/net/http/#Request.WithContext
It seems like the easiest way to do this is to modify
QueryOptions
to include an optionalContext
.The text was updated successfully, but these errors were encountered: