Improve communication error tolerance for long running queries #6784
Conversation
Increase error tolerance for communication between coordinator and worker up to the length of time the query ran for before it hit failures
| private synchronized Duration elapsedErrorDuration() | ||
| { | ||
| if (errorStopwatch.isRunning()) { | ||
| errorStopwatch.stop(); |
There was a problem hiding this comment.
It looks like this only counts time while the request is outstanding. My PR changes that to count total wall time since it started failing, which seems like it makes more sense to me. However, I'm not sure if this was intentionally written to only count the time when the request running
There was a problem hiding this comment.
This code is ancient, and the new Backoff class is much better, so let's just use that
|
@dain, there's one other behavior change due to a refactoring I made, and it seems like the more appropriate behavior. I noted it with a comment, so please let me know if that's the right thing to do |
| @@ -64,6 +66,7 @@ public Backoff(Duration maxFailureInterval, Ticker ticker, Duration... backoffDe | |||
| } | |||
|
|
|||
| this.lastSuccessTime = this.ticker.read(); | |||
| this.createTime = this.ticker.read(); | |||
There was a problem hiding this comment.
I'd pass this into the constructor so this can be the start time of the query instead of the start time of this task. This will help deal with new tasks added late in the query due to the phased scheduler.
| minErrorDuration, | ||
| maxErrorDuration, | ||
| ticker, | ||
| new Duration(0, MILLISECONDS), |
There was a problem hiding this comment.
I'd use the default back off schedule. This one seems way to aggressive.
There was a problem hiding this comment.
This is how the code works today, which is why I used it. I agree it seems aggressive though. I'll change it to the default one
| private synchronized Duration elapsedErrorDuration() | ||
| { | ||
| if (errorStopwatch.isRunning()) { | ||
| errorStopwatch.stop(); |
There was a problem hiding this comment.
This code is ancient, and the new Backoff class is much better, so let's just use that
Also refactor HttpPageBufferClient to use Backoff
This dynamically adjusts the tolerance for communication errors based on the length of time that the query has run (without error)