CASSCPP-5 cass_cluster_set_load_balance_dc_aware_n() fails with NULL/empty local DC

include/cassandra.h

@@ -2139,6 +2139,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
@@ -2154,7 +2158,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
@@ -2173,6 +2177,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
@@ -2181,11 +2189,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()
  */

src/cluster_config.cpp

@@ -281,8 +281,11 @@ 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;
+  // Allow NULL or empty local_dc to use the DC of the first connected node
+  if (local_dc == NULL || *local_dc == '\0') {
+    return cass_cluster_set_load_balance_dc_aware_n(cluster, NULL, 0,
+                                                    used_hosts_per_remote_dc,
+                                                    allow_remote_dcs_for_local_cl);
   }
   return cass_cluster_set_load_balance_dc_aware_n(cluster, local_dc, SAFE_STRLEN(local_dc),
                                                   used_hosts_per_remote_dc,
@@ -293,11 +296,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;
 }
 

tests/src/integration/tests/test_cluster.cpp

@@ -22,16 +22,26 @@ class ClusterTests : public Integration {
 };
 
 /**
- * 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));
+  }
 }
 
 /**

tests/src/unit/tests/test_cluster_config.cpp

@@ -0,0 +1,163 @@
+/*
+  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_);
+  }
+
+private:
+
+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; // Should just use "my_da" as the datacenter name
+  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 be using the partial string as local DC
+  EXPECT_EQ(dc_policy->local_dc(), String(long_dc_name, partial_length));
+  EXPECT_EQ(dc_policy->local_dc(), String("my_da"));
+  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());
+}