-
Notifications
You must be signed in to change notification settings - Fork 109
Give vitess a chance to enforce connection timeouts if we've been waiting for a row for longer #801
Conversation
Signed-off-by: Juanjo Alvarez <juanjo@sourced.tech>
Signed-off-by: Juanjo Alvarez <juanjo@sourced.tech>
Signed-off-by: Juanjo Alvarez <juanjo@sourced.tech>
…usage Signed-off-by: Juanjo Alvarez <juanjo@sourced.tech>
Signed-off-by: Juanjo Alvarez <juanjo@sourced.tech>
|
||
rowchan := make(chan sql.Row) | ||
errchan := make(chan error) | ||
quit := make(chan struct{}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[optional] You could put defer close(quit)
here, instead of calling it in multiple places below. As far as I can see, anytime this function returns, you'd want quit
closed anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to make it explicit so the code reader can see where we are actively signaling the goroutine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (as far as I understand what the function does)
Signed-off-by: Juanjo Alvarez <juanjo@sourced.tech> Refactor rowloop select Signed-off-by: Juanjo Alvarez <juanjo@sourced.tech>
@ajnavarro dont merge, maybe my next PR will make this unnecesary. |
Signed-off-by: lwsanty <lwsanty@gmail.com>
Closing, definitely superseeded by the next PR that improves also this part and fixes a bug. |
Fixes #799
Related issue: https://github.com/src-d/gitbase-spark-connector-enterprise/issues/81
This PR abort further
rowIterator.Next()
calls if the time waiting for a single row is bigger than the configuredConnReadTimeout
thus giving a change to Vitess to call theHandler.CloseConnection()
callback when it detects the timeout. It does it by running the calls to the iterator on a looped goroutine with anTimer
which checks that we didn't exceed the timeout.This doesn't fix issue #800 which implies detecting a closed connection before the timeout expires and killing the associated queries, but serves to somewhat workaround it by configuring a timeout.