From fe57308915ecce7cd1481bf39147c54293fcb9df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20Bary=C5=82a?= Date: Tue, 19 Mar 2024 17:57:44 +0100 Subject: [PATCH 1/3] session_test: Increase tracing info timeout Default values for fetching tracing info are quite strict in the driver: it will make 5 attempts to fetch the data, waiting 3ms between attempts. For Scylla this is enough, but for Cassandra it is not. For this reason our tests sometimes fail. This was partially (for one test) fixed previously: https://github.com/scylladb/scylla-rust-driver/commit/61ad8781f1a01c9ec5ff087296229c2245c429b3 This commit fixes rest of the tests in session_test that could be affected. It also changes timeout parameters. Previous fixed used 50 attempts with 200ms wait, I changed it to 200 attempts with 50ms wait in order to not slow down Scylla tests unnecessarily. --- scylla/src/transport/session_test.rs | 8 ++++++-- scylla/src/utils/test_utils.rs | 4 ++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/scylla/src/transport/session_test.rs b/scylla/src/transport/session_test.rs index 1cd9534ea0..7fbd31aa9c 100644 --- a/scylla/src/transport/session_test.rs +++ b/scylla/src/transport/session_test.rs @@ -1054,7 +1054,11 @@ async fn assert_in_tracing_table(session: &Session, tracing_uuid: Uuid) { // Tracing info might not be immediately available // If rows are empty perform 8 retries with a 32ms wait in between - for _ in 0..8 { + // The reason why we enable so long waiting for TracingInfo is... Cassandra. (Yes, again.) + // In Cassandra Java Driver, the wait time for tracing info is 10 seconds, so here we do the same. + // However, as Scylla usually gets TracingInfo ready really fast (our default interval is hence 3ms), + // we stick to a not-so-much-terribly-long interval here. + for _ in 0..200 { let rows_num = session .query(traces_query.clone(), (tracing_uuid,)) .await @@ -1068,7 +1072,7 @@ async fn assert_in_tracing_table(session: &Session, tracing_uuid: Uuid) { } // Otherwise retry - tokio::time::sleep(std::time::Duration::from_millis(32)).await; + tokio::time::sleep(std::time::Duration::from_millis(50)).await; } // If all retries failed panic with an error diff --git a/scylla/src/utils/test_utils.rs b/scylla/src/utils/test_utils.rs index b46a0715c1..b0696c7bfb 100644 --- a/scylla/src/utils/test_utils.rs +++ b/scylla/src/utils/test_utils.rs @@ -89,8 +89,8 @@ pub fn create_new_session_builder() -> GenericSessionBuilder Date: Tue, 19 Mar 2024 18:05:20 +0100 Subject: [PATCH 2/3] session_builder: Warn C* users about tracing Cassandra sometimes takes a lot of time to update tracing tables. Default values in session builder are suitable for Scylla, so Cassandra users may encounter unexpected timeouts. This commit adds a note suggesting them to increase the values. --- scylla/src/transport/session_builder.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/scylla/src/transport/session_builder.rs b/scylla/src/transport/session_builder.rs index a3f67520b4..e697bb6154 100644 --- a/scylla/src/transport/session_builder.rs +++ b/scylla/src/transport/session_builder.rs @@ -816,6 +816,10 @@ impl GenericSessionBuilder { /// Tracing info might not be available immediately on queried node - that's why /// the driver performs a few attempts with sleeps in between. /// + /// Cassandra users may want to increase this value - the default is good + /// for Scylla, but Cassandra sometimes needs more time for the data to + /// appear in tracing table. + /// /// # Example /// ``` /// # use scylla::{Session, SessionBuilder}; @@ -841,6 +845,10 @@ impl GenericSessionBuilder { /// Tracing info might not be available immediately on queried node - that's why /// the driver performs a few attempts with sleeps in between. /// + /// Cassandra users may want to increase this value - the default is good + /// for Scylla, but Cassandra sometimes needs more time for the data to + /// appear in tracing table. + /// /// # Example /// ``` /// # use scylla::{Session, SessionBuilder}; From c70faecc814a55371b6564ef7e18a7e165f08d5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20Bary=C5=82a?= Date: Tue, 19 Mar 2024 18:08:54 +0100 Subject: [PATCH 3/3] session: increase default tracing fetch attempts Default values are a bit optimistic: fetching will be attempted 5 times, with 3ms sleeps between attempts. I am reluctant to increase sleep time, because it will increase latency in optimistic case. This commit increases default attempts number from 5 to 10, so that fetching tracing is more reliable, bot not overly lengthy. --- scylla/src/transport/session.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scylla/src/transport/session.rs b/scylla/src/transport/session.rs index da35560878..81e44cd78e 100644 --- a/scylla/src/transport/session.rs +++ b/scylla/src/transport/session.rs @@ -329,7 +329,7 @@ impl SessionConfig { #[cfg(feature = "cloud")] cloud_config: None, enable_write_coalescing: true, - tracing_info_fetch_attempts: NonZeroU32::new(5).unwrap(), + tracing_info_fetch_attempts: NonZeroU32::new(10).unwrap(), tracing_info_fetch_interval: Duration::from_millis(3), tracing_info_fetch_consistency: Consistency::One, cluster_metadata_refresh_interval: Duration::from_secs(60),