Skip to content

Commit 14bb41a

Browse files
wprzytulawprzytula
authored
Merge pull request #321 from wprzytula/dc-aware-lb-better-support
DC-aware LBP - better support the deprecated parameters
2 parents 91b67d5 + 946ccca commit 14bb41a

File tree

7 files changed

+552
-105
lines changed

7 files changed

+552
-105
lines changed

‎.github/workflows/build-lint-and-test.yml

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,16 @@ name: Build
22

33
on:
44
push:
5-
branches: [ master ]
5+
branches: [master]
66
pull_request:
7-
branches: [ master ]
7+
branches: [master]
88

99
env:
1010
CARGO_TERM_COLOR: always
1111
# Should include `INTEGRATION_TEST_BIN` from the `Makefile`
1212
# TODO: Remove `build/libscylla-cpp-driver.*` after https://github.com/scylladb/cpp-rust-driver/issues/164 is fixed.
1313
INTEGRATION_TEST_BIN: |
14-
build/cassandra-integration-tests
14+
build/cassandra-integration-tests
1515
build/libscylla-cpp-driver.*
1616
INTEGRATION_TEST_BIN_CACHE_KEY: integration-test-bin-${{ github.sha }}
1717
# Goes to `Makefile` to let it pickup cached binary
@@ -25,7 +25,7 @@ jobs:
2525
steps:
2626
- name: Checkout
2727
uses: actions/checkout@v4
28-
28+
2929
- name: Update apt cache
3030
run: sudo apt-get update -y
3131

@@ -57,7 +57,13 @@ jobs:
5757

5858
strategy:
5959
matrix:
60-
scylla-version: [ENTERPRISE-RELEASE, ENTERPRISE-PRIOR-RELEASE, OSS-RELEASE, OSS-PRIOR-RELEASE, 5.4.8]
60+
scylla-version:
61+
[
62+
ENTERPRISE-RELEASE,
63+
ENTERPRISE-PRIOR-RELEASE,
64+
OSS-RELEASE,
65+
OSS-PRIOR-RELEASE,
66+
]
6167
fail-fast: false
6268

6369
steps:
@@ -67,7 +73,7 @@ jobs:
6773
- name: Setup Python 3
6874
uses: actions/setup-python@v5
6975
with:
70-
python-version: '3.11'
76+
python-version: "3.11"
7177

7278
- name: Update apt cache
7379
run: sudo apt-get update -y
@@ -165,13 +171,13 @@ jobs:
165171
uses: actions/setup-java@v4
166172
with:
167173
java-version: ${{ matrix.java-version }}
168-
distribution: 'adopt'
174+
distribution: "adopt"
169175

170176
- name: Setup Python 3
171177
uses: actions/setup-python@v5
172178
with:
173-
python-version: '3.11'
174-
179+
python-version: "3.11"
180+
175181
- name: Update apt cache
176182
run: sudo apt-get update -y
177183

