From ec78020031808988c437effa5bbd4c494f156d18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Przytu=C5=82a?= Date: Wed, 11 Jun 2025 23:35:42 +0200 Subject: [PATCH 01/13] set_lb_dc_aware: warn on null/empty local DC name cass_{cluster,exec_profile}_set_load_balance_dc_aware(_n) now warn on null or empty local DC name provided. --- scylla-rust-wrapper/src/cluster.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scylla-rust-wrapper/src/cluster.rs b/scylla-rust-wrapper/src/cluster.rs index 45a6b6fd..96400c6f 100644 --- a/scylla-rust-wrapper/src/cluster.rs +++ b/scylla-rust-wrapper/src/cluster.rs @@ -827,6 +827,9 @@ pub(crate) unsafe fn set_load_balance_dc_aware_n( allow_remote_dcs_for_local_cl: cass_bool_t, ) -> CassError { if local_dc_raw.is_null() || local_dc_length == 0 { + tracing::error!( + "Provided null or empty local DC name to cass_*_set_load_balance_dc_aware(_n)!" + ); return CassError::CASS_ERROR_LIB_BAD_PARAMS; } From 77072983b7b8b66733e3f1517bd9c855efe34505 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Przytu=C5=82a?= Date: Thu, 12 Jun 2025 11:48:04 +0200 Subject: [PATCH 02/13] set_lb_dc_aware: `unwrap()` -> pattern matching `set_load_balance_dc_aware_n` unnecessarily `unwrap()`ped the local DC name instead of pattern matching. This commit fixes that. --- scylla-rust-wrapper/src/cluster.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/scylla-rust-wrapper/src/cluster.rs b/scylla-rust-wrapper/src/cluster.rs index 96400c6f..7b0578aa 100644 --- a/scylla-rust-wrapper/src/cluster.rs +++ b/scylla-rust-wrapper/src/cluster.rs @@ -826,11 +826,16 @@ pub(crate) unsafe fn set_load_balance_dc_aware_n( used_hosts_per_remote_dc: c_uint, allow_remote_dcs_for_local_cl: cass_bool_t, ) -> CassError { - if local_dc_raw.is_null() || local_dc_length == 0 { + let Some(local_dc) = (unsafe { ptr_to_cstr_n(local_dc_raw, local_dc_length) }) else { tracing::error!( - "Provided null or empty local DC name to cass_*_set_load_balance_dc_aware(_n)!" + "Provided null or non-UTF-8 local DC name to cass_*_set_load_balance_dc_aware(_n)!" ); return CassError::CASS_ERROR_LIB_BAD_PARAMS; + }; + + if local_dc_length == 0 { + tracing::error!("Provided empty local DC name to cass_*_set_load_balance_dc_aware(_n)!"); + return CassError::CASS_ERROR_LIB_BAD_PARAMS; } if used_hosts_per_remote_dc != 0 || allow_remote_dcs_for_local_cl != 0 { @@ -838,11 +843,9 @@ pub(crate) unsafe fn set_load_balance_dc_aware_n( return CassError::CASS_ERROR_LIB_BAD_PARAMS; } - let local_dc = unsafe { ptr_to_cstr_n(local_dc_raw, local_dc_length) } - .unwrap() - .to_string(); - - load_balancing_config.load_balancing_kind = Some(LoadBalancingKind::DcAware { local_dc }); + load_balancing_config.load_balancing_kind = Some(LoadBalancingKind::DcAware { + local_dc: local_dc.to_owned(), + }); CassError::CASS_OK } From 6a2002dffa020404a85838e7b9e0d3ad404e1792 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Przytu=C5=82a?= Date: Wed, 11 Jun 2025 23:39:54 +0200 Subject: [PATCH 03/13] set_lb_dc_aware: warn on deprecated params used cass_{cluster,exec_profile}_set_load_balance_dc_aware(_n) now warn if the deprecated parameters: - `used_hosts_per_remote_dc`, or - `allow_remote_dcs_for_local_cl` are used (set to non-zero value). This commit does not change the behavior of the driver, it just informs the user that these parameters are no longer supported and should be set to 0. NOTE: next few commits implement (partial) support for these parameters and thus change the messages introduces here. --- scylla-rust-wrapper/src/cluster.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/scylla-rust-wrapper/src/cluster.rs b/scylla-rust-wrapper/src/cluster.rs index 7b0578aa..bc5eb172 100644 --- a/scylla-rust-wrapper/src/cluster.rs +++ b/scylla-rust-wrapper/src/cluster.rs @@ -838,8 +838,19 @@ pub(crate) unsafe fn set_load_balance_dc_aware_n( return CassError::CASS_ERROR_LIB_BAD_PARAMS; } - if used_hosts_per_remote_dc != 0 || allow_remote_dcs_for_local_cl != 0 { - // TODO: Add warning that the parameters are deprecated and not supported in the driver. + if used_hosts_per_remote_dc != 0 { + tracing::error!( + "cass_*_set_load_balance_dc_aware(_n): `used_hosts_per_remote_dc` parameter is no longer \ + supported in the driver. Set it to 0 to avoid this error." + ); + return CassError::CASS_ERROR_LIB_BAD_PARAMS; + } + + if allow_remote_dcs_for_local_cl != 0 { + tracing::error!( + "cass_*_set_load_balance_dc_aware(_n): `allow_remote_dcs_for_local_cl` parameter is no longer \ + supported in the driver. Set it to 0 to avoid this error." + ); return CassError::CASS_ERROR_LIB_BAD_PARAMS; } From 95d8f993c1f663e227fecc67e15a3ac8c1ffd817 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Przytu=C5=82a?= Date: Thu, 12 Jun 2025 13:01:16 +0200 Subject: [PATCH 04/13] lbp: `used_hosts_per_remote_dc=0` disables DC failover The parameter `used_hosts_per_remote_dc` of the DC-aware LBP has been long deprecated. However, it's been still supported in the cpp-driver. Due to Rust driver API's, it'd be very hard to support it correctly in the case of `>0`, because of reasons explained in . Therefore, the partial support approach is taken: - case `=0` -> employ `HostFilter`, disable DC failover and we're done. Our semantics here are in line with the CPP Driver. - case `>0` -> Treat this as `+inf` case - don't limit connections nor requests to remote nodes. Emit a warning that the semantics are different than in CPP Driver. This commit only configures the LBP to set `permit_dc_failover` to false when `used_hosts_per_remote_dc` is `0`, and to true when it's greater than `0`. The next commit will implement the connection management based on this parameter: If a DC is remote to **all** LBPs **and** those LBPs disallow DC failover, then the driver will not connect to any hosts in that DC. This will finish proper support for the `used_hosts_per_remote_dc=0` case. --- scylla-rust-wrapper/src/cluster.rs | 50 ++++++++++++++----------- scylla-rust-wrapper/src/exec_profile.rs | 21 ++++------- 2 files changed, 36 insertions(+), 35 deletions(-) diff --git a/scylla-rust-wrapper/src/cluster.rs b/scylla-rust-wrapper/src/cluster.rs index bc5eb172..5ee37f9b 100644 --- a/scylla-rust-wrapper/src/cluster.rs +++ b/scylla-rust-wrapper/src/cluster.rs @@ -838,13 +838,19 @@ pub(crate) unsafe fn set_load_balance_dc_aware_n( return CassError::CASS_ERROR_LIB_BAD_PARAMS; } - if used_hosts_per_remote_dc != 0 { - tracing::error!( - "cass_*_set_load_balance_dc_aware(_n): `used_hosts_per_remote_dc` parameter is no longer \ - supported in the driver. Set it to 0 to avoid this error." + let permit_dc_failover = if used_hosts_per_remote_dc > 0 { + // TODO: update cassandra.h documentation to reflect this behaviour. + tracing::warn!( + "cass_*_set_load_balance_dc_aware(_n): `used_hosts_per_remote_dc` parameter is only partially \ + supported in the driver: `0` is supported correctly, and any value `>0` has the semantics of \"+inf\", \ + which means no limit on the number of hosts per remote DC. This is different from the original cpp-driver! \ + To clarify, you can understand this parameter as \"permit_dc_failover\", with `0` being `false` and `>0` \ + being `true`." ); - return CassError::CASS_ERROR_LIB_BAD_PARAMS; - } + true + } else { + false + }; if allow_remote_dcs_for_local_cl != 0 { tracing::error!( @@ -856,6 +862,7 @@ pub(crate) unsafe fn set_load_balance_dc_aware_n( load_balancing_config.load_balancing_kind = Some(LoadBalancingKind::DcAware { local_dc: local_dc.to_owned(), + permit_dc_failover, }); CassError::CASS_OK @@ -1774,7 +1781,7 @@ mod tests { cass_cluster_set_load_balance_dc_aware( cluster_raw.borrow_mut(), c"eu".as_ptr(), - 0, + 0, // forbid DC failover 0 ), CassError::CASS_OK @@ -1794,8 +1801,12 @@ mod tests { let cluster = BoxFFI::as_ref(cluster_raw.borrow()).unwrap(); let load_balancing_kind = &cluster.load_balancing_config.load_balancing_kind; match load_balancing_kind { - Some(LoadBalancingKind::DcAware { local_dc }) => { - assert_eq!(local_dc, "eu") + Some(LoadBalancingKind::DcAware { + local_dc, + permit_dc_failover, + }) => { + assert_eq!(local_dc, "eu"); + assert!(!permit_dc_failover); } _ => panic!("Expected preferred dc"), } @@ -1831,7 +1842,7 @@ mod tests { cass_cluster_set_load_balance_dc_aware( cluster_raw.borrow_mut(), c"eu".as_ptr(), - 0, + 42, // allow DC failover 0 ), CassError::CASS_OK @@ -1841,24 +1852,19 @@ mod tests { let node_location_preference = &cluster.load_balancing_config.load_balancing_kind; match node_location_preference { - Some(LoadBalancingKind::DcAware { local_dc }) => { - assert_eq!(local_dc, "eu") + Some(LoadBalancingKind::DcAware { + local_dc, + permit_dc_failover, + }) => { + assert_eq!(local_dc, "eu"); + assert!(permit_dc_failover); } _ => panic!("Expected preferred dc"), } } /* Test invalid configurations */ { - // Nonzero deprecated parameters - assert_cass_error_eq!( - cass_cluster_set_load_balance_dc_aware( - cluster_raw.borrow_mut(), - c"eu".as_ptr(), - 1, - 0 - ), - CassError::CASS_ERROR_LIB_BAD_PARAMS - ); + // Nonzero (deprecated and unsupported) parameter assert_cass_error_eq!( cass_cluster_set_load_balance_dc_aware( cluster_raw.borrow_mut(), diff --git a/scylla-rust-wrapper/src/exec_profile.rs b/scylla-rust-wrapper/src/exec_profile.rs index 9c481242..5363c8e3 100644 --- a/scylla-rust-wrapper/src/exec_profile.rs +++ b/scylla-rust-wrapper/src/exec_profile.rs @@ -933,7 +933,7 @@ mod tests { cass_execution_profile_set_load_balance_dc_aware( profile_raw.borrow_mut(), c"eu".as_ptr(), - 0, + 0, // forbid DC failover 0 ), CassError::CASS_OK @@ -953,8 +953,12 @@ mod tests { let profile = BoxFFI::as_ref(profile_raw.borrow()).unwrap(); let load_balancing_kind = &profile.load_balancing_config.load_balancing_kind; match load_balancing_kind { - Some(LoadBalancingKind::DcAware { local_dc }) => { - assert_eq!(local_dc, "eu") + Some(LoadBalancingKind::DcAware { + local_dc, + permit_dc_failover, + }) => { + assert_eq!(local_dc, "eu"); + assert!(!permit_dc_failover); } _ => panic!("Expected preferred dc"), } @@ -963,16 +967,7 @@ mod tests { } /* Test invalid configurations */ { - // Nonzero deprecated parameters - assert_cass_error_eq!( - cass_execution_profile_set_load_balance_dc_aware( - profile_raw.borrow_mut(), - c"eu".as_ptr(), - 1, - 0 - ), - CassError::CASS_ERROR_LIB_BAD_PARAMS - ); + // Nonzero (deprecated and unsupported) parameter assert_cass_error_eq!( cass_execution_profile_set_load_balance_dc_aware( profile_raw.borrow_mut(), From a745e334296b42e7c6316f112a7123757d15b11c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Przytu=C5=82a?= Date: Thu, 12 Jun 2025 13:06:38 +0200 Subject: [PATCH 05/13] lbp: `used_hosts_per_remote_dc=0` prevents opening connections The parameter `used_hosts_per_remote_dc` of the DC-aware LBP has been long deprecated. However, it's been still supported in the cpp-driver. Due to Rust driver API's, it'd be very hard to support it correctly in the case of `>0`, because of reasons explained in . Therefore, the partial support approach is taken: - case `=0` -> employ `HostFilter`, disable DC failover and we're done. Our semantics here are in line with the CPP Driver. - case `>0` -> Treat this as `+inf` case - don't limit connections nor requests to remote nodes. Emit a warning that the semantics are different than in CPP Driver. This commit introduces specific connection management for the case when `used_hosts_per_remote_dc=0`: if a DC is remote to **all** LBPs **and** those LBPs disallow DC failover, then the driver will not connect to any hosts in that DC. Implementation is done using the existing `Filtering{Config,Info}` machinery. LBPs declare whether they restrict DCs to the specific local DC, or allow all DCs - `DcRestriction` enum serves this purpose. Then, `CassHostFilter` is built from the composition of DC restrictions, by building up an `AllowedDcs` enum: if any LBP allows all DCs, then `AllowedDcs::All` is used, otherwise, we build a whitelist of allowed DCs is constructed (`AllowedDcs::Whitelist(Vec)`). Finally, if `AllowedDcs` is `Whitelist`, then `CassHostFilter` filters out hosts whose DCs are not in that whitelist. This finishes proper support for the `used_hosts_per_remote_dc=0` case. --- scylla-rust-wrapper/src/cluster.rs | 9 +- scylla-rust-wrapper/src/load_balancing.rs | 179 +++++++++++++++++++++- 2 files changed, 181 insertions(+), 7 deletions(-) diff --git a/scylla-rust-wrapper/src/cluster.rs b/scylla-rust-wrapper/src/cluster.rs index 5ee37f9b..f5664e86 100644 --- a/scylla-rust-wrapper/src/cluster.rs +++ b/scylla-rust-wrapper/src/cluster.rs @@ -3,7 +3,9 @@ use crate::cass_error::CassError; use crate::cass_types::CassConsistency; use crate::exec_profile::{CassExecProfile, ExecProfileName, exec_profile_builder_modify}; use crate::future::CassFuture; -use crate::load_balancing::{CassHostFilter, LoadBalancingConfig, LoadBalancingKind}; +use crate::load_balancing::{ + CassHostFilter, DcRestriction, LoadBalancingConfig, LoadBalancingKind, +}; use crate::retry_policy::CassRetryPolicy; use crate::ssl::CassSsl; use crate::timestamp_generator::CassTimestampGen; @@ -864,6 +866,11 @@ pub(crate) unsafe fn set_load_balance_dc_aware_n( local_dc: local_dc.to_owned(), permit_dc_failover, }); + load_balancing_config.filtering.dc_restriction = if permit_dc_failover { + DcRestriction::None + } else { + DcRestriction::Local(local_dc.to_owned()) + }; CassError::CASS_OK } diff --git a/scylla-rust-wrapper/src/load_balancing.rs b/scylla-rust-wrapper/src/load_balancing.rs index debcd397..783644e3 100644 --- a/scylla-rust-wrapper/src/load_balancing.rs +++ b/scylla-rust-wrapper/src/load_balancing.rs @@ -10,12 +10,26 @@ use scylla::policies::load_balancing::{ DefaultPolicyBuilder, FallbackPlan, LatencyAwarenessBuilder, LoadBalancingPolicy, RoutingInfo, }; +/// Whether the LBP allows contacting any hosts in remote datacenters. +/// It strictly corresponds to `allow_hosts_per_remote_dcs` argument of +/// `cass_{cluster,exec_profile}_set_load_balancing_dc_aware()`: +/// - `allow_hosts_per_remote_dcs = 0` <-> `dc_restriction = Local(local_dc)`, +/// - `allow_hosts_per_remote_dcs > 0` <-> `dc_restriction = None`. +#[derive(Clone, Debug)] +pub(crate) enum DcRestriction { + /// No restriction on datacenters. + None, + /// Only the specified local datacenter is allowed. + Local(String), +} + #[derive(Clone, Debug)] pub(crate) struct FilteringConfig { pub(crate) whitelist_hosts: Vec, pub(crate) blacklist_hosts: Vec, pub(crate) whitelist_dc: Vec, pub(crate) blacklist_dc: Vec, + pub(crate) dc_restriction: DcRestriction, } impl FilteringConfig { @@ -27,16 +41,35 @@ impl FilteringConfig { blacklist_hosts: (!self.blacklist_hosts.is_empty()).then_some(self.blacklist_hosts), whitelist_dc: (!self.whitelist_dc.is_empty()).then_some(self.whitelist_dc), blacklist_dc: (!self.blacklist_dc.is_empty()).then_some(self.blacklist_dc), + allowed_dcs: match self.dc_restriction { + DcRestriction::None => AllowedDcs::All, + DcRestriction::Local(local_dc) => AllowedDcs::Whitelist(vec![local_dc]), + }, } } } +#[derive(Clone, Debug)] +#[cfg_attr(test, derive(PartialEq, Eq))] +pub(crate) enum AllowedDcs { + /// All datacenters are allowed, including remote ones. + All, + /// Only the specified datacenters are allowed (the local DCs of DC-aware policies). + Whitelist(Vec), +} + #[derive(Debug)] pub(crate) struct FilteringInfo { pub(crate) whitelist_hosts: Option>, pub(crate) blacklist_hosts: Option>, pub(crate) whitelist_dc: Option>, pub(crate) blacklist_dc: Option>, + /// This is different from `whitelist_dc` in its origin: + /// - `whitelist_dc` is a user-provided list of datacenters that are allowed, + /// - `allowed_dcs` is a set built from the load balancing policies, based + /// on their allowance of remote datacenters or lack thereof (in case of + /// the DC-aware policy with `used_hosts_per_remote_dc=0`). + pub(crate) allowed_dcs: AllowedDcs, } impl FilteringInfo { @@ -49,6 +82,16 @@ impl FilteringInfo { // Treat missing dc as empty string. let dc = dc.unwrap_or_default(); + if let AllowedDcs::Whitelist(ref allowed_dcs) = self.allowed_dcs { + // If the host's DC is not in the whitelist of DCs, reject it. + if !allowed_dcs + .iter() + .any(|allowed_dc| allowed_dc.as_str() == dc) + { + return false; + } + } + if self .whitelist_hosts .as_ref() @@ -112,8 +155,13 @@ impl LoadBalancingConfig { } match load_balancing_kind { - LoadBalancingKind::DcAware { local_dc } => { - builder = builder.prefer_datacenter(local_dc).permit_dc_failover(true) + LoadBalancingKind::DcAware { + local_dc, + permit_dc_failover, + } => { + builder = builder + .prefer_datacenter(local_dc) + .permit_dc_failover(permit_dc_failover) } LoadBalancingKind::RackAware { local_dc, @@ -151,6 +199,7 @@ impl Default for LoadBalancingConfig { blacklist_hosts: Vec::new(), whitelist_dc: Vec::new(), blacklist_dc: Vec::new(), + dc_restriction: DcRestriction::None, // Round-robin policy, which is the default, allows contacting remote DCs. }, } } @@ -161,6 +210,7 @@ pub(crate) enum LoadBalancingKind { RoundRobin, DcAware { local_dc: String, + permit_dc_failover: bool, }, RackAware { local_dc: String, @@ -296,9 +346,25 @@ impl CassHostFilter { /// /// Now, if a host is not in the union of whitelists, it is rejected. /// If a host is in the intersection of blacklists, it is rejected. + /// + /// Apart from black- and whitelists, we also compute the allowed datacenters, + /// which are based on the `used_hosts_per_remote_dc` parameter of the + /// DC-aware load balancing policy. We only correctly handle the case + /// where 0 is passed, which means that the policy does not allow contacting + /// remote datacenters. + /// If all policies forbid contacting remote datacenters, we prevent + /// opening connections to hosts in remote datacenters. pub(crate) fn new_from_lbp_configs<'a>( configs: impl Iterator + Clone, ) -> Arc { + Arc::new(Self::new_from_lbp_configs_inner(configs)) + } + + /// This is the inner implementation of `new_from_lbp_configs`, + /// extracted in order to allow testing. + fn new_from_lbp_configs_inner<'a>( + configs: impl Iterator + Clone, + ) -> Self { let whitelist_hosts = nonempty_union( configs .clone() @@ -317,22 +383,51 @@ impl CassHostFilter { .map(|lbp_config| &lbp_config.filtering.whitelist_dc), ); - let blacklist_dc = - nonempty_intersection(configs.map(|lbp_config| &lbp_config.filtering.blacklist_dc)); + let blacklist_dc = nonempty_intersection( + configs + .clone() + .map(|lbp_config| &lbp_config.filtering.blacklist_dc), + ); + + let allowed_dcs = configs + .fold(None, |allowed_dcs, lbp_config| { + match (allowed_dcs, &lbp_config.filtering.dc_restriction) { + (None, DcRestriction::None) => Some(AllowedDcs::All), + (None, DcRestriction::Local(local_dc)) => { + Some(AllowedDcs::Whitelist(vec![local_dc.clone()])) + } + // If this policy allows only a specified DC, add it to the allowed DCs. + (Some(AllowedDcs::Whitelist(mut dcs)), DcRestriction::Local(local_dc)) => { + dcs.push(local_dc.clone()); + Some(AllowedDcs::Whitelist(dcs)) + } + // If some policy allowed all DCs, allow all DCs globally. + (Some(AllowedDcs::All), _) => Some(AllowedDcs::All), + // If this policy allows all DCs, allow all DCs globally. + (_, DcRestriction::None) => Some(AllowedDcs::All), + } + }) + // Note: this should never happen, as at least one (cluster-level) policy should be present. + .unwrap_or(AllowedDcs::All); - Arc::new(Self { + Self { filtering: FilteringInfo { whitelist_hosts, blacklist_hosts, whitelist_dc, blacklist_dc, + allowed_dcs, }, - }) + } } } #[cfg(test)] mod tests { + use crate::load_balancing::LoadBalancingConfig; + + use super::{AllowedDcs, CassHostFilter, DcRestriction, FilteringConfig}; + #[test] fn test_union_and_intersection() { struct TestCase { @@ -402,4 +497,76 @@ mod tests { assert_eq!(intersection, test.expected_intersection); } } + + #[test] + fn test_allowed_dcs() { + struct TestCase { + input: Vec, + merged: AllowedDcs, + } + + let test_cases = &[ + TestCase { + input: vec![DcRestriction::None], + merged: AllowedDcs::All, + }, + TestCase { + input: vec![DcRestriction::Local("dc1".to_owned())], + merged: AllowedDcs::Whitelist(vec!["dc1".to_owned()]), + }, + TestCase { + input: vec![ + DcRestriction::Local("dc1".to_owned()), + DcRestriction::Local("dc2".to_owned()), + ], + merged: AllowedDcs::Whitelist(vec!["dc1".to_owned(), "dc2".to_owned()]), + }, + TestCase { + input: vec![DcRestriction::Local("dc1".to_owned()), DcRestriction::None], + merged: AllowedDcs::All, + }, + TestCase { + input: vec![DcRestriction::None, DcRestriction::Local("dc1".to_owned())], + merged: AllowedDcs::All, + }, + TestCase { + input: vec![ + DcRestriction::Local("dc1".to_owned()), + DcRestriction::None, + DcRestriction::Local("dc1".to_owned()), + ], + merged: AllowedDcs::All, + }, + // Non-mandatory case. Should never happen in practice, as at least one (cluster-level) policy should be present. + TestCase { + input: vec![], + merged: AllowedDcs::All, + }, + ]; + + for TestCase { input, merged } in test_cases { + let filtering_configs = input.iter().map(|dc_restriction| FilteringConfig { + whitelist_hosts: Vec::new(), + blacklist_hosts: Vec::new(), + whitelist_dc: Vec::new(), + blacklist_dc: Vec::new(), + dc_restriction: dc_restriction.clone(), + }); + let load_balancing_configs = filtering_configs + .map(|filtering| LoadBalancingConfig { + token_awareness_enabled: false, + token_aware_shuffling_replicas_enabled: false, + load_balancing_kind: None, + latency_awareness_enabled: false, + latency_awareness_builder: Default::default(), + filtering, + }) + .collect::>(); + + let cass_filter = + CassHostFilter::new_from_lbp_configs_inner(load_balancing_configs.iter()); + + assert_eq!(&cass_filter.filtering.allowed_dcs, merged); + } + } } From 91226004d8229bd8e310e1613fccaa494513250b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Przytu=C5=82a?= Date: Tue, 17 Jun 2025 13:28:18 +0200 Subject: [PATCH 06/13] lbp: introduce `DcLocalConsistencyAllowance` The purpose of `DcLocalConsistencyAllowance` is to determine whether remote DCs are allowed to be contacted when a local consistency level is used. It's going to be used in the next commit, to support `allow_remote_dcs_for_local_cl` parameter of the DC-aware LBP. --- scylla-rust-wrapper/src/load_balancing.rs | 123 ++++++++++++++++++++++ 1 file changed, 123 insertions(+) diff --git a/scylla-rust-wrapper/src/load_balancing.rs b/scylla-rust-wrapper/src/load_balancing.rs index 783644e3..3d9edf33 100644 --- a/scylla-rust-wrapper/src/load_balancing.rs +++ b/scylla-rust-wrapper/src/load_balancing.rs @@ -218,6 +218,51 @@ pub(crate) enum LoadBalancingKind { }, } +/// Determines whether remote DCs are allowed to be contacted when a local consistency +/// level is used. +#[derive(Debug)] +enum DcLocalConsistencyAllowance { + /// The policy allows contacting hosts in all datacenters: the local one and remote ones. + AllowAll, + /// The policy does not allow contacting hosts in remote datacenters + /// if a local consistency level is used. + DisallowRemotes { + /// The local datacenter, which is the only one that is allowed to be contacted + /// when a local consistency level is used. + local_dc: String, + }, +} + +impl DcLocalConsistencyAllowance { + fn is_consistency_local(cl: Consistency) -> bool { + match cl { + Consistency::Any + | Consistency::One + | Consistency::Two + | Consistency::Three + | Consistency::Quorum + | Consistency::All + | Consistency::EachQuorum + | Consistency::Serial => false, + Consistency::LocalQuorum | Consistency::LocalOne | Consistency::LocalSerial => true, + } + } + + fn is_dc_allowed(&self, dc: Option<&str>, cl: Consistency) -> bool { + match self { + DcLocalConsistencyAllowance::AllowAll => true, + DcLocalConsistencyAllowance::DisallowRemotes { local_dc } + if Self::is_consistency_local(cl) => + { + // If the DC is not the one that is allowed - the local one, return false. + dc.is_some_and(|dc| local_dc == dc) + } + // Consistency is not local, so we allow all datacenters. + DcLocalConsistencyAllowance::DisallowRemotes { .. } => true, + } + } +} + #[derive(Debug)] pub(crate) struct FilteringLoadBalancingPolicy { pub(crate) filtering: FilteringInfo, @@ -569,4 +614,82 @@ mod tests { assert_eq!(&cass_filter.filtering.allowed_dcs, merged); } } + + #[test] + fn test_dc_local_cl_allowance() { + use super::DcLocalConsistencyAllowance; + use scylla::statement::Consistency; + + struct TestCase { + allowance: DcLocalConsistencyAllowance, + dc: Option<&'static str>, + cl: Consistency, + expected: bool, + } + + let test_cases = vec![ + TestCase { + allowance: DcLocalConsistencyAllowance::AllowAll, + dc: Some("dc1"), + cl: Consistency::LocalQuorum, + expected: true, + }, + TestCase { + allowance: DcLocalConsistencyAllowance::AllowAll, + dc: Some("dc1"), + cl: Consistency::Quorum, + expected: true, + }, + TestCase { + allowance: DcLocalConsistencyAllowance::DisallowRemotes { + local_dc: "dc1".to_owned(), + }, + dc: Some("dc1"), + cl: Consistency::LocalQuorum, + expected: true, + }, + TestCase { + allowance: DcLocalConsistencyAllowance::DisallowRemotes { + local_dc: "dc1".to_owned(), + }, + dc: Some("dc2"), + cl: Consistency::LocalQuorum, + expected: false, + }, + TestCase { + allowance: DcLocalConsistencyAllowance::DisallowRemotes { + local_dc: "dc1".to_owned(), + }, + dc: None, + cl: Consistency::LocalQuorum, + expected: false, + }, + TestCase { + allowance: DcLocalConsistencyAllowance::DisallowRemotes { + local_dc: "dc1".to_owned(), + }, + dc: Some("dc1"), + cl: Consistency::Quorum, + expected: true, + }, + TestCase { + allowance: DcLocalConsistencyAllowance::DisallowRemotes { + local_dc: "dc1".to_owned(), + }, + dc: Some("dc2"), + cl: Consistency::Quorum, + expected: true, + }, + ]; + + for TestCase { + allowance, + dc, + cl, + expected, + } in test_cases + { + assert_eq!(allowance.is_dc_allowed(dc, cl), expected); + } + } } From f8fb31b387fcccf1aa7103aba28cf7248f67fff5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Przytu=C5=82a?= Date: Tue, 17 Jun 2025 13:32:29 +0200 Subject: [PATCH 07/13] lbp: support `allow_remote_dcs_for_local_cl` Even though the parameter has long been deprecated, we should support it. This commit makes both functions: - `cass_cluster_set_load_balance_dc_aware`, - `cass_execution_profile_set_load_balance_dc_aware` handle the parameter `allow_remote_dcs_for_local_cl` correctly. --- scylla-rust-wrapper/src/cluster.rs | 28 ++++--------- scylla-rust-wrapper/src/exec_profile.rs | 15 +------ scylla-rust-wrapper/src/load_balancing.rs | 49 +++++++++++++++++------ 3 files changed, 47 insertions(+), 45 deletions(-) diff --git a/scylla-rust-wrapper/src/cluster.rs b/scylla-rust-wrapper/src/cluster.rs index f5664e86..0432c6ca 100644 --- a/scylla-rust-wrapper/src/cluster.rs +++ b/scylla-rust-wrapper/src/cluster.rs @@ -854,17 +854,12 @@ pub(crate) unsafe fn set_load_balance_dc_aware_n( false }; - if allow_remote_dcs_for_local_cl != 0 { - tracing::error!( - "cass_*_set_load_balance_dc_aware(_n): `allow_remote_dcs_for_local_cl` parameter is no longer \ - supported in the driver. Set it to 0 to avoid this error." - ); - return CassError::CASS_ERROR_LIB_BAD_PARAMS; - } + let allow_remote_dcs_for_local_cl = allow_remote_dcs_for_local_cl != 0; load_balancing_config.load_balancing_kind = Some(LoadBalancingKind::DcAware { local_dc: local_dc.to_owned(), permit_dc_failover, + allow_remote_dcs_for_local_cl, }); load_balancing_config.filtering.dc_restriction = if permit_dc_failover { DcRestriction::None @@ -1811,9 +1806,11 @@ mod tests { Some(LoadBalancingKind::DcAware { local_dc, permit_dc_failover, + allow_remote_dcs_for_local_cl, }) => { assert_eq!(local_dc, "eu"); assert!(!permit_dc_failover); + assert!(!allow_remote_dcs_for_local_cl); } _ => panic!("Expected preferred dc"), } @@ -1849,8 +1846,8 @@ mod tests { cass_cluster_set_load_balance_dc_aware( cluster_raw.borrow_mut(), c"eu".as_ptr(), - 42, // allow DC failover - 0 + 42, // allow DC failover + cass_true // allow remote DCs for local CL ), CassError::CASS_OK ); @@ -1862,26 +1859,17 @@ mod tests { Some(LoadBalancingKind::DcAware { local_dc, permit_dc_failover, + allow_remote_dcs_for_local_cl, }) => { assert_eq!(local_dc, "eu"); assert!(permit_dc_failover); + assert!(allow_remote_dcs_for_local_cl); } _ => panic!("Expected preferred dc"), } } /* Test invalid configurations */ { - // Nonzero (deprecated and unsupported) parameter - assert_cass_error_eq!( - cass_cluster_set_load_balance_dc_aware( - cluster_raw.borrow_mut(), - c"eu".as_ptr(), - 0, - 1 - ), - CassError::CASS_ERROR_LIB_BAD_PARAMS - ); - // null pointers assert_cass_error_eq!( cass_cluster_set_load_balance_dc_aware( diff --git a/scylla-rust-wrapper/src/exec_profile.rs b/scylla-rust-wrapper/src/exec_profile.rs index 5363c8e3..05c798be 100644 --- a/scylla-rust-wrapper/src/exec_profile.rs +++ b/scylla-rust-wrapper/src/exec_profile.rs @@ -956,28 +956,17 @@ mod tests { Some(LoadBalancingKind::DcAware { local_dc, permit_dc_failover, + allow_remote_dcs_for_local_cl, }) => { assert_eq!(local_dc, "eu"); assert!(!permit_dc_failover); + assert!(!allow_remote_dcs_for_local_cl); } _ => panic!("Expected preferred dc"), } assert!(!profile.load_balancing_config.token_awareness_enabled); assert!(profile.load_balancing_config.latency_awareness_enabled); } - /* Test invalid configurations */ - { - // Nonzero (deprecated and unsupported) parameter - assert_cass_error_eq!( - cass_execution_profile_set_load_balance_dc_aware( - profile_raw.borrow_mut(), - c"eu".as_ptr(), - 0, - 1 - ), - CassError::CASS_ERROR_LIB_BAD_PARAMS - ); - } } cass_execution_profile_free(profile_raw); diff --git a/scylla-rust-wrapper/src/load_balancing.rs b/scylla-rust-wrapper/src/load_balancing.rs index 3d9edf33..20e860d9 100644 --- a/scylla-rust-wrapper/src/load_balancing.rs +++ b/scylla-rust-wrapper/src/load_balancing.rs @@ -9,6 +9,7 @@ use scylla::policies::host_filter::HostFilter; use scylla::policies::load_balancing::{ DefaultPolicyBuilder, FallbackPlan, LatencyAwarenessBuilder, LoadBalancingPolicy, RoutingInfo, }; +use scylla::statement::Consistency; /// Whether the LBP allows contacting any hosts in remote datacenters. /// It strictly corresponds to `allow_hosts_per_remote_dcs` argument of @@ -154,14 +155,28 @@ impl LoadBalancingConfig { builder.enable_shuffling_replicas(self.token_aware_shuffling_replicas_enabled); } - match load_balancing_kind { + // Configure DC-related settings. + // This includes: + // - local DC-awareness, + // - DC failover, + // - remote DCs allowance for local consistency levels. + let dc_local_cl_allowance = match load_balancing_kind { LoadBalancingKind::DcAware { local_dc, permit_dc_failover, + allow_remote_dcs_for_local_cl, } => { + let dc_local_cl_allowance = if allow_remote_dcs_for_local_cl { + DcLocalConsistencyAllowance::AllowAll + } else { + DcLocalConsistencyAllowance::DisallowRemotes { + local_dc: local_dc.clone(), + } + }; builder = builder .prefer_datacenter(local_dc) - .permit_dc_failover(permit_dc_failover) + .permit_dc_failover(permit_dc_failover); + dc_local_cl_allowance } LoadBalancingKind::RackAware { local_dc, @@ -169,10 +184,11 @@ impl LoadBalancingConfig { } => { builder = builder .prefer_datacenter_and_rack(local_dc, local_rack) - .permit_dc_failover(true) + .permit_dc_failover(true); + DcLocalConsistencyAllowance::AllowAll } - LoadBalancingKind::RoundRobin => {} - } + LoadBalancingKind::RoundRobin => DcLocalConsistencyAllowance::AllowAll, + }; if self.latency_awareness_enabled { builder = builder.latency_awareness(self.latency_awareness_builder); @@ -182,6 +198,7 @@ impl LoadBalancingConfig { Arc::new(FilteringLoadBalancingPolicy { filtering: self.filtering.into_filtering_info(), child_policy, + dc_local_cl_allowance, }) } } @@ -211,6 +228,7 @@ pub(crate) enum LoadBalancingKind { DcAware { local_dc: String, permit_dc_failover: bool, + allow_remote_dcs_for_local_cl: bool, }, RackAware { local_dc: String, @@ -265,8 +283,9 @@ impl DcLocalConsistencyAllowance { #[derive(Debug)] pub(crate) struct FilteringLoadBalancingPolicy { - pub(crate) filtering: FilteringInfo, - pub(crate) child_policy: Arc, + filtering: FilteringInfo, + dc_local_cl_allowance: DcLocalConsistencyAllowance, + child_policy: Arc, } impl LoadBalancingPolicy for FilteringLoadBalancingPolicy { @@ -279,9 +298,12 @@ impl LoadBalancingPolicy for FilteringLoadBalancingPolicy { picked.and_then(|target| { let node = target.0; - self.filtering - .is_host_allowed(&node.address.ip(), node.datacenter.as_deref()) - .then_some(target) + let dc = node.datacenter.as_deref(); + (self.filtering.is_host_allowed(&node.address.ip(), dc) + && self + .dc_local_cl_allowance + .is_dc_allowed(dc, request.consistency)) + .then_some(target) }) } @@ -294,8 +316,11 @@ impl LoadBalancingPolicy for FilteringLoadBalancingPolicy { self.child_policy .fallback(request, cluster) .filter(|(node, _shard)| { - self.filtering - .is_host_allowed(&node.address.ip(), node.datacenter.as_deref()) + let dc = node.datacenter.as_deref(); + self.filtering.is_host_allowed(&node.address.ip(), dc) + && self + .dc_local_cl_allowance + .is_dc_allowed(dc, request.consistency) }), ) } From e43914d7e862e02a07ba883603f3bd62d7c71018 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Przytu=C5=82a?= Date: Tue, 17 Jun 2025 10:18:01 +0200 Subject: [PATCH 08/13] lbp: FilteringLoadBalancingPolicy logs decisions Those logs are emitted at trace level, so they will not clutter the output. They are useful for debugging. --- scylla-rust-wrapper/src/load_balancing.rs | 27 ++++++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/scylla-rust-wrapper/src/load_balancing.rs b/scylla-rust-wrapper/src/load_balancing.rs index 20e860d9..4badb872 100644 --- a/scylla-rust-wrapper/src/load_balancing.rs +++ b/scylla-rust-wrapper/src/load_balancing.rs @@ -296,7 +296,9 @@ impl LoadBalancingPolicy for FilteringLoadBalancingPolicy { ) -> Option<(scylla::cluster::NodeRef<'a>, Option)> { let picked = self.child_policy.pick(request, cluster); - picked.and_then(|target| { + tracing::trace!("Child policy pick'd {:?}", picked); + + let our_pick = picked.and_then(|target| { let node = target.0; let dc = node.datacenter.as_deref(); (self.filtering.is_host_allowed(&node.address.ip(), dc) @@ -304,7 +306,9 @@ impl LoadBalancingPolicy for FilteringLoadBalancingPolicy { .dc_local_cl_allowance .is_dc_allowed(dc, request.consistency)) .then_some(target) - }) + }); + tracing::trace!("Filtering policy pick'd {:?}", our_pick); + our_pick } fn fallback<'a>( @@ -317,10 +321,21 @@ impl LoadBalancingPolicy for FilteringLoadBalancingPolicy { .fallback(request, cluster) .filter(|(node, _shard)| { let dc = node.datacenter.as_deref(); - self.filtering.is_host_allowed(&node.address.ip(), dc) - && self - .dc_local_cl_allowance - .is_dc_allowed(dc, request.consistency) + let is_host_allowed = self.filtering.is_host_allowed(&node.address.ip(), dc); + let is_dc_allowed = self + .dc_local_cl_allowance + .is_dc_allowed(dc, request.consistency); + tracing::trace!( + "Filtering policy got {:?} in fallback and decided to {}.", + node, + match (is_host_allowed, is_dc_allowed) { + (false, false) => "DROP it because neither host nor DC are not allowed", + (false, true) => "DROP it because host is not allowed", + (true, false) => "DROP it because DC is not allowed", + (true, true) => "KEEP it because both host and DC are allowed", + } + ); + is_host_allowed && is_dc_allowed }), ) } From 68a338c7847b3cd9afd1cc6b67da22636d656d8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Przytu=C5=82a?= Date: Tue, 17 Jun 2025 11:41:18 +0200 Subject: [PATCH 09/13] Revert "tests: adjust DCExecutionProfileTests constructor" This reverts commit 6b44c2cb5d2ab82e1674d4e83c7fbd2266c263bf. The adjustments are no longer needed, because the offending parameters are now supported. --- tests/src/integration/tests/test_exec_profile.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/src/integration/tests/test_exec_profile.cpp b/tests/src/integration/tests/test_exec_profile.cpp index a0c2d8fa..7b2d3554 100644 --- a/tests/src/integration/tests/test_exec_profile.cpp +++ b/tests/src/integration/tests/test_exec_profile.cpp @@ -193,15 +193,15 @@ class DCExecutionProfileTest : public ExecutionProfileTest { void SetUp() { // Create the execution profiles for the test cases profiles_["dc_aware"] = ExecutionProfile::build() - .with_load_balance_dc_aware("dc1", 0, false) + .with_load_balance_dc_aware("dc1", 1, false) .with_consistency(CASS_CONSISTENCY_LOCAL_ONE); profiles_["blacklist_dc"] = ExecutionProfile::build() .with_blacklist_dc_filtering("dc1") - .with_load_balance_dc_aware("dc1", 0, false) + .with_load_balance_dc_aware("dc1", 1, true) .with_consistency(CASS_CONSISTENCY_LOCAL_ONE); profiles_["whitelist_dc"] = ExecutionProfile::build() .with_whitelist_dc_filtering("dc2") - .with_load_balance_dc_aware("dc1", 0, false) + .with_load_balance_dc_aware("dc1", 1, true) .with_consistency(CASS_CONSISTENCY_LOCAL_ONE); // Call the parent setup function From 84893b63f9c6e150f36f67af4f96025b30742f04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Przytu=C5=82a?= Date: Tue, 17 Jun 2025 18:02:08 +0200 Subject: [PATCH 10/13] CI: don't test with Scylla 5.4.8 The Scylla 5.4.8 was introduced to the CI 9 months ago, in acf9d3d23b794d19b9dd370dff0b07328ed58e95. The rationale was to test on the same version as the cpp-driver CI. There's no use in testing neither cpp-driver nor this driver with such old version anymore. As some tests fail due to ccm errors on that version, we remove it from the CI. --- .github/workflows/build-lint-and-test.yml | 24 ++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/.github/workflows/build-lint-and-test.yml b/.github/workflows/build-lint-and-test.yml index 615ddbc0..500fa5c3 100644 --- a/.github/workflows/build-lint-and-test.yml +++ b/.github/workflows/build-lint-and-test.yml @@ -2,16 +2,16 @@ name: Build on: push: - branches: [ master ] + branches: [master] pull_request: - branches: [ master ] + branches: [master] env: CARGO_TERM_COLOR: always # Should include `INTEGRATION_TEST_BIN` from the `Makefile` # TODO: Remove `build/libscylla-cpp-driver.*` after https://github.com/scylladb/cpp-rust-driver/issues/164 is fixed. INTEGRATION_TEST_BIN: | - build/cassandra-integration-tests + build/cassandra-integration-tests build/libscylla-cpp-driver.* INTEGRATION_TEST_BIN_CACHE_KEY: integration-test-bin-${{ github.sha }} # Goes to `Makefile` to let it pickup cached binary @@ -25,7 +25,7 @@ jobs: steps: - name: Checkout uses: actions/checkout@v4 - + - name: Update apt cache run: sudo apt-get update -y @@ -57,7 +57,13 @@ jobs: strategy: matrix: - scylla-version: [ENTERPRISE-RELEASE, ENTERPRISE-PRIOR-RELEASE, OSS-RELEASE, OSS-PRIOR-RELEASE, 5.4.8] + scylla-version: + [ + ENTERPRISE-RELEASE, + ENTERPRISE-PRIOR-RELEASE, + OSS-RELEASE, + OSS-PRIOR-RELEASE, + ] fail-fast: false steps: @@ -67,7 +73,7 @@ jobs: - name: Setup Python 3 uses: actions/setup-python@v5 with: - python-version: '3.11' + python-version: "3.11" - name: Update apt cache run: sudo apt-get update -y @@ -165,13 +171,13 @@ jobs: uses: actions/setup-java@v4 with: java-version: ${{ matrix.java-version }} - distribution: 'adopt' + distribution: "adopt" - name: Setup Python 3 uses: actions/setup-python@v5 with: - python-version: '3.11' - + python-version: "3.11" + - name: Update apt cache run: sudo apt-get update -y From 3f6927f5ab99313f12d86e42478ba5166fce23ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Przytu=C5=82a?= Date: Thu, 12 Jun 2025 18:40:18 +0200 Subject: [PATCH 11/13] IT: enable DCExecutionProfileTest_DCAware test The test DCExecutionProfileTest.DCAware was disabled because it expected deterministic behaviour of the token-unaware round robin load balancing. As the Rust Driver round-robins nodes with a random shift, the test would fail. I reworked it to check that the node in the other DC is not used, instead of expecting a specific node in the local DC to be used. The test is now enabled in the Makefile. --- Makefile | 2 -- .../integration/tests/test_exec_profile.cpp | 18 +++++++----------- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/Makefile b/Makefile index 49b9c81e..9fe04279 100644 --- a/Makefile +++ b/Makefile @@ -46,7 +46,6 @@ SCYLLA_TEST_FILTER := $(subst ${SPACE},${EMPTY},ClusterTests.*\ :ExecutionProfileTest.Integration_Cassandra_RoundRobin\ :ExecutionProfileTest.Integration_Cassandra_TokenAwareRouting\ :ExecutionProfileTest.Integration_Cassandra_SpeculativeExecutionPolicy\ -:DCExecutionProfileTest.Integration_Cassandra_DCAware\ :ControlConnectionTests.Integration_Cassandra_TopologyChange\ :ControlConnectionTests.Integration_Cassandra_FullOutage\ :ControlConnectionTests.Integration_Cassandra_TerminatedUsingMultipleIoThreadsWithError\ @@ -107,7 +106,6 @@ CASSANDRA_TEST_FILTER := $(subst ${SPACE},${EMPTY},ClusterTests.*\ :ExecutionProfileTest.Integration_Cassandra_RoundRobin\ :ExecutionProfileTest.Integration_Cassandra_TokenAwareRouting\ :ExecutionProfileTest.Integration_Cassandra_SpeculativeExecutionPolicy\ -:DCExecutionProfileTest.Integration_Cassandra_DCAware\ :ControlConnectionTests.Integration_Cassandra_TopologyChange\ :ControlConnectionTests.Integration_Cassandra_FullOutage\ :ControlConnectionTests.Integration_Cassandra_TerminatedUsingMultipleIoThreadsWithError\ diff --git a/tests/src/integration/tests/test_exec_profile.cpp b/tests/src/integration/tests/test_exec_profile.cpp index 7b2d3554..172c4908 100644 --- a/tests/src/integration/tests/test_exec_profile.cpp +++ b/tests/src/integration/tests/test_exec_profile.cpp @@ -715,6 +715,11 @@ CASSANDRA_INTEGRATION_TEST_F(ExecutionProfileTest, SpeculativeExecutionPolicy) { CASSANDRA_INTEGRATION_TEST_F(DCExecutionProfileTest, DCAware) { CHECK_FAILURE; + // Determine the disallowed IP address for the statement DC-aware execution + int const unexpected_node = 3; // The node in DC2. + std::stringstream unexpected_ip_address; + unexpected_ip_address << ccm_->get_ip_prefix() << unexpected_node; + // Execute statements over all the nodes in the cluster twice for (size_t i = 0; i < total_nodes_ * 2; ++i) { // Execute the same query with the cluster default profile @@ -722,28 +727,19 @@ CASSANDRA_INTEGRATION_TEST_F(DCExecutionProfileTest, DCAware) { Result result = session_.execute(insert_); ASSERT_EQ(CASS_OK, result.error_code()); - // Determine the expected IP address for the statement execution - int expected_node = ((i * 2) % number_dc1_nodes_) + 1; - std::stringstream expected_ip_address; - expected_ip_address << ccm_->get_ip_prefix() << expected_node; - // Execute a batched query with assigned profile Batch batch; batch.add(insert_); batch.set_execution_profile("dc_aware"); result = session_.execute(batch); ASSERT_EQ(CASS_OK, result.error_code()); - ASSERT_STREQ(expected_ip_address.str().c_str(), result.host().c_str()); - - // Increment the expected round robin IP address - expected_ip_address.str(""); - expected_ip_address << ccm_->get_ip_prefix() << expected_node + 1; + ASSERT_STRNE(unexpected_ip_address.str().c_str(), result.host().c_str()); // Execute a simple query with assigned profile insert_.set_execution_profile("dc_aware"); result = session_.execute(insert_); ASSERT_EQ(CASS_OK, result.error_code()); - ASSERT_STREQ(expected_ip_address.str().c_str(), result.host().c_str()); + ASSERT_STRNE(unexpected_ip_address.str().c_str(), result.host().c_str()); } } From a8c5a5127e7a900ffdfce22f1bc410bc5d346e36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Przytu=C5=82a?= Date: Mon, 16 Jun 2025 18:45:32 +0200 Subject: [PATCH 12/13] IT: test `allow_remote_dcs_for_local_cl` DCAwarePolicy integration test was missing a test case for the `allow_remote_dcs_for_local_cl` parameter. This commit adds a test case that verifies the behavior of the DCAwarePolicy when this parameter is set to either logical value. --- .../tests/test_dc_aware_policy.cpp | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/tests/src/integration/tests/test_dc_aware_policy.cpp b/tests/src/integration/tests/test_dc_aware_policy.cpp index 0281b1bb..2b53771f 100644 --- a/tests/src/integration/tests/test_dc_aware_policy.cpp +++ b/tests/src/integration/tests/test_dc_aware_policy.cpp @@ -16,7 +16,11 @@ #include "cassandra.h" #include "integration.hpp" +#include "objects/error_result.hpp" +#include "objects/execution_profile.hpp" +#include "objects/statement.hpp" +#include "gtest/gtest.h" #include #include @@ -56,6 +60,45 @@ class DcAwarePolicyTest : public Integration { return attempted_hosts; } + void validate_with_statement(Statement const& statement, bool const expect_no_hosts_available_error, bool const expect_remotes_used) { + std::vector attempted_hosts, temp; + Result result; + + result = session_.execute(statement, !expect_no_hosts_available_error); + temp = result.attempted_hosts(); + std::copy(temp.begin(), temp.end(), std::back_inserter(attempted_hosts)); + + if (expect_no_hosts_available_error) { + EXPECT_EQ(result.error_result().error_code(), CASS_ERROR_LIB_NO_HOSTS_AVAILABLE); + return; // No hosts available, so nothing more to check. + } else { + // The query should succeed, so we can check the result. + EXPECT_EQ(result.first_row().next().as(), Varchar("one")); + } + + if (expect_remotes_used) { + // Verify that remote DC hosts were used. + EXPECT_TRUE(contains(ccm_->get_ip_prefix() + "3", attempted_hosts) || + contains(ccm_->get_ip_prefix() + "4", attempted_hosts)); + + // Verify that no local DC hosts where used. + // + // Commented out, because I'm not sure if this is guaranteed: + // the driver may have already noticed that the local DC hosts + // are down and removed them from the host list. + // EXPECT_TRUE(!contains(ccm_->get_ip_prefix() + "1", attempted_hosts) && + // !contains(ccm_->get_ip_prefix() + "2", attempted_hosts)); + } else { + // Verify that local DC hosts were used. + EXPECT_TRUE(contains(ccm_->get_ip_prefix() + "1", attempted_hosts) || + contains(ccm_->get_ip_prefix() + "2", attempted_hosts)); + + // Verify that no remote DC hosts were used. + EXPECT_TRUE(!contains(ccm_->get_ip_prefix() + "3", attempted_hosts) && + !contains(ccm_->get_ip_prefix() + "4", attempted_hosts)); + } + } + Statement select_statement(const std::string& key) { Statement statement( format_string(CASSANDRA_SELECT_VALUE_FORMAT, table_name_.c_str(), key.c_str())); @@ -119,3 +162,73 @@ CASSANDRA_INTEGRATION_TEST_F(DcAwarePolicyTest, UsedHostsRemoteDc) { !contains(ccm_->get_ip_prefix() + "2", attempted_hosts)); } } + +/** + * Verify that the "allow remote DCs for local CL" setting allows requests that are using + * a local consistency level to be routed to remote DC nodes when the local DC nodes + * are unavailable. + * Also, verify that requests using a local consistency level are not routed to remote DC nodes + * when the local DC nodes are unavailable if the "allow remote DCs for local CL" setting is false. + * + * @test_category load_balancing_policy:dc_aware + */ +CASSANDRA_INTEGRATION_TEST_F(DcAwarePolicyTest, AllowRemoteDcsForLocalCl) { + CHECK_FAILURE + + cluster_ = default_cluster(); + char const *const local_dc = "dc1"; + cluster_.with_load_balance_dc_aware(local_dc, 42, false); + + ExecutionProfile profile_allowing_remote_dcs_for_local_cl = ExecutionProfile::build().with_load_balance_dc_aware(local_dc, 42, true); + char const *const profile_name = "allow_remote_dcs_for_local_cl"; + cluster_.with_execution_profile(profile_name, profile_allowing_remote_dcs_for_local_cl); + + connect(cluster_); + + // Create a test table and add test data to it + initialize(); + + Statement statement_with_nonlocal_consistency = select_statement("1"); + statement_with_nonlocal_consistency.set_consistency(CASS_CONSISTENCY_TWO); + + Statement statement_with_local_consistency = select_statement("1"); + statement_with_local_consistency.set_consistency(CASS_CONSISTENCY_LOCAL_ONE); + + std::cerr << "Testing with `allow_remote_dcs_for_local_cl`=`false`" << std::endl; + { + std::cerr << "Running nonlocal statement with local DC available" << std::endl; + // With local DC available, everything should succeed and only local DC hosts should be used. + validate_with_statement(statement_with_nonlocal_consistency, false, false); + + std::cerr << "Running local statement with local DC available" << std::endl; + // With local DC available, everything should succeed and only local DC hosts should be used. + validate_with_statement(statement_with_local_consistency, false, false); + + // Stop the whole local DC. + stop_node(1, true); + stop_node(2, true); + + std::cerr << "Running nonlocal statement with local DC unavailable" << std::endl; + // With local DC unavailable, remote DC available and nonlocal consistency used, remote hosts should be used and the request should succeed. + validate_with_statement(statement_with_nonlocal_consistency, false, true); + + std::cerr << "Running local statement with local DC unavailable" << std::endl; + // With local DC unavailable, remote DC available and local consistency used, remote hosts should be forbidden and the request should fail. + validate_with_statement(statement_with_local_consistency, true, false); + } + + statement_with_local_consistency.set_execution_profile(profile_name); + statement_with_nonlocal_consistency.set_execution_profile(profile_name); + std::cerr << "Testing with `allow_remote_dcs_for_local_cl`=`true`" << std::endl; + { + // The local DC nodes are still dead. + + std::cerr << "Running nonlocal statement with local DC unavailable" << std::endl; + // With local DC unavailable, remote DC available and nonlocal consistency used, remote hosts should be used and the request should succeed. + validate_with_statement(statement_with_nonlocal_consistency, false, true); + + std::cerr << "Running local statement with local DC unavailable" << std::endl; + // With local DC unavailable, remote DC available and local consistency used, remote hosts should be used and the request should succeed. + validate_with_statement(statement_with_local_consistency, false, true); + } +} From 946cccabf35cea4da9905278874d311d3cc95d61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Przytu=C5=82a?= Date: Tue, 17 Jun 2025 10:34:03 +0200 Subject: [PATCH 13/13] Makefile: enable DcAwarePolicyTest Once we: - implemented `get_attempted_hosts_from_future`, and - implemented correct support of the two deprecated options: - `used_hosts_per_remote_dc`, - `allow_remote_dcs_for_local_cl`, we can enable the `DcAwarePolicyTest` test suite. --- Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Makefile b/Makefile index 9fe04279..d89813a0 100644 --- a/Makefile +++ b/Makefile @@ -38,6 +38,7 @@ SCYLLA_TEST_FILTER := $(subst ${SPACE},${EMPTY},ClusterTests.*\ :MetricsTests.Integration_Cassandra_ErrorsRequestTimeouts\ :MetricsTests.Integration_Cassandra_Requests\ :MetricsTests.Integration_Cassandra_StatsShardConnections\ +:DcAwarePolicyTest.*\ :-PreparedTests.Integration_Cassandra_PreparedIDUnchangedDuringReprepare\ :SchemaMetadataTest.Integration_Cassandra_RegularMetadataNotMarkedVirtual\ :SchemaMetadataTest.Integration_Cassandra_VirtualMetadata\ @@ -97,6 +98,7 @@ CASSANDRA_TEST_FILTER := $(subst ${SPACE},${EMPTY},ClusterTests.*\ :MetricsTests.Integration_Cassandra_ErrorsRequestTimeouts\ :MetricsTests.Integration_Cassandra_Requests\ :MetricsTests.Integration_Cassandra_StatsShardConnections\ +:DcAwarePolicyTest.*\ :-PreparedTests.Integration_Cassandra_PreparedIDUnchangedDuringReprepare\ :PreparedTests.Integration_Cassandra_FailFastWhenPreparedIDChangesDuringReprepare\ :SchemaMetadataTest.Integration_Cassandra_RegularMetadataNotMarkedVirtual\