main: Refuse to start node when RF-rack-invalid keyspace exists

When a node is started with the option `rf_rack_valid_keyspaces`
enabled, the initialization will fail if there is an RF-rack-invalid
keyspace. We want to force the user to adjust their existing
keyspaces when upgrading to 2025.* so that the invariant that
every keyspace is RF-rack-valid is always satisfied.

Fixes scylladb#23300

Author: dawmd

main.cc

@@ -13,6 +13,7 @@
 
 #include <seastar/util/closeable.hh>
 #include <seastar/core/abort_source.hh>
+#include "exceptions/exceptions.hh"
 #include "gms/inet_address.hh"
 #include "auth/allow_all_authenticator.hh"
 #include "auth/allow_all_authorizer.hh"
@@ -2176,6 +2177,14 @@ sharded<locator::shared_token_metadata> token_metadata;
                 return ss.local().join_cluster(proxy, service::start_hint_manager::yes, generation_number);
             }).get();
 
+            // At this point, `locator::topology` should be stable, i.e. we should have complete information
+            // about the layout of the cluster (= list of nodes along with the racks/DCs).
+            if (cfg->rf_rack_valid_keyspaces()) {
+                startlog.info("Verifying that all of the keyspaces are RF-rack-valid");
+                db.local().check_rf_rack_validity(token_metadata.local().get());
+                startlog.info("All keyspaces are RF-rack-valid");
+            }
+
             dictionary_service dict_service(
                 dict_sampler,
                 sys_ks.local(),

replica/database.cc

@@ -10,6 +10,9 @@
 
 #include <fmt/ranges.h>
 #include <fmt/std.h>
+#include "locator/network_topology_strategy.hh"
+#include "locator/tablets.hh"
+#include "locator/token_metadata_fwd.hh"
 #include "utils/log.hh"
 #include "replica/database_fwd.hh"
 #include "utils/assert.hh"
@@ -3184,4 +3187,12 @@ database::on_effective_service_levels_cache_reloaded() {
     co_return;
 }
 
+void database::check_rf_rack_validity(const locator::token_metadata_ptr tmptr) const {
+    SCYLLA_ASSERT(get_config().rf_rack_valid_keyspaces());
+
+    for (const auto& [name, info] : get_keyspaces()) {
+        locator::assert_rf_rack_valid_keyspace(name, tmptr, info.get_replication_strategy());
+    }
+}
+
 }

replica/database.hh

@@ -1951,6 +1951,14 @@ public:
     /** This callback is going to be called just before the service level is changed **/
     virtual future<> on_before_service_level_change(qos::service_level_options slo_before, qos::service_level_options slo_after, qos::service_level_info sl_info) override;
     virtual future<> on_effective_service_levels_cache_reloaded() override;
+
+    // Verify that the existing keyspaces are all RF-rack-valid.
+    //
+    // Preconditions:
+    // * the option `rf_rack_valid_keyspaces` in enabled,
+    // * the `locator::topology` instance corresponding to the passed `locator::token_metadata_ptr`
+    //   must contain a complete list of racks and data centers in the cluster.
+    void check_rf_rack_validity(const locator::token_metadata_ptr) const;
 };
 
 // A helper function to parse the directory name back

test/cluster/test_multidc.py

@@ -384,3 +384,81 @@ async def create_fail(rfs: Union[List[int], int], failed_dc: int, rf: int, rack_
     async with asyncio.TaskGroup() as tg:
         for task in [*valid_keyspaces, *invalid_keyspaces]:
             _ = tg.create_task(task)
+
+async def test_startup_with_keyspaces_violating_rf_rack_valid_keyspaces(manager: ManagerClient):
+    """
+    This test verifies that starting a Scylla node fails when there's an RF-rack-invalid keyspace.
+    We aim to simulate the behavior of a node when upgrading to 2025.*.
+
+    For more context, see: scylladb/scylladb#23300.
+    """
+
+    cfg_false = {"rf_rack_valid_keyspaces": "false"}
+
+    s1 = await manager.server_add(config=cfg_false, property_file={"dc": "dc1", "rack": "r1"})
+    _ = await manager.server_add(config=cfg_false, property_file={"dc": "dc1", "rack": "r2"})
+    _ = await manager.server_add(config=cfg_false, property_file={"dc": "dc1", "rack": "r3"})
+    _ = await manager.server_add(config=cfg_false, property_file={"dc": "dc2", "rack": "r4"})
+    # Note: This rack should behave as if it never existed.
+    _ = await manager.server_add(config={"join_ring": "false", "rf_rack_valid_keyspaces": "false"}, property_file={"dc": "dc1", "rack": "rzerotoken"})
+
+    # Current situation:
+    # DC1: {r1, r2, r3}, DC2: {r4}
+
+    cql = manager.get_cql()
+
+    async def create_keyspace(rfs: List[int], tablets: bool) -> str:
+        dcs = ", ".join([f"'dc{i + 1}': {rf}" for i, rf in enumerate(rfs)])
+        name = unique_name()
+        tablets = str(tablets).lower()
+        await cql.run_async(f"CREATE KEYSPACE {name} WITH REPLICATION = {{'class': 'NetworkTopologyStrategy', {dcs}}} "
+                            f"AND tablets = {{'enabled': {tablets}}}")
+        logger.info(f"Created keyspace {name} with {rfs} and tablets={tablets}")
+        return name
+
+    valid_keyspaces = [
+        # For each DC: RF \in {0, 1, #racks}.
+        ([0, 0], True),
+        ([1, 0], True),
+        ([3, 0], True),
+        ([1, 1], True),
+        ([3, 1], True),
+        # Reminder: Keyspaces not using tablets are all valid.
+        ([0, 0], False),
+        ([1, 0], False),
+        ([2, 0], False),
+        ([3, 0], False),
+        ([4, 0], False),
+        ([0, 1], False),
+        ([1, 1], False),
+        ([2, 1], False),
+        ([3, 1], False),
+        ([4, 1], False),
+        ([0, 2], False),
+        ([1, 2], False),
+        ([2, 2], False),
+        ([3, 2], False),
+        ([4, 2], False)
+    ]
+
+    # Populate RF-rack-valid keyspaces.
+    async with asyncio.TaskGroup() as tg:
+        for rfs, tablets in valid_keyspaces:
+            _ = tg.create_task(create_keyspace(rfs, tablets))
+
+    await manager.server_stop_gracefully(s1.server_id)
+    await manager.server_update_config(s1.server_id, "rf_rack_valid_keyspaces", "true")
+
+    async def try_fail(rfs: List[int], dc: str, rf: int, rack_count: int):
+        ks = await create_keyspace(rfs, True)
+        err = r"The option `rf_rack_valid_keyspaces` is enabled. It requires that all keyspaces are RF-rack-valid. " \
+              f"That condition is violated: keyspace '{ks}' doesn't satisfy it for DC '{dc}': RF={rf} vs. rack count={rack_count}."
+        _ = await manager.server_start(s1.server_id, expected_error=err)
+        await cql.run_async(f"DROP KEYSPACE {ks}")
+
+    # Test RF-rack-invalid keyspaces.
+    await try_fail([2, 0], "dc1", 2, 3)
+    await try_fail([3, 2], "dc2", 2, 1)
+    await try_fail([4, 1], "dc1", 4, 3)
+
+    _ = await manager.server_start(s1.server_id)

test/lib/cql_test_env.cc

@@ -1050,6 +1050,12 @@ class single_node_cql_env : public cql_test_env {
                 throw;
             }
 
+            if (cfg->rf_rack_valid_keyspaces()) {
+                startlog.info("Verifying that all of the keyspaces are RF-rack-valid");
+                _db.local().check_rf_rack_validity(_token_metadata.local().get());
+                startlog.info("All keyspaces are RF-rack-valid");
+            }
+
             utils::loading_cache_config perm_cache_config;
             perm_cache_config.max_size = cfg->permissions_cache_max_entries();
             perm_cache_config.expiry = std::chrono::milliseconds(cfg->permissions_validity_in_ms());