‎Makefile

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ SCYLLA_TEST_FILTER := $(subst ${SPACE},${EMPTY},ClusterTests.*\
3838
:MetricsTests.Integration_Cassandra_ErrorsRequestTimeouts\
3939
:MetricsTests.Integration_Cassandra_Requests\
4040
:MetricsTests.Integration_Cassandra_StatsShardConnections\
41+
:DcAwarePolicyTest.*\
4142
:-PreparedTests.Integration_Cassandra_PreparedIDUnchangedDuringReprepare\
4243
:SchemaMetadataTest.Integration_Cassandra_RegularMetadataNotMarkedVirtual\
4344
:SchemaMetadataTest.Integration_Cassandra_VirtualMetadata\
@@ -46,7 +47,6 @@ SCYLLA_TEST_FILTER := $(subst ${SPACE},${EMPTY},ClusterTests.*\
4647
:ExecutionProfileTest.Integration_Cassandra_RoundRobin\
4748
:ExecutionProfileTest.Integration_Cassandra_TokenAwareRouting\
4849
:ExecutionProfileTest.Integration_Cassandra_SpeculativeExecutionPolicy\
49-
:DCExecutionProfileTest.Integration_Cassandra_DCAware\
5050
:ControlConnectionTests.Integration_Cassandra_TopologyChange\
5151
:ControlConnectionTests.Integration_Cassandra_FullOutage\
5252
:ControlConnectionTests.Integration_Cassandra_TerminatedUsingMultipleIoThreadsWithError\
@@ -98,6 +98,7 @@ CASSANDRA_TEST_FILTER := $(subst ${SPACE},${EMPTY},ClusterTests.*\
9898
:MetricsTests.Integration_Cassandra_ErrorsRequestTimeouts\
9999
:MetricsTests.Integration_Cassandra_Requests\
100100
:MetricsTests.Integration_Cassandra_StatsShardConnections\
101+
:DcAwarePolicyTest.*\
101102
:-PreparedTests.Integration_Cassandra_PreparedIDUnchangedDuringReprepare\
102103
:PreparedTests.Integration_Cassandra_FailFastWhenPreparedIDChangesDuringReprepare\
103104
:SchemaMetadataTest.Integration_Cassandra_RegularMetadataNotMarkedVirtual\
@@ -107,7 +108,6 @@ CASSANDRA_TEST_FILTER := $(subst ${SPACE},${EMPTY},ClusterTests.*\
107108
:ExecutionProfileTest.Integration_Cassandra_RoundRobin\
108109
:ExecutionProfileTest.Integration_Cassandra_TokenAwareRouting\
109110
:ExecutionProfileTest.Integration_Cassandra_SpeculativeExecutionPolicy\
110-
:DCExecutionProfileTest.Integration_Cassandra_DCAware\
111111
:ControlConnectionTests.Integration_Cassandra_TopologyChange\
112112
:ControlConnectionTests.Integration_Cassandra_FullOutage\
113113
:ControlConnectionTests.Integration_Cassandra_TerminatedUsingMultipleIoThreadsWithError\

‎scylla-rust-wrapper/src/cluster.rs

Lines changed: 54 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ use crate::cass_error::CassError;
33
use crate::cass_types::CassConsistency;
44
use crate::exec_profile::{CassExecProfile, ExecProfileName, exec_profile_builder_modify};
55
use crate::future::CassFuture;
6-
use crate::load_balancing::{CassHostFilter, LoadBalancingConfig, LoadBalancingKind};
6+
use crate::load_balancing::{
7+
CassHostFilter, DcRestriction, LoadBalancingConfig, LoadBalancingKind,
8+
};
79
use crate::retry_policy::CassRetryPolicy;
810
use crate::ssl::CassSsl;
911
use crate::timestamp_generator::CassTimestampGen;
@@ -826,20 +828,44 @@ pub(crate) unsafe fn set_load_balance_dc_aware_n(
826828
used_hosts_per_remote_dc: c_uint,
827829
allow_remote_dcs_for_local_cl: cass_bool_t,
828830
) -> CassError {
829-
if local_dc_raw.is_null() || local_dc_length == 0 {
831+
let Some(local_dc) = (unsafe { ptr_to_cstr_n(local_dc_raw, local_dc_length) }) else {
832+
tracing::error!(
833+
"Provided null or non-UTF-8 local DC name to cass_*_set_load_balance_dc_aware(_n)!"
834+
);
830835
return CassError::CASS_ERROR_LIB_BAD_PARAMS;
831-
}
836+
};
832837

833-
if used_hosts_per_remote_dc != 0 || allow_remote_dcs_for_local_cl != 0 {
834-
// TODO: Add warning that the parameters are deprecated and not supported in the driver.
838+
if local_dc_length == 0 {
839+
tracing::error!("Provided empty local DC name to cass_*_set_load_balance_dc_aware(_n)!");
835840
return CassError::CASS_ERROR_LIB_BAD_PARAMS;
836841
}
837842

838-
let local_dc = unsafe { ptr_to_cstr_n(local_dc_raw, local_dc_length) }
839-
.unwrap()
840-
.to_string();
843+
let permit_dc_failover = if used_hosts_per_remote_dc > 0 {
844+
// TODO: update cassandra.h documentation to reflect this behaviour.
845+
tracing::warn!(
846+
"cass_*_set_load_balance_dc_aware(_n): `used_hosts_per_remote_dc` parameter is only partially \
847+
supported in the driver: `0` is supported correctly, and any value `>0` has the semantics of \"+inf\", \
848+
which means no limit on the number of hosts per remote DC. This is different from the original cpp-driver! \
849+
To clarify, you can understand this parameter as \"permit_dc_failover\", with `0` being `false` and `>0` \
850+
being `true`."
851+
);
852+
true
853+
} else {
854+
false
855+
};
841856

842-
load_balancing_config.load_balancing_kind = Some(LoadBalancingKind::DcAware { local_dc });
857+
let allow_remote_dcs_for_local_cl = allow_remote_dcs_for_local_cl != 0;
858+
859+
load_balancing_config.load_balancing_kind = Some(LoadBalancingKind::DcAware {
860+
local_dc: local_dc.to_owned(),
861+
permit_dc_failover,
862+
allow_remote_dcs_for_local_cl,
863+
});
864+
load_balancing_config.filtering.dc_restriction = if permit_dc_failover {
865+
DcRestriction::None
866+
} else {
867+
DcRestriction::Local(local_dc.to_owned())
868+
};
843869

844870
CassError::CASS_OK
845871
}
@@ -1757,7 +1783,7 @@ mod tests {
17571783
cass_cluster_set_load_balance_dc_aware(
17581784
cluster_raw.borrow_mut(),
17591785
c"eu".as_ptr(),
1760-
0,
1786+
0, // forbid DC failover
17611787
0
17621788
),
17631789
CassError::CASS_OK
@@ -1777,8 +1803,14 @@ mod tests {
17771803
let cluster = BoxFFI::as_ref(cluster_raw.borrow()).unwrap();
17781804
let load_balancing_kind = &cluster.load_balancing_config.load_balancing_kind;
17791805
match load_balancing_kind {
1780-
Some(LoadBalancingKind::DcAware { local_dc }) => {
1781-
assert_eq!(local_dc, "eu")
1806+
Some(LoadBalancingKind::DcAware {
1807+
local_dc,
1808+
permit_dc_failover,
1809+
allow_remote_dcs_for_local_cl,
1810+
}) => {
1811+
assert_eq!(local_dc, "eu");
1812+
assert!(!permit_dc_failover);
1813+
assert!(!allow_remote_dcs_for_local_cl);
17821814
}
17831815
_ => panic!("Expected preferred dc"),
17841816
}
@@ -1814,8 +1846,8 @@ mod tests {
18141846
cass_cluster_set_load_balance_dc_aware(
18151847
cluster_raw.borrow_mut(),
18161848
c"eu".as_ptr(),
1817-
0,
1818-
0
1849+
42, // allow DC failover
1850+
cass_true // allow remote DCs for local CL
18191851
),
18201852
CassError::CASS_OK
18211853
);
@@ -1824,34 +1856,20 @@ mod tests {
18241856
let node_location_preference =
18251857
&cluster.load_balancing_config.load_balancing_kind;
18261858
match node_location_preference {
1827-
Some(LoadBalancingKind::DcAware { local_dc }) => {
1828-
assert_eq!(local_dc, "eu")
1859+
Some(LoadBalancingKind::DcAware {
1860+
local_dc,
1861+
permit_dc_failover,
1862+
allow_remote_dcs_for_local_cl,
1863+
}) => {
1864+
assert_eq!(local_dc, "eu");
1865+
assert!(permit_dc_failover);
1866+
assert!(allow_remote_dcs_for_local_cl);
18291867
}
18301868
_ => panic!("Expected preferred dc"),
18311869
}
18321870
}
18331871
/* Test invalid configurations */
18341872
{
1835-
// Nonzero deprecated parameters
1836-
assert_cass_error_eq!(
1837-
cass_cluster_set_load_balance_dc_aware(
1838-
cluster_raw.borrow_mut(),
1839-
c"eu".as_ptr(),
1840-
1,
1841-
0
1842-
),
1843-
CassError::CASS_ERROR_LIB_BAD_PARAMS
1844-
);
1845-
assert_cass_error_eq!(
1846-
cass_cluster_set_load_balance_dc_aware(
1847-
cluster_raw.borrow_mut(),
1848-
c"eu".as_ptr(),
1849-
0,
1850-
1
1851-
),
1852-
CassError::CASS_ERROR_LIB_BAD_PARAMS
1853-
);
1854-
18551873
// null pointers
18561874
assert_cass_error_eq!(
18571875
cass_cluster_set_load_balance_dc_aware(

‎scylla-rust-wrapper/src/exec_profile.rs

Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -933,7 +933,7 @@ mod tests {
933933
cass_execution_profile_set_load_balance_dc_aware(
934934
profile_raw.borrow_mut(),
935935
c"eu".as_ptr(),
936-
0,
936+
0, // forbid DC failover
937937
0
938938
),
939939
CassError::CASS_OK
@@ -953,36 +953,20 @@ mod tests {
953953
let profile = BoxFFI::as_ref(profile_raw.borrow()).unwrap();
954954
let load_balancing_kind = &profile.load_balancing_config.load_balancing_kind;
955955
match load_balancing_kind {
956-
Some(LoadBalancingKind::DcAware { local_dc }) => {
957-
assert_eq!(local_dc, "eu")
956+
Some(LoadBalancingKind::DcAware {
957+
local_dc,
958+
permit_dc_failover,
959+
allow_remote_dcs_for_local_cl,
960+
}) => {
961+
assert_eq!(local_dc, "eu");
962+
assert!(!permit_dc_failover);
963+
assert!(!allow_remote_dcs_for_local_cl);
958964
}
959965
_ => panic!("Expected preferred dc"),
960966
}
961967
assert!(!profile.load_balancing_config.token_awareness_enabled);
962968
assert!(profile.load_balancing_config.latency_awareness_enabled);
963969
}
964-
/* Test invalid configurations */
965-
{
966-
// Nonzero deprecated parameters
967-
assert_cass_error_eq!(
968-
cass_execution_profile_set_load_balance_dc_aware(
969-
profile_raw.borrow_mut(),
970-
c"eu".as_ptr(),
971-
1,
972-
0
973-
),
974-
CassError::CASS_ERROR_LIB_BAD_PARAMS
975-
);
976-
assert_cass_error_eq!(
977-
cass_execution_profile_set_load_balance_dc_aware(
978-
profile_raw.borrow_mut(),
979-
c"eu".as_ptr(),
980-
0,
981-
1
982-
),
983-
CassError::CASS_ERROR_LIB_BAD_PARAMS
984-
);
985-
}
986970
}
987971

988972
cass_execution_profile_free(profile_raw);

0 commit comments

Comments
 (0)