feat: support retry #239
feat: support retry #239
Conversation
sqllogictest/src/runner.rs
Outdated
| let result = self.run_async(record.clone()).await; | ||
| if result.is_ok() { | ||
| return result; | ||
| } |
There was a problem hiding this comment.
Do we distinguish connection error and application error? I think it's not worth retrying if there's connection error.
There was a problem hiding this comment.
Considering that tests with retry should only take a very small proportion, I don't think it's necessary.
sqllogictest/src/runner.rs
Outdated
| @@ -875,6 +877,35 @@ impl<D: AsyncDB, M: MakeConnection<Conn = D>> Runner<D, M> { | |||
| } | |||
| } | |||
|
|
|||
| /// Run a single record with retry. | |||
| pub async fn run_async_with_retry( | |||
There was a problem hiding this comment.
It seems confusing to introduce a new method specifically to make retry work. For example, run_parallel_async does not hit this path.
I suggest moving the retry logic into apply_record instead, given that it's a per-record modifier.
There was a problem hiding this comment.
I feel that it's a bit confusing to do retry in sqllogictest --override, so I did it outside of apply_record.
For example, run_parallel_async does not hit this path.
How about rename run_async_with_retry to run_async to ensure all path are covered?
fuyufjh
force-pushed
the
eric/support_retry
branch
2 times, most recently
from
8eb4368 to
3529c2b
Compare
Signed-off-by: Eric Fu <fuyufjh@gmail.com>
fuyufjh
force-pushed
the
eric/support_retry
branch
from
3529c2b to
f138b6c
Compare
Signed-off-by: Eric Fu <fuyufjh@gmail.com>
fuyufjh
force-pushed
the
eric/support_retry
branch
from
373f252 to
1458d31
Compare
|
published |
Resolves #238.
Also removed
labelfromQueryExpect::Resultsbecause it's never used.