errors: fix driver's logic that bases on error variants returned from query execution

muzarski commented 2 months ago

Ref: https://github.com/scylladb/scylla-rust-driver/issues/519

Motivation

The error refactor that I'm currently working on, surprisingly changed driver's logic in some cases where the decision is made based on the error returned from query execution. There are three places where driver's logic was silently altered:

retry_policy -> driver should retry execution in case of IoError
use_keyspace -> if query execution returned IoErrors for some (not all) nodes/connections, these errors should be ignored
speculative_exeuction -> driver should ignore IoErrors in this context

During error refactor I changed all places where QueryError::IoError was constructed. Now all of these IoErrors are represented as BrokenConnectionError and ConnectionPoolError types (and their variants). These types represent an error which implies that a corresponding connection/node is broken/unreachable.

Changes

Adjusted driver's logic in places mentioned above.
Removed [Query/NewSession]Error::IoError variants altogether. They are no longer constructed anywhere - they were replaced by ConnectionPoolError and BrokenConnection variants during previous refactors.

Pre-review checklist

[x] I have split my patch into logically separate commits.
[x] All commit messages clearly explain what they change and why.
~[ ] I added relevant tests for new features and bug fixes.~
[x] All commits compile, pass static checks and pass test.
[x] PR description sums up the changes and reasons why they should be introduced.
~[ ] I have provided docstrings for the public items that I want to introduce.~
~[ ] I have adjusted the documentation in ./docs/source/.~
[x] I added appropriate Fixes: annotations to PR description.

github-actions[bot] commented 2 months ago

cargo semver-checks detected some API incompatibilities in this PR. Checked commit: ce0fdca7f917598efd5b60e0c96316c2648398a4

See the following report for details:

cargo semver-checks output

``` ./scripts/semver-checks.sh --baseline-rev 44a309343e897da31ddfe226a94af8e1f9561b0b + cargo semver-checks -p scylla -p scylla-cql --baseline-rev 44a309343e897da31ddfe226a94af8e1f9561b0b Cloning 44a309343e897da31ddfe226a94af8e1f9561b0b Parsing scylla v0.14.0 (current) Parsed [ 21.605s] (current) Parsing scylla v0.14.0 (baseline) Parsed [ 20.108s] (baseline) Checking scylla v0.14.0 -> v0.14.0 (no change) Checked [ 0.103s] 89 checks: 88 pass, 1 fail, 0 warn, 0 skip --- failure enum_variant_missing: pub enum variant removed or renamed --- Description: A publicly-visible enum has at least one variant that is no longer available under its prior name. It may have been renamed or removed entirely. ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.35.0/src/lints/enum_variant_missing.ron Failed in: variant NewSessionError::IoError, previously in file /home/runner/work/scylla-rust-driver/scylla-rust-driver/target/semver-checks/git-44a309343e897da31ddfe226a94af8e1f9561b0b/edcf569f3961e73847e8e0c9f14adf367bbeb770/scylla/src/transport/errors.rs:212 variant QueryError::IoError, previously in file /home/runner/work/scylla-rust-driver/scylla-rust-driver/target/semver-checks/git-44a309343e897da31ddfe226a94af8e1f9561b0b/edcf569f3961e73847e8e0c9f14adf367bbeb770/scylla/src/transport/errors.rs:57 Summary semver requires new major version: 1 major and 0 minor checks failed Finished [ 41.871s] scylla Parsing scylla-cql v0.3.0 (current) Parsed [ 10.192s] (current) Parsing scylla-cql v0.3.0 (baseline) Parsed [ 10.193s] (baseline) Checking scylla-cql v0.3.0 -> v0.3.0 (no change) Checked [ 0.099s] 89 checks: 89 pass, 0 skip Summary no semver update required Finished [ 20.540s] scylla-cql make: *** [Makefile:61: semver-rev] Error 1 ```

Lorak-mmk commented 2 months ago

The problems you mentioned sound like serious bugs. Are they present in 0.14 or only on main?

wprzytula commented 2 months ago

The problems you mentioned sound like serious bugs. Are they present in 0.14 or only on main?

From what I see, they are directly caused by PRs merged in recent days.

muzarski commented 2 months ago

The problems you mentioned sound like serious bugs. Are they present in 0.14 or only on main?

It was merged yesterday: https://github.com/scylladb/scylla-rust-driver/pull/1067, so only main is affected

muzarski commented 2 months ago

v1.1: Deduplicated the logic shared between ClusterWorker and PoolRefiller

scylladb / scylla-rust-driver