Cancelling queries #422
Cancelling queries #422
Conversation
|
Hi, is there anyone to merge/review this PR? |
|
Is anybody here? |
dancannon
left a comment
dancannon
left a comment
There was a problem hiding this comment.
Looks great, thanks for spending the time working on this (sorry for taking so long to review)! I left a couple of comments, let me know what you think.
| "github.com/Sirupsen/logrus" | ||
| "github.com/cenkalti/backoff" | ||
| "github.com/hailocab/go-hostpool" | ||
| "golang.org/x/net/context" |
There was a problem hiding this comment.
I would recommend using the built-in context package.
There was a problem hiding this comment.
Most of the libraries use x/net/context, such as grpc-go, I'm not sure about old version of go, It can be difficult to pass x/net/context to context argument.
| } | ||
|
|
||
| func (c *Connection) contextFromConnectionOpts() context.Context { | ||
| sum := c.opts.ReadTimeout + c.opts.WriteTimeout |
There was a problem hiding this comment.
I am a little worried that this behaviour might be a little unexpected, I think it might make more sense to keep the previous behaviour if no context is passed to a query (I am just trying to think about existing code that might be affected)
There was a problem hiding this comment.
Previous behaviour is a quite useless. If server has continuous queries flow, socket's deadline updates every time so that even first query will not timeout never due to this reason. For me it seems like a bug.
And this is unexpected if you don't know that timeouts from config are just passed to socket.
The new behaviour is that every query ends not more than read+write timeout.
There was a problem hiding this comment.
Ok fair enough, I am still a little worried about changing the existing behaviour but hopefully the new behaviour should not cause too many issues.
|
Thanks. How about new tag? v3.0.3? for gopkg.in |
2 participants
As discussed in #420 , now
RunOptscontainsContextparam, which affects all the query including reading from cursor. If context deadline exceeds stop-query will be sent.If there is no context, then it will be formed from
ConnectionOptssocket timeouts.