-
Notifications
You must be signed in to change notification settings - Fork 85
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
context for commands (deadline / cancellation) #47
Comments
That's a good question. At first glance, I'm not sure how useful it'd be, because the protocol doesn't have any way to cancel requests (other than to close the connection). So in the case where a deadline is exceeded or the context is explicitly canceled, there is a question of what this package should do: should it close the connection, or perhaps leave the canceled request pending? Both of those have significant drawbacks. It is not obvious to me what the "correct" behavior should be. |
One thing we could do pretty easily would be to convert a context's deadline into a timeout for a reserve command, but there is still no way in the protocol to cancel a reserve command if its context is explicitly canceled. |
Hmm yes you have a point about not being able to cancel requests at the protocol level... But then I guess this is more generally true for any non-idempotent request with for example an HTTP request, and go's HTTP client still lets you cancel those using the context. The situation I have in mind is a service with a given time budget (SLA), which needs to enqueue a job: with the current API, ensuring that my I don't have experience using beanstalkd in production (yet :-) ) so I might be missing something completely here, any input from more seasoned users is welcome! |
I think idempotence is not the main problem. Non-idempotent HTTP requests can be canceled. The problem here is that the only way to cancel a beanstalkd request is to close the connection, and that can affect other commands that the application might not have wanted to cancel. So I think that is not a good approach for this general-purpose client library. (But, as you point out, it would be reasonable for an application to set a read deadline on the connection, to produce a similar effect.) The other way this package might handle a canceled context is to abandon the command. It could return to the call site early so the application can continue, while leaving the connection open and leaving the command pending. This package would still internally have to wait for and read the response, so that subsequent commands still work. I think that would be reasonable. An application can get that effect today by doing something like this: var id uint64
var err error
completed := make(chan struct{})
go func() {
id, err = conn.Put(…)
close(completed)
}()
select {
case <-completed:
// use id and err…
case <-ctx.Done():
// canceled or timed out…
} If we added context support to go-beanstalk, the implementation would probably look a lot like this. Note that this approach introduces concurrency around the arguments to Put, and that would probably still be true even with a version baked in to this package. That might be a surprising pitfall. We should think about it carefully. |
Is there a plan (or wish?) to add APIs for
Put
etc that would take acontext.Context
in, and handle cancellation/deadlines? Put another way, how would the current API deal with (for example) stalled connections?The text was updated successfully, but these errors were encountered: