From fc4c8def57988cceb1848ee8582bedb9621d06ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Uzarski?= Date: Thu, 24 Oct 2024 16:30:47 +0200 Subject: [PATCH 1/2] exec_profiles: store retry policy in Arc instead of Box This way, it's easier to reuse retry policy for multiple execution profiles/statement configs - there is no need for additional allocation of Box. --- .../execution-profiles/maximal-example.md | 2 +- docs/source/retry-policy/default.md | 7 ++++--- .../retry-policy/downgrading-consistency.md | 9 ++++++--- docs/source/retry-policy/fallthrough.md | 9 ++++++--- examples/execution_profile.rs | 4 ++-- scylla/src/transport/execution_profile.rs | 19 +++++++++++-------- scylla/src/transport/session_test.rs | 2 +- scylla/tests/integration/consistency.rs | 4 ++-- .../tests/integration/execution_profiles.rs | 4 ++-- scylla/tests/integration/lwt_optimisation.rs | 2 +- scylla/tests/integration/retries.rs | 4 ++-- 11 files changed, 38 insertions(+), 28 deletions(-) diff --git a/docs/source/execution-profiles/maximal-example.md b/docs/source/execution-profiles/maximal-example.md index 8209b926a1..2e328970d6 100644 --- a/docs/source/execution-profiles/maximal-example.md +++ b/docs/source/execution-profiles/maximal-example.md @@ -18,7 +18,7 @@ let profile = ExecutionProfile::builder() .consistency(Consistency::All) .serial_consistency(Some(SerialConsistency::Serial)) .request_timeout(Some(Duration::from_secs(30))) - .retry_policy(Box::new(FallthroughRetryPolicy::new())) + .retry_policy(Arc::new(FallthroughRetryPolicy::new())) .load_balancing_policy(Arc::new(DefaultPolicy::default())) .speculative_execution_policy( Some( diff --git a/docs/source/retry-policy/default.md b/docs/source/retry-policy/default.md index 9e5a3697e5..3b57562c29 100644 --- a/docs/source/retry-policy/default.md +++ b/docs/source/retry-policy/default.md @@ -9,13 +9,14 @@ To use in `Session`: # extern crate scylla; # use scylla::Session; # use std::error::Error; +# use std::sync::Arc; # async fn check_only_compiles() -> Result<(), Box> { use scylla::{Session, SessionBuilder}; use scylla::transport::ExecutionProfile; use scylla::transport::retry_policy::DefaultRetryPolicy; let handle = ExecutionProfile::builder() - .retry_policy(Box::new(DefaultRetryPolicy::new())) + .retry_policy(Arc::new(DefaultRetryPolicy::new())) .build() .into_handle(); @@ -45,7 +46,7 @@ my_query.set_retry_policy(Some(Arc::new(DefaultRetryPolicy::new()))); // You can also set retry policy in an execution profile let handle = ExecutionProfile::builder() - .retry_policy(Box::new(DefaultRetryPolicy::new())) + .retry_policy(Arc::new(DefaultRetryPolicy::new())) .build() .into_handle(); my_query.set_execution_profile_handle(Some(handle)); @@ -76,7 +77,7 @@ prepared.set_retry_policy(Some(Arc::new(DefaultRetryPolicy::new()))); // You can also set retry policy in an execution profile let handle = ExecutionProfile::builder() - .retry_policy(Box::new(DefaultRetryPolicy::new())) + .retry_policy(Arc::new(DefaultRetryPolicy::new())) .build() .into_handle(); prepared.set_execution_profile_handle(Some(handle)); diff --git a/docs/source/retry-policy/downgrading-consistency.md b/docs/source/retry-policy/downgrading-consistency.md index 0fea0b6a02..2e335f6e4e 100644 --- a/docs/source/retry-policy/downgrading-consistency.md +++ b/docs/source/retry-policy/downgrading-consistency.md @@ -50,13 +50,14 @@ To use in `Session`: # extern crate scylla; # use scylla::Session; # use std::error::Error; +# use std::sync::Arc; # async fn check_only_compiles() -> Result<(), Box> { use scylla::{Session, SessionBuilder}; use scylla::transport::ExecutionProfile; use scylla::transport::downgrading_consistency_retry_policy::DowngradingConsistencyRetryPolicy; let handle = ExecutionProfile::builder() - .retry_policy(Box::new(DowngradingConsistencyRetryPolicy::new())) + .retry_policy(Arc::new(DowngradingConsistencyRetryPolicy::new())) .build() .into_handle(); @@ -74,13 +75,14 @@ To use in a [simple query](../queries/simple.md): # extern crate scylla; # use scylla::Session; # use std::error::Error; +# use std::sync::Arc; # async fn check_only_compiles(session: &Session) -> Result<(), Box> { use scylla::query::Query; use scylla::transport::ExecutionProfile; use scylla::transport::downgrading_consistency_retry_policy::DowngradingConsistencyRetryPolicy; let handle = ExecutionProfile::builder() - .retry_policy(Box::new(DowngradingConsistencyRetryPolicy::new())) + .retry_policy(Arc::new(DowngradingConsistencyRetryPolicy::new())) .build() .into_handle(); @@ -100,13 +102,14 @@ To use in a [prepared query](../queries/prepared.md): # extern crate scylla; # use scylla::Session; # use std::error::Error; +# use std::sync::Arc; # async fn check_only_compiles(session: &Session) -> Result<(), Box> { use scylla::prepared_statement::PreparedStatement; use scylla::transport::ExecutionProfile; use scylla::transport::downgrading_consistency_retry_policy::DowngradingConsistencyRetryPolicy; let handle = ExecutionProfile::builder() - .retry_policy(Box::new(DowngradingConsistencyRetryPolicy::new())) + .retry_policy(Arc::new(DowngradingConsistencyRetryPolicy::new())) .build() .into_handle(); diff --git a/docs/source/retry-policy/fallthrough.md b/docs/source/retry-policy/fallthrough.md index 089bb1eb41..a2056c52ef 100644 --- a/docs/source/retry-policy/fallthrough.md +++ b/docs/source/retry-policy/fallthrough.md @@ -8,13 +8,14 @@ To use in `Session`: # extern crate scylla; # use scylla::Session; # use std::error::Error; +# use std::sync::Arc; # async fn check_only_compiles() -> Result<(), Box> { use scylla::{Session, SessionBuilder}; use scylla::transport::ExecutionProfile; use scylla::transport::retry_policy::FallthroughRetryPolicy; let handle = ExecutionProfile::builder() - .retry_policy(Box::new(FallthroughRetryPolicy::new())) + .retry_policy(Arc::new(FallthroughRetryPolicy::new())) .build() .into_handle(); @@ -32,13 +33,14 @@ To use in a [simple query](../queries/simple.md): # extern crate scylla; # use scylla::Session; # use std::error::Error; +# use std::sync::Arc; # async fn check_only_compiles(session: &Session) -> Result<(), Box> { use scylla::query::Query; use scylla::transport::ExecutionProfile; use scylla::transport::retry_policy::FallthroughRetryPolicy; let handle = ExecutionProfile::builder() - .retry_policy(Box::new(FallthroughRetryPolicy::new())) + .retry_policy(Arc::new(FallthroughRetryPolicy::new())) .build() .into_handle(); @@ -58,13 +60,14 @@ To use in a [prepared query](../queries/prepared.md): # extern crate scylla; # use scylla::Session; # use std::error::Error; +# use std::sync::Arc; # async fn check_only_compiles(session: &Session) -> Result<(), Box> { use scylla::prepared_statement::PreparedStatement; use scylla::transport::ExecutionProfile; use scylla::transport::retry_policy::FallthroughRetryPolicy; let handle = ExecutionProfile::builder() - .retry_policy(Box::new(FallthroughRetryPolicy::new())) + .retry_policy(Arc::new(FallthroughRetryPolicy::new())) .build() .into_handle(); diff --git a/examples/execution_profile.rs b/examples/execution_profile.rs index b912c2780c..3562966ac5 100644 --- a/examples/execution_profile.rs +++ b/examples/execution_profile.rs @@ -22,7 +22,7 @@ async fn main() -> Result<()> { .serial_consistency(Some(SerialConsistency::Serial)) .request_timeout(Some(Duration::from_secs(42))) .load_balancing_policy(Arc::new(load_balancing::DefaultPolicy::default())) - .retry_policy(Box::new(FallthroughRetryPolicy::new())) + .retry_policy(Arc::new(FallthroughRetryPolicy::new())) .speculative_execution_policy(Some(Arc::new(PercentileSpeculativeExecutionPolicy { max_retry_count: 2, percentile: 42.0, @@ -34,7 +34,7 @@ async fn main() -> Result<()> { .serial_consistency(None) .request_timeout(Some(Duration::from_secs(3))) .load_balancing_policy(Arc::new(load_balancing::DefaultPolicy::default())) - .retry_policy(Box::new(DefaultRetryPolicy::new())) + .retry_policy(Arc::new(DefaultRetryPolicy::new())) .speculative_execution_policy(None) .build(); diff --git a/scylla/src/transport/execution_profile.rs b/scylla/src/transport/execution_profile.rs index 7b7a14faaf..cb09532a79 100644 --- a/scylla/src/transport/execution_profile.rs +++ b/scylla/src/transport/execution_profile.rs @@ -191,8 +191,8 @@ pub(crate) mod defaults { pub(crate) fn load_balancing_policy() -> Arc { Arc::new(load_balancing::DefaultPolicy::default()) } - pub(crate) fn retry_policy() -> Box { - Box::new(DefaultRetryPolicy::new()) + pub(crate) fn retry_policy() -> Arc { + Arc::new(DefaultRetryPolicy::new()) } pub(crate) fn speculative_execution_policy() -> Option> { None @@ -218,10 +218,11 @@ pub(crate) mod defaults { /// ``` /// # use scylla::transport::{ExecutionProfile, retry_policy::FallthroughRetryPolicy}; /// # use scylla::statement::Consistency; +/// # use std::sync::Arc; /// # fn example() -> Result<(), Box> { /// let profile: ExecutionProfile = ExecutionProfile::builder() /// .consistency(Consistency::Three) // as this is the number we shall count to -/// .retry_policy(Box::new(FallthroughRetryPolicy::new())) +/// .retry_policy(Arc::new(FallthroughRetryPolicy::new())) /// .build(); /// # Ok(()) /// # } @@ -232,7 +233,7 @@ pub struct ExecutionProfileBuilder { consistency: Option, serial_consistency: Option>, load_balancing_policy: Option>, - retry_policy: Option>, + retry_policy: Option>, speculative_execution_policy: Option>>, } @@ -302,14 +303,15 @@ impl ExecutionProfileBuilder { /// ``` /// use scylla::transport::retry_policy::DefaultRetryPolicy; /// # use scylla::transport::ExecutionProfile; + /// # use std::sync::Arc; /// # fn example() -> Result<(), Box> { /// let profile: ExecutionProfile = ExecutionProfile::builder() - /// .retry_policy(Box::new(DefaultRetryPolicy::new())) + /// .retry_policy(Arc::new(DefaultRetryPolicy::new())) /// .build(); /// # Ok(()) /// # } /// ``` - pub fn retry_policy(mut self, retry_policy: Box) -> Self { + pub fn retry_policy(mut self, retry_policy: Arc) -> Self { self.retry_policy = Some(retry_policy); self } @@ -352,9 +354,10 @@ impl ExecutionProfileBuilder { /// ``` /// use scylla::transport::retry_policy::DefaultRetryPolicy; /// # use scylla::transport::ExecutionProfile; + /// # use std::sync::Arc; /// # fn example() -> Result<(), Box> { /// let profile: ExecutionProfile = ExecutionProfile::builder() - /// .retry_policy(Box::new(DefaultRetryPolicy::new())) + /// .retry_policy(Arc::new(DefaultRetryPolicy::new())) /// .build(); /// # Ok(()) /// # } @@ -402,7 +405,7 @@ pub(crate) struct ExecutionProfileInner { pub(crate) serial_consistency: Option, pub(crate) load_balancing_policy: Arc, - pub(crate) retry_policy: Box, + pub(crate) retry_policy: Arc, pub(crate) speculative_execution_policy: Option>, } diff --git a/scylla/src/transport/session_test.rs b/scylla/src/transport/session_test.rs index ad2c21960d..41382d4257 100644 --- a/scylla/src/transport/session_test.rs +++ b/scylla/src/transport/session_test.rs @@ -2724,7 +2724,7 @@ async fn test_iter_works_when_retry_policy_returns_ignore_write_error() { let handle = ExecutionProfile::builder() .consistency(Consistency::All) - .retry_policy(Box::new(MyRetryPolicy(retried_flag.clone()))) + .retry_policy(Arc::new(MyRetryPolicy(retried_flag.clone()))) .build() .into_handle(); diff --git a/scylla/tests/integration/consistency.rs b/scylla/tests/integration/consistency.rs index 6402a9ba9d..f12f2d8677 100644 --- a/scylla/tests/integration/consistency.rs +++ b/scylla/tests/integration/consistency.rs @@ -288,7 +288,7 @@ async fn consistency_is_correctly_set_in_cql_requests() { } let fallthrough_exec_profile_builder = - ExecutionProfile::builder().retry_policy(Box::new(FallthroughRetryPolicy)); + ExecutionProfile::builder().retry_policy(Arc::new(FallthroughRetryPolicy)); let translation_map = Arc::new(translation_map); @@ -421,7 +421,7 @@ async fn consistency_is_correctly_set_in_routing_info() { }; let exec_profile_builder = ExecutionProfile::builder() - .retry_policy(Box::new(FallthroughRetryPolicy)) + .retry_policy(Arc::new(FallthroughRetryPolicy)) .load_balancing_policy(Arc::new(reporting_load_balancer)); // DB preparation phase diff --git a/scylla/tests/integration/execution_profiles.rs b/scylla/tests/integration/execution_profiles.rs index 948a1a1502..6e723c9efb 100644 --- a/scylla/tests/integration/execution_profiles.rs +++ b/scylla/tests/integration/execution_profiles.rs @@ -145,7 +145,7 @@ async fn test_execution_profiles() { let profile1 = ExecutionProfile::builder() .load_balancing_policy(policy1.clone()) - .retry_policy(Box::new(policy1.deref().clone())) + .retry_policy(Arc::new(policy1.deref().clone())) .consistency(Consistency::One) .serial_consistency(None) .speculative_execution_policy(None) @@ -153,7 +153,7 @@ async fn test_execution_profiles() { let profile2 = ExecutionProfile::builder() .load_balancing_policy(policy2.clone()) - .retry_policy(Box::new(policy2.deref().clone())) + .retry_policy(Arc::new(policy2.deref().clone())) .consistency(Consistency::Two) .serial_consistency(Some(SerialConsistency::LocalSerial)) .speculative_execution_policy(Some(policy2)) diff --git a/scylla/tests/integration/lwt_optimisation.rs b/scylla/tests/integration/lwt_optimisation.rs index 689a352b39..ca56cff930 100644 --- a/scylla/tests/integration/lwt_optimisation.rs +++ b/scylla/tests/integration/lwt_optimisation.rs @@ -47,7 +47,7 @@ async fn if_lwt_optimisation_mark_offered_then_negotiatied_and_lwt_routed_optima }); let handle = ExecutionProfile::builder() - .retry_policy(Box::new(FallthroughRetryPolicy)) + .retry_policy(Arc::new(FallthroughRetryPolicy)) .build() .into_handle(); diff --git a/scylla/tests/integration/retries.rs b/scylla/tests/integration/retries.rs index 8aca5d197f..43cbf58074 100644 --- a/scylla/tests/integration/retries.rs +++ b/scylla/tests/integration/retries.rs @@ -26,7 +26,7 @@ async fn speculative_execution_is_fired() { let simple_speculative_no_retry_profile = ExecutionProfile::builder().speculative_execution_policy(Some(Arc::new(SimpleSpeculativeExecutionPolicy { max_retry_count: 2, retry_interval: Duration::from_millis(10), - }))).retry_policy(Box::new(FallthroughRetryPolicy)).build(); + }))).retry_policy(Arc::new(FallthroughRetryPolicy)).build(); let session: Session = SessionBuilder::new() .known_node(proxy_uris[0].as_str()) .default_execution_profile_handle(simple_speculative_no_retry_profile.into_handle()) @@ -180,7 +180,7 @@ async fn speculative_execution_panic_regression_test() { }; let profile = ExecutionProfile::builder() .speculative_execution_policy(Some(Arc::new(se))) - .retry_policy(Box::new(FallthroughRetryPolicy)) + .retry_policy(Arc::new(FallthroughRetryPolicy)) .build(); // DB preparation phase let session: Session = SessionBuilder::new() From c958c9bd8b99527381f926729b8c1362c887dba7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Uzarski?= Date: Thu, 24 Oct 2024 16:34:39 +0200 Subject: [PATCH 2/2] retry_policy: remove clone_boxed() method from RetryPolicy trait Again, from now on we will prefer storing dyn RetryPolicy behind an Arc. There is no need for `clone_boxed` method anymore. Removed `impl Clone for Box` --- .../downgrading_consistency_retry_policy.rs | 4 ---- scylla/src/transport/retry_policy.rs | 17 ----------------- scylla/src/transport/session_test.rs | 3 --- scylla/tests/integration/execution_profiles.rs | 4 ---- 4 files changed, 28 deletions(-) diff --git a/scylla/src/transport/downgrading_consistency_retry_policy.rs b/scylla/src/transport/downgrading_consistency_retry_policy.rs index 51fcdfd3ea..e52a44cbf1 100644 --- a/scylla/src/transport/downgrading_consistency_retry_policy.rs +++ b/scylla/src/transport/downgrading_consistency_retry_policy.rs @@ -30,10 +30,6 @@ impl RetryPolicy for DowngradingConsistencyRetryPolicy { fn new_session(&self) -> Box { Box::new(DowngradingConsistencyRetrySession::new()) } - - fn clone_boxed(&self) -> Box { - Box::new(DowngradingConsistencyRetryPolicy) - } } pub struct DowngradingConsistencyRetrySession { diff --git a/scylla/src/transport/retry_policy.rs b/scylla/src/transport/retry_policy.rs index 44baa44248..54f87380eb 100644 --- a/scylla/src/transport/retry_policy.rs +++ b/scylla/src/transport/retry_policy.rs @@ -29,15 +29,6 @@ pub enum RetryDecision { pub trait RetryPolicy: std::fmt::Debug + Send + Sync { /// Called for each new query, starts a session of deciding about retries fn new_session(&self) -> Box; - - /// Used to clone this RetryPolicy - fn clone_boxed(&self) -> Box; -} - -impl Clone for Box { - fn clone(&self) -> Box { - self.clone_boxed() - } } /// Used throughout a single query to decide when to retry it @@ -71,10 +62,6 @@ impl RetryPolicy for FallthroughRetryPolicy { fn new_session(&self) -> Box { Box::new(FallthroughRetrySession) } - - fn clone_boxed(&self) -> Box { - Box::new(FallthroughRetryPolicy) - } } impl RetrySession for FallthroughRetrySession { @@ -106,10 +93,6 @@ impl RetryPolicy for DefaultRetryPolicy { fn new_session(&self) -> Box { Box::new(DefaultRetrySession::new()) } - - fn clone_boxed(&self) -> Box { - Box::new(DefaultRetryPolicy) - } } pub struct DefaultRetrySession { diff --git a/scylla/src/transport/session_test.rs b/scylla/src/transport/session_test.rs index 41382d4257..bfaed4d515 100644 --- a/scylla/src/transport/session_test.rs +++ b/scylla/src/transport/session_test.rs @@ -2708,9 +2708,6 @@ async fn test_iter_works_when_retry_policy_returns_ignore_write_error() { fn new_session(&self) -> Box { Box::new(MyRetrySession(self.0.clone())) } - fn clone_boxed(&self) -> Box { - Box::new(MyRetryPolicy(self.0.clone())) - } } struct MyRetrySession(Arc); diff --git a/scylla/tests/integration/execution_profiles.rs b/scylla/tests/integration/execution_profiles.rs index 6e723c9efb..0a49bae785 100644 --- a/scylla/tests/integration/execution_profiles.rs +++ b/scylla/tests/integration/execution_profiles.rs @@ -95,10 +95,6 @@ impl RetryPolicy for BoundToPredefinedNodePolicy { self.report_node(Report::RetryPolicy); Box::new(self.clone()) } - - fn clone_boxed(&self) -> Box { - Box::new(self.clone()) - } } impl RetrySession for BoundToPredefinedNodePolicy {