246df0a75a63b2db154f97f143c1c3ffebb30d4b | Author: Daniel Lohse <info@asapdesign.de>
| 2018-08-27 13:14:42+02:00
Revert #1164 and #1151 (#1166)
* Revert "Fix bug in query executor that prevented retries from ceasing (#1164)"
This reverts commit 0d4b564ca003d3cba68f2df1be788df6ec8d7f02.
* Revert "Provide a way to limit per-query-attempt deadlines (#1151)"
This reverts commit e48272ffe6343736ca6acace2c51a312c7a58baf.
0d4b564ca003d3cba68f2df1be788df6ec8d7f02 | Author: Daniel Lohse <info@asapdesign.de>
| 2018-08-25 17:47:38+02:00
Fix bug in query executor that prevented retries from ceasing (#1164)
If a retry policy is defined for the session/query, retries wouldn't
stop even if the retry policy returned false in the call to `Attempt()`.
The bug was introduced in #1151. As long as queries keep failing,
the query would be retried. The unit test that checks the retries
being executed now checks for an exact number of query retries, making
it a regression test as well.
e48272ffe6343736ca6acace2c51a312c7a58baf | Author: Daniel Lohse <info@asapdesign.de>
| 2018-08-21 22:13:07+02:00
Provide a way to limit per-query-attempt deadlines (#1151)
* Limit query fetch timeout
* Simplify query executor
* Bail out of Conn.exec if context was canceled/expired
This happens just before anything is put onto the wire.
* Add GetContext method to the RetryableQuery interface
This enables both the query executor as well as the retry policy to
access the context.
* Always set the host on the iter and exit early if query was successful
There's really no need to evaluate the retry policy if the query
succeeded.
* Rework query executor to respect per attempt timeouts
This adds a private wrapper around the optional retry policy and
handles the context that potentially was an attempt timeout.
* Collapse context check in retry policy wrapper, rework some comments
* Add docs to exported methods
* Unexport internal method in retry policy wrapper
* Add ourselves to the AUTHORS file
* Replace retryPolicyWrapper struct with checkRetryPolicy func
* Cleanup query attempt test
* Move AttemptTimeout to the retry policy via an optional interface
A retry policy may now implement an `AttemptTimeout` method which is
used to control per query attempt timeouts. It's part of an optional
interface in order for this feature to not break backwards compatibility.
* Unexport `numRetries` field in test retry policy
It's really not necessary for this to be exported.
* Add assertion to query attempt unit test to guard against regressions
This makes sure a number of query attempts are executed.
* Rename `GetContext()` to `Context()` in RetryableQuery interface
This is more in line with Go style.
* Embed `RetryPolicy` interface in extension interface
This doesn't change anything code-wise and is more source documentation
than anything else but I like the explicitness.
* Rework attempt timeout handling unit test
It now uses the existing slow query test query. I introduced a
dedicated atomic counter for the number of queries done in the
test server (the other counter `nreq` also includes other ops
besides queries, making assertions for query retries brittle
and opaque).
* Rework attempt timeout implementation
This replaces the attempt timeout via `context.WithDeadline` with a
cancelable context and a recycled timer on the query akin to the
strategy used in #661. The unit test fix is actually a side effect
of the overhead saved by recycling the timer. Previously each attempt
took ~27ms (on a 25ms attempt timeout) because of the overhead of
`context.WithDeadline`. It's now more like 25.2ms which occasionally
resulted in a fifth query attempt being made because the 100ms query
context deadline timer fired a millisecond later.