This is an automated email from the ASF dual-hosted git repository.
absurdfarce pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra-cpp-driver.git
The following commit(s) were added to refs/heads/trunk by this push:
new 9c8fa2b3 CASSCPP-5 cass_cluster_set_load_balance_dc_aware_n fails with
NULL/empty local DC
9c8fa2b3 is described below
commit 9c8fa2b358ae27c55229c0c2323d97d23f1e66c0
Author: absurdfarce <[email protected]>
AuthorDate: Thu May 1 01:21:13 2025 -0500
CASSCPP-5 cass_cluster_set_load_balance_dc_aware_n fails with NULL/empty
local DC
patch by Bret McGuire; reviewed by Bret McGuire and Yifan Cai
---
include/cassandra.h | 16 ++-
src/cluster_config.cpp | 13 +--
tests/src/integration/tests/test_cluster.cpp | 22 +++-
tests/src/unit/tests/test_cluster_config.cpp | 160 +++++++++++++++++++++++++++
4 files changed, 194 insertions(+), 17 deletions(-)
diff --git a/include/cassandra.h b/include/cassandra.h
index 6d1ac7aa..c51bfa28 100644
--- a/include/cassandra.h
+++ b/include/cassandra.h
@@ -2141,6 +2141,10 @@ cass_cluster_set_load_balance_round_robin(CassCluster*
cluster);
* For each query, all live nodes in a primary 'local' DC are tried first,
* followed by any node from other DCs.
*
+ * <b>Important:</b> Pass in NULL for local_dc to automatically use the data
center of the first
+ * connected control connection host as the primary data center. This approach
is recommended
+ * over manually selecting the local_dc.
+ *
* <b>Note:</b> This is the default, and does not need to be called unless
* switching an existing from another policy or changing settings.
* Without further configuration, a default local_dc is chosen from the
@@ -2156,7 +2160,7 @@ cass_cluster_set_load_balance_round_robin(CassCluster*
cluster);
* @public @memberof CassCluster
*
* @param[in] cluster
- * @param[in] local_dc The primary data center to try first
+ * @param[in] local_dc The primary data center to try first, or NULL to use
the DC of the first connected node
* @param[in] used_hosts_per_remote_dc The number of hosts used in each remote
* DC if no hosts are available in the local dc (<b>deprecated</b>)
* @param[in] allow_remote_dcs_for_local_cl Allows remote hosts to be used if
no
@@ -2175,6 +2179,10 @@ cass_cluster_set_load_balance_dc_aware(CassCluster*
cluster,
* Same as cass_cluster_set_load_balance_dc_aware(), but with lengths for
string
* parameters.
*
+ * <b>Important:</b> If local_dc is NULL or local_dc_length is 0, the data
center of
+ * the first connected control connection host will be automatically used as
the
+ * primary data center. This approach is recommended over manually selecting
the local_dc.
+ *
* @deprecated The remote DC settings for DC-aware are not suitable for most
* scenarios that require DC failover. There is also unhandled gap between
* replication factor number of nodes failing and the full cluster failing.
Only
@@ -2183,11 +2191,11 @@ cass_cluster_set_load_balance_dc_aware(CassCluster*
cluster,
* @public @memberof CassCluster
*
* @param[in] cluster
- * @param[in] local_dc
- * @param[in] local_dc_length
+ * @param[in] local_dc Primary data center to try first, or NULL to use the DC
of the first connected node
+ * @param[in] local_dc_length Length of local_dc string
* @param[in] used_hosts_per_remote_dc (<b>deprecated</b>)
* @param[in] allow_remote_dcs_for_local_cl (<b>deprecated</b>)
- * @return same as cass_cluster_set_load_balance_dc_aware()
+ * @return CASS_OK if successful, otherwise an error occurred
*
* @see cass_cluster_set_load_balance_dc_aware()
*/
diff --git a/src/cluster_config.cpp b/src/cluster_config.cpp
index ddeae121..5dcc1308 100644
--- a/src/cluster_config.cpp
+++ b/src/cluster_config.cpp
@@ -283,9 +283,6 @@ void cass_cluster_set_load_balance_round_robin(CassCluster*
cluster) {
CassError cass_cluster_set_load_balance_dc_aware(CassCluster* cluster, const
char* local_dc,
unsigned
used_hosts_per_remote_dc,
cass_bool_t
allow_remote_dcs_for_local_cl) {
- if (local_dc == NULL) {
- return CASS_ERROR_LIB_BAD_PARAMS;
- }
return cass_cluster_set_load_balance_dc_aware_n(cluster, local_dc,
SAFE_STRLEN(local_dc),
used_hosts_per_remote_dc,
allow_remote_dcs_for_local_cl);
@@ -295,11 +292,13 @@ CassError
cass_cluster_set_load_balance_dc_aware_n(CassCluster* cluster, const c
size_t local_dc_length,
unsigned
used_hosts_per_remote_dc,
cass_bool_t
allow_remote_dcs_for_local_cl) {
- if (local_dc == NULL || local_dc_length == 0) {
- return CASS_ERROR_LIB_BAD_PARAMS;
- }
+ // If local_dc is NULL or length is 0, we use an empty string which causes
the driver
+ // to use the DC of the first connected node
+ String dc_name = (local_dc != NULL && local_dc_length > 0) ?
+ String(local_dc, local_dc_length) : String();
+
cluster->config().set_load_balancing_policy(new DCAwarePolicy(
- String(local_dc, local_dc_length), used_hosts_per_remote_dc,
!allow_remote_dcs_for_local_cl));
+ dc_name, used_hosts_per_remote_dc, !allow_remote_dcs_for_local_cl));
return CASS_OK;
}
diff --git a/tests/src/integration/tests/test_cluster.cpp
b/tests/src/integration/tests/test_cluster.cpp
index a21ca6dc..41dbb605 100644
--- a/tests/src/integration/tests/test_cluster.cpp
+++ b/tests/src/integration/tests/test_cluster.cpp
@@ -24,16 +24,26 @@ public:
};
/**
- * Set local dc to null for dc-aware lbp
+ * Set local dc to null or empty for dc-aware lbp
*
* @jira_ticket CPP-368
* @test_category configuration
- * @expected_result Error out because it is illegal to specify a null local-dc.
+ * @expected_result Success because NULL or empty local-dc now means use the
DC of the first connected node.
*/
-CASSANDRA_INTEGRATION_TEST_F(ClusterTests, SetLoadBalanceDcAwareNullLocalDc) {
- test::driver::Cluster cluster;
- EXPECT_EQ(CASS_ERROR_LIB_BAD_PARAMS,
- cass_cluster_set_load_balance_dc_aware(cluster.get(), NULL, 99,
cass_false));
+CASSANDRA_INTEGRATION_TEST_F(ClusterTests,
SetLoadBalanceDcAwareNullOrEmptyLocalDc) {
+ // Test with NULL local_dc
+ {
+ test::driver::Cluster cluster;
+ EXPECT_EQ(CASS_OK,
+ cass_cluster_set_load_balance_dc_aware(cluster.get(), NULL, 99,
cass_false));
+ }
+
+ // Test with empty string local_dc
+ {
+ test::driver::Cluster cluster;
+ EXPECT_EQ(CASS_OK,
+ cass_cluster_set_load_balance_dc_aware(cluster.get(), "", 99,
cass_false));
+ }
}
/**
diff --git a/tests/src/unit/tests/test_cluster_config.cpp
b/tests/src/unit/tests/test_cluster_config.cpp
new file mode 100644
index 00000000..73eaa002
--- /dev/null
+++ b/tests/src/unit/tests/test_cluster_config.cpp
@@ -0,0 +1,160 @@
+/*
+ Copyright (c) DataStax, Inc.
+
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+#include <gtest/gtest.h>
+
+#include "cluster_config.hpp"
+#include "cassandra.h"
+#include "dc_aware_policy.hpp"
+#include "string.hpp"
+
+using namespace datastax;
+using namespace datastax::internal;
+using namespace datastax::internal::core;
+
+class ClusterConfigUnitTest : public testing::Test {
+public:
+ ClusterConfigUnitTest() {}
+
+ virtual void SetUp() {
+ cluster_ = cass_cluster_new();
+ }
+
+ virtual void TearDown() {
+ cass_cluster_free(cluster_);
+ }
+
+protected:
+ CassCluster* cluster_;
+
+ const DCAwarePolicy* build_dc_policy() {
+
+ // Disable token-aware routing in this case in order to avoid illegal
instructions
+ // when creating a TokenAwarePolicy in build_load_balancing_policy()
+ cluster_->config().set_token_aware_routing(false);
+ cluster_->config().default_profile().build_load_balancing_policy();
+
+ // Verify the policy was set correctly with the right datacenter name
+ const LoadBalancingPolicy* policy =
+ cluster_->config().load_balancing_policy().get();
+ return static_cast<const DCAwarePolicy*>(policy);
+ }
+};
+
+// ==================== cass_cluster_set_load_balance_dc_aware_n
====================
+TEST_F(ClusterConfigUnitTest, SetLoadBalanceDcAwareNHappyPath) {
+
+ // Test valid parameters
+ const char* valid_dc = "my_datacenter";
+ EXPECT_EQ(CASS_OK, cass_cluster_set_load_balance_dc_aware_n(
+ cluster_, valid_dc, strlen(valid_dc),
+ 2, cass_true));
+
+ const DCAwarePolicy* dc_policy = build_dc_policy();
+
+ // Should be using the partial string as local DC
+ EXPECT_EQ(dc_policy->local_dc(), valid_dc);
+ EXPECT_EQ(dc_policy->used_hosts_per_remote_dc(), 2u);
+ EXPECT_FALSE(dc_policy->skip_remote_dcs_for_local_cl());
+}
+
+TEST_F(ClusterConfigUnitTest, SetLoadBalanceDcAwareNWithNullLocalDc) {
+
+ // Test with NULL pointer (should now succeed and use empty string for DC)
+ EXPECT_EQ(CASS_OK,
+ cass_cluster_set_load_balance_dc_aware_n(
+ cluster_, NULL, 10, 2, cass_true));
+
+ const DCAwarePolicy* dc_policy = build_dc_policy();
+
+ EXPECT_EQ(dc_policy->local_dc(), String());
+ EXPECT_EQ(dc_policy->used_hosts_per_remote_dc(), 2u);
+ EXPECT_FALSE(dc_policy->skip_remote_dcs_for_local_cl());
+}
+
+TEST_F(ClusterConfigUnitTest, SetLoadBalanceDcAwareNWithZeroLengthLocalDc) {
+
+ // Test with zero length (should now succeed and use empty string for DC)
+ const char* valid_dc = "my_datacenter";
+ EXPECT_EQ(CASS_OK,
+ cass_cluster_set_load_balance_dc_aware_n(
+ cluster_, valid_dc, 0, 2, cass_true));
+
+ const DCAwarePolicy* dc_policy = build_dc_policy();
+
+ EXPECT_EQ(dc_policy->local_dc(), String());
+ EXPECT_EQ(dc_policy->used_hosts_per_remote_dc(), 2u);
+ EXPECT_FALSE(dc_policy->skip_remote_dcs_for_local_cl());
+}
+
+TEST_F(ClusterConfigUnitTest, SetLoadBalanceDcAwareNWithEmptyLocalDc) {
+
+ // Test with empty string (should now succeed)
+ const char* empty_string = "";
+ EXPECT_EQ(CASS_OK,
+ cass_cluster_set_load_balance_dc_aware_n(
+ cluster_, empty_string, strlen(empty_string), 2, cass_true));
+
+ const DCAwarePolicy* dc_policy = build_dc_policy();
+
+ EXPECT_EQ(dc_policy->local_dc(), String());
+ EXPECT_EQ(dc_policy->used_hosts_per_remote_dc(), 2u);
+ EXPECT_FALSE(dc_policy->skip_remote_dcs_for_local_cl());
+}
+
+TEST_F(ClusterConfigUnitTest, SetLoadBalanceDcAwareNWithPartialStringLocalDc) {
+
+ // Test with partial string length
+ const char* long_dc_name = "my_datacenter_with_a_long_name";
+ size_t partial_length = 5;
+ EXPECT_EQ(CASS_OK, cass_cluster_set_load_balance_dc_aware_n(
+ cluster_, long_dc_name, partial_length,
+ 2, cass_true));
+
+ const DCAwarePolicy* dc_policy = build_dc_policy();
+
+ // Should just use up to partial_length of the datacenter name here.
+ EXPECT_EQ(dc_policy->local_dc(), String(long_dc_name, partial_length));
+ EXPECT_EQ(dc_policy->used_hosts_per_remote_dc(), 2u);
+ EXPECT_FALSE(dc_policy->skip_remote_dcs_for_local_cl());
+}
+
+// ==================== cass_cluster_set_load_balance_dc_aware
====================
+TEST_F(ClusterConfigUnitTest, SetLoadBalanceDcAwareWithNullLocalDc) {
+ // Test with NULL to use local DC from connected node
+ EXPECT_EQ(CASS_OK, cass_cluster_set_load_balance_dc_aware(
+ cluster_, NULL, 3, cass_false));
+
+ const DCAwarePolicy* dc_policy = build_dc_policy();
+
+ // Should be using empty string as local DC (will be determined at runtime)
+ EXPECT_EQ(dc_policy->local_dc(), String());
+ EXPECT_EQ(dc_policy->used_hosts_per_remote_dc(), 3u);
+ EXPECT_TRUE(dc_policy->skip_remote_dcs_for_local_cl());
+}
+
+TEST_F(ClusterConfigUnitTest, SetLoadBalanceDcAwareWithEmptyLocalDc) {
+ // Test with empty string to use local DC from connected node
+ EXPECT_EQ(CASS_OK, cass_cluster_set_load_balance_dc_aware(
+ cluster_, "", 2, cass_true));
+
+ const DCAwarePolicy* dc_policy = build_dc_policy();
+
+ // Should be using empty string as local DC (will be determined at runtime)
+ EXPECT_EQ(dc_policy->local_dc(), String());
+ EXPECT_EQ(dc_policy->used_hosts_per_remote_dc(), 2u);
+ EXPECT_FALSE(dc_policy->skip_remote_dcs_for_local_cl());
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
