From 5785a57f2912ab369dc859cc5263c66c4339bf6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Uzarski?= Date: Tue, 8 Oct 2024 15:44:48 +0200 Subject: [PATCH] lbp: don't use '_' match in reliable_latency_measure Since last time, during error refactor I introduced a silent bug to the code (https://github.com/scylladb/scylla-rust-driver/pull/1075), I'd like to prevent that from happening in the future. This is why we replace a `_` match with explicit error variants in `reliable_latency_measure` function. We also enable the `wildcard_enum_match_arm` clippy lint here. --- .../src/transport/load_balancing/default.rs | 50 +++++++++++++++---- 1 file changed, 41 insertions(+), 9 deletions(-) diff --git a/scylla/src/transport/load_balancing/default.rs b/scylla/src/transport/load_balancing/default.rs index 51db7f97f..773b0c19c 100644 --- a/scylla/src/transport/load_balancing/default.rs +++ b/scylla/src/transport/load_balancing/default.rs @@ -2838,29 +2838,61 @@ mod latency_awareness { } pub(crate) fn reliable_latency_measure(error: &QueryError) -> bool { + // Do not remove this lint! + // It's there for a reason - we don't want new variants + // automatically fall under `_` pattern when they are introduced. + #[deny(clippy::wildcard_enum_match_arm)] match error { - // "fast" errors, i.e. ones that are returned quickly after the query begins + // "fast" errors, i.e. ones that appeared on driver side, before sending the request QueryError::BadQuery(_) | QueryError::CqlRequestSerialization(_) | QueryError::BrokenConnection(_) | QueryError::ConnectionPoolError(_) | QueryError::EmptyPlan - | QueryError::UnableToAllocStreamId - | QueryError::DbError(DbError::IsBootstrapping, _) - | QueryError::DbError(DbError::Unavailable { .. }, _) - | QueryError::DbError(DbError::Unprepared { .. }, _) - | QueryError::DbError(DbError::Overloaded { .. }, _) - | QueryError::DbError(DbError::RateLimitReached { .. }, _) => false, + | QueryError::UnableToAllocStreamId => false, // "slow" errors, i.e. ones that are returned after considerable time of query being run - QueryError::DbError(_, _) - | QueryError::CqlResultParseError(_) + QueryError::CqlResultParseError(_) | QueryError::CqlErrorParseError(_) | QueryError::BodyExtensionsParseError(_) | QueryError::MetadataError(_) | QueryError::ProtocolError(_) | QueryError::TimeoutError | QueryError::RequestTimeout(_) => true, + + // handle db errors + QueryError::DbError(db_error, _) => { + // Do not remove this lint! + // It's there for a reason - we don't want new variants + // automatically fall under `_` pattern when they are introduced. + #[deny(clippy::wildcard_enum_match_arm)] + match db_error { + // An errors that appeared on server side, but did not require selected node + // to contact other nodes (i.e. fast server side errors). + DbError::IsBootstrapping + | DbError::Unavailable { .. } + | DbError::Unprepared { .. } + | DbError::Overloaded { .. } + | DbError::RateLimitReached { .. } => false, + + // Rest of server side errors that take considerable amount of time to be returned + DbError::SyntaxError + | DbError::Invalid + | DbError::AlreadyExists { .. } + | DbError::FunctionFailure { .. } + | DbError::AuthenticationError + | DbError::TruncateError + | DbError::ReadTimeout { .. } + | DbError::WriteTimeout { .. } + | DbError::ReadFailure { .. } + | DbError::WriteFailure { .. } + | DbError::ServerError + | DbError::ProtocolError + | DbError::Unauthorized + | DbError::ConfigError + | DbError::Other(_) => true, + } + } } } }