diff --git a/test/boost/network_topology_strategy_test.cc b/test/boost/network_topology_strategy_test.cc index b090217785..0a5f78c8f9 100644 --- a/test/boost/network_topology_strategy_test.cc +++ b/test/boost/network_topology_strategy_test.cc @@ -10,6 +10,7 @@ #include #include "db/tablet_options.hh" #include "gms/inet_address.hh" +#include "gms/feature_service.hh" #include "inet_address_vectors.hh" #include "locator/abstract_replication_strategy.hh" #include "locator/host_id.hh" @@ -42,8 +43,10 @@ #include "test/lib/key_utils.hh" #include "test/lib/random_utils.hh" #include "test/lib/test_utils.hh" +#include "test/lib/topology_builder.hh" #include #include "db/schema_tables.hh" +#include "db/config.hh" using namespace locator; @@ -948,6 +951,144 @@ SEASTAR_TEST_CASE(test_invalid_dcs) { }); } +static +sstring describe(cql_test_env& e, sstring ks_name) { + auto& ks = e.local_db().find_keyspace(ks_name); + cql3::description desc = ks.metadata()->describe(e.local_db(), cql3::with_create_statement::yes); + auto result = desc.create_statement->linearize(); + testlog.info("DESCRIBE KEYSPACE {}: {}", ks_name, result); + return result; +} + +SEASTAR_TEST_CASE(test_rack_list_rf) { + auto cfg = cql_test_config(); + cfg.db_config->tablets_mode_for_new_keyspaces(db::tablets_mode_t::mode::enabled); + return do_with_cql_env_thread([] (auto& e) { + topology_builder topo(e); + + unsigned shard_count = 2; + topo.start_new_dc({"dc1", "rack1a"}); + topo.add_node(service::node_state::normal, shard_count); + topo.start_new_rack("rack1b"); + topo.add_node(service::node_state::normal, shard_count); + topo.start_new_dc({"dc2", "rack2a"}); + topo.add_node(service::node_state::normal, shard_count); + topo.start_new_rack("rack2b"); + topo.add_node(service::node_state::normal, shard_count); + + // Single rack + e.execute_cql("CREATE KEYSPACE ks11 WITH REPLICATION = {'class': 'NetworkTopologyStrategy', 'dc1': ['rack1a']}").get(); + BOOST_REQUIRE(describe(e, "ks11").contains("'dc1': ['rack1a']")); + + // Two racks + e.execute_cql("CREATE KEYSPACE ks12 WITH REPLICATION = {'class': 'NetworkTopologyStrategy', 'dc1': ['rack1a', 'rack1b']}").get(); + BOOST_REQUIRE(describe(e, "ks12").contains("'dc1': ['rack1a', 'rack1b']")); + + // Two DCs, two racks each + { + e.execute_cql("CREATE KEYSPACE ks22 WITH REPLICATION = {'class': 'NetworkTopologyStrategy', 'dc1': ['rack1a', 'rack1b'], " + "'dc2': ['rack2a', 'rack2b']} AND tablets = {'enabled':true}").get(); + auto& opts = e.local_db().find_keyspace("ks22").get_replication_strategy().get_config_options(); + BOOST_REQUIRE_EQUAL(replication_factor_data(opts.at("dc1")).get_rack_list(), + std::vector({"rack1a", "rack1b"})); + BOOST_REQUIRE_EQUAL(replication_factor_data(opts.at("dc2")).get_rack_list(), + std::vector({"rack2a", "rack2b"})); + BOOST_REQUIRE_EQUAL(replication_factor_data(opts.at("dc1")).count(), 2); + BOOST_REQUIRE_EQUAL(replication_factor_data(opts.at("dc2")).count(), 2); + BOOST_REQUIRE(describe(e, "ks22").contains("'dc1': ['rack1a', 'rack1b']")); + BOOST_REQUIRE(describe(e, "ks22").contains("'dc2': ['rack2a', 'rack2b']")); + } + + // Two DCs, one using rack list, one using numeric RF + // No auto-expansion to rack list. + { + e.execute_cql("CREATE KEYSPACE ks2n2 WITH REPLICATION = {'class': 'NetworkTopologyStrategy', 'dc1': 2, " + "'dc2': ['rack2a', 'rack2b']} AND tablets = {'enabled':true}").get(); + auto& opts = e.local_db().find_keyspace("ks2n2").get_replication_strategy().get_config_options(); + BOOST_REQUIRE(replication_factor_data(opts.at("dc1")).is_numeric()); + BOOST_REQUIRE_EQUAL(replication_factor_data(opts.at("dc2")).get_rack_list(), + std::vector({"rack2a", "rack2b"})); + BOOST_REQUIRE_EQUAL(replication_factor_data(opts.at("dc1")).count(), 2); + BOOST_REQUIRE_EQUAL(replication_factor_data(opts.at("dc2")).count(), 2); + BOOST_REQUIRE(describe(e, "ks2n2").contains("'dc1': '2'")); + BOOST_REQUIRE(describe(e, "ks2n2").contains("'dc2': ['rack2a', 'rack2b']")); + } + + // Non-existent DC + BOOST_REQUIRE_THROW(e.execute_cql( + "CREATE KEYSPACE fail WITH REPLICATION = {'class': 'NetworkTopologyStrategy', 'dc3': ['rack2a']}").get(), + exceptions::configuration_exception); + + // Rack from the wrong DC + BOOST_REQUIRE_THROW(e.execute_cql( + "CREATE KEYSPACE fail WITH REPLICATION = {'class': 'NetworkTopologyStrategy', 'dc1': ['rack2a']}").get(), + exceptions::configuration_exception); + + // Duplicated racks + BOOST_REQUIRE_THROW(e.execute_cql( + "CREATE KEYSPACE fail WITH REPLICATION = {'class': 'NetworkTopologyStrategy', 'dc1': ['rack1a', 'rack1b', 'rack1a']}").get(), + exceptions::configuration_exception); + + // Duplicated racks + BOOST_REQUIRE_THROW(e.execute_cql( + "CREATE KEYSPACE fail WITH REPLICATION = {'class': 'NetworkTopologyStrategy', 'dc1': ['rack1a', 'rack1a']}").get(), + exceptions::configuration_exception); + + // Alter to numeric with different count + BOOST_REQUIRE_THROW(e.execute_cql( + "ALTER KEYSPACE ks12 WITH REPLICATION = {'class': 'NetworkTopologyStrategy', 'dc1': 3}").get(), + exceptions::configuration_exception); + BOOST_REQUIRE_THROW(e.execute_cql( + "ALTER KEYSPACE ks12 WITH REPLICATION = {'class': 'NetworkTopologyStrategy', 'dc1': 1}").get(), + exceptions::configuration_exception); + }, cfg); +} + +SEASTAR_TEST_CASE(test_rack_list_rejected_when_feature_not_enabled) { + auto cfg = cql_test_config(); + cfg.db_config->tablets_mode_for_new_keyspaces(db::tablets_mode_t::mode::enabled); + cfg.disabled_features.insert("RACK_LIST_RF"); + return do_with_cql_env_thread([] (auto& e) { + auto& topo = e.get_shared_token_metadata().local().get()->get_topology(); + auto loc = topo.get_location(); + auto create_stmt = fmt::format("CREATE KEYSPACE test WITH REPLICATION = {{'class': 'NetworkTopologyStrategy'," + " '{}': ['{}']}}", loc.dc, loc.rack); + BOOST_REQUIRE_THROW(e.execute_cql(create_stmt).get(), exceptions::configuration_exception); + + // Auto-expansion to numeric RF should not happen + e.execute_cql(fmt::format("CREATE KEYSPACE test2 WITH REPLICATION = {{'class': 'NetworkTopologyStrategy', '{}': 1}}", loc.dc)).get(); + auto& opts = e.local_db().find_keyspace("test2").get_replication_strategy().get_config_options(); + BOOST_REQUIRE(replication_factor_data(opts.at(loc.dc)).is_numeric()); + BOOST_REQUIRE_EQUAL(replication_factor_data(opts.at(loc.dc)).count(), 1); + BOOST_REQUIRE(describe(e, "test2").contains(fmt::format("'{}': '1'", loc.dc))); + + // When feature is enabled, rack list is accepted. + e.get_feature_service().local().rack_list_rf.enable(); + e.execute_cql(create_stmt).get(); + + // Altering numeric RF to rack list is not supported yet. + BOOST_REQUIRE_THROW(e.execute_cql(fmt::format("ALTER KEYSPACE test2 WITH REPLICATION = {{'class': 'NetworkTopologyStrategy'," + " '{}': ['{}']}}", loc.dc, loc.rack)).get(), + exceptions::configuration_exception); + }, cfg); +} + +SEASTAR_TEST_CASE(test_rack_list_rejected_when_using_vnodes) { + auto cfg = cql_test_config(); + return do_with_cql_env_thread([] (auto& e) { + auto& topo = e.get_shared_token_metadata().local().get()->get_topology(); + auto loc = topo.get_location(); + auto create_stmt = [&] (bool tablets) { + return fmt::format("CREATE KEYSPACE abc WITH REPLICATION = {{'class': 'NetworkTopologyStrategy'," + " '{}': ['{}']}} and TABLETS = {{'enabled': {}}}", + loc.dc, loc.rack, tablets ? "true" : "false"); + }; + + BOOST_REQUIRE_THROW(e.execute_cql(create_stmt(false)).get(), exceptions::configuration_exception); + e.execute_cql(create_stmt(true)).get(); + }, cfg); +} + } // namespace network_topology_strategy_test namespace locator { diff --git a/test/cluster/util.py b/test/cluster/util.py index 3d7922687a..9f59d754e0 100644 --- a/test/cluster/util.py +++ b/test/cluster/util.py @@ -602,3 +602,26 @@ def disable_schema_agreement_wait(cql: Session): yield finally: cql.cluster.max_schema_agreement_wait = old_value + + +def parse_replication_options(replication_column) -> dict: + """ + Parses the value of the "replication" column from system_schema.keyspaces, which is a flattened map of options, + into an expanded map. + Expands a flattened map like {"dc0:0": "r1", "dc0:1": "r2"} into {"dc0": ["r1", "r2"]}. + See docs/dev/system_schema_keyspace.md + """ + result = {} + for key, value in replication_column.items(): + if ':' in key: + sub_key, index_str = key.split(':', 1) + if sub_key not in result: + result[sub_key] = [] + index = int(index_str) + while len(result[sub_key]) <= index: + result[sub_key].append(None) + if index >= 0: + result[sub_key][index] = value + else: + result[key] = value + return result diff --git a/test/cqlpy/test_keyspace.py b/test/cqlpy/test_keyspace.py index 17456b4003..ccc2f64666 100644 --- a/test/cqlpy/test_keyspace.py +++ b/test/cqlpy/test_keyspace.py @@ -10,14 +10,20 @@ import pytest from cassandra.protocol import SyntaxException, AlreadyExists, InvalidRequest, ConfigurationException from threading import Thread +from test.cluster.util import parse_replication_options + + # A basic tests for successful CREATE KEYSPACE and DROP KEYSPACE def test_create_and_drop_keyspace(cql, this_dc): cql.execute("CREATE KEYSPACE test_create_and_drop_keyspace WITH REPLICATION = { 'class' : 'NetworkTopologyStrategy', '" + this_dc + "' : 1 }") cql.execute("DROP KEYSPACE test_create_and_drop_keyspace") -def assert_keyspace(cql, keyspace, expected_class, rf_key): +def get_replication(cql, keyspace): row = cql.execute(f"SELECT replication FROM system_schema.keyspaces WHERE keyspace_name='{keyspace}'").one() - rep = row.replication + return parse_replication_options(row.replication) + +def assert_keyspace(cql, keyspace, expected_class, rf_key): + rep = get_replication(cql, keyspace) assert rep["class"] == expected_class assert rep[rf_key] == "1" @@ -72,6 +78,27 @@ def test_create_keyspace_if_not_exists(cql, this_dc): cql.execute("CREATE KEYSPACE IF NOT EXISTS test_create_keyspace_if_not_exists WITH REPLICATION = { 'class' : 'SimpleStrategy', 'replication_factor' : 2 }") cql.execute("DROP KEYSPACE test_create_keyspace_if_not_exists") +# We treat ALTER to numeric RF of same count as no-op. +def test_alter_rack_list_to_same_count_numeric_rf(cql, this_dc): + with new_test_keyspace(cql, f"WITH REPLICATION = {{ 'class' : 'NetworkTopologyStrategy', '{this_dc}': ['rack1'] }}") as keyspace: + cql.execute(f"ALTER KEYSPACE {keyspace} WITH REPLICATION = {{ 'class' : 'NetworkTopologyStrategy', '{this_dc}': 1 }}") + assert get_replication(cql, keyspace)[this_dc] == ['rack1'] + cql.execute(f"ALTER KEYSPACE {keyspace} WITH REPLICATION = {{ 'class' : 'NetworkTopologyStrategy', '{this_dc}': ['rack1'] }}") + +def test_empty_rack_list_is_accepted(cql, this_dc): + with new_test_keyspace(cql, f"WITH REPLICATION = {{ 'class' : 'NetworkTopologyStrategy', '{this_dc}': ['rack1'] }}") as keyspace: + cql.execute(f"ALTER KEYSPACE {keyspace} WITH REPLICATION = {{ 'class' : 'NetworkTopologyStrategy', '{this_dc}': [] }}") + assert this_dc not in get_replication(cql, keyspace) + +def test_can_alter_rack_list_to_0(cql, this_dc): + with new_test_keyspace(cql, f"WITH REPLICATION = {{ 'class' : 'NetworkTopologyStrategy', '{this_dc}': ['rack1'] }}") as keyspace: + cql.execute(f"ALTER KEYSPACE {keyspace} WITH REPLICATION = {{ 'class' : 'NetworkTopologyStrategy', '{this_dc}': 0 }}") + +def test_can_alter_to_rack_list_from_0(cql, this_dc): + with new_test_keyspace(cql, f"WITH REPLICATION = {{ 'class' : 'NetworkTopologyStrategy', '{this_dc}': 0 }}") as keyspace: + cql.execute(f"ALTER KEYSPACE {keyspace} WITH REPLICATION = {{ 'class' : 'NetworkTopologyStrategy', '{this_dc}': ['rack1'] }}") + assert get_replication(cql, keyspace)[this_dc] == ['rack1'] + # The documentation states that "Keyspace names can have alpha- # numeric characters and contain underscores; only letters and numbers are # supported as the first character.". This is not accurate. Test what is actually diff --git a/test/lib/topology_builder.hh b/test/lib/topology_builder.hh index 96873a528c..a12c80c2ea 100644 --- a/test/lib/topology_builder.hh +++ b/test/lib/topology_builder.hh @@ -156,6 +156,13 @@ public: return rack(); } + // Starts building a new rack in the current DC. + // Returns location of the new rack. + endpoint_dc_rack start_new_rack(sstring rack_name) { + _rack = std::move(rack_name); + return rack(); + } + // Starts building a new DC. // DC is named uniquely in the scope of the process, not just this object. endpoint_dc_rack start_new_dc() { @@ -165,6 +172,13 @@ public: return start_new_rack(); } + // Starts building a new DC. + endpoint_dc_rack start_new_dc(endpoint_dc_rack dc_and_rack) { + _dc = dc_and_rack.dc; + _rack = dc_and_rack.rack; + return rack(); + } + locator::load_stats_ptr get_load_stats() const { return _load_stats.get(); }