From 562fcf0c1931f478ac860e04e765b8adee6f6972 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Thu, 21 Dec 2023 15:06:53 +0300 Subject: [PATCH] locator: Keep optional initial_tablets on r.s. params Now all the callers have it at hands (spoiler: not yet initialized, but still) so the params can also have it. Signed-off-by: Pavel Emelyanov --- cdc/log.cc | 2 +- cql3/statements/create_keyspace_statement.cc | 2 +- data_dictionary/data_dictionary.cc | 2 +- locator/abstract_replication_strategy.hh | 3 ++- replica/database.cc | 2 +- service/tablet_allocator.cc | 2 +- test/boost/network_topology_strategy_test.cc | 14 +++++++------- test/boost/token_metadata_test.cc | 2 +- 8 files changed, 15 insertions(+), 14 deletions(-) diff --git a/cdc/log.cc b/cdc/log.cc index aea9360ea0..5413be7d7f 100644 --- a/cdc/log.cc +++ b/cdc/log.cc @@ -272,7 +272,7 @@ private: // to be attempted - in particular the log table we try to create will not // have tablets, and will cause a failure. static void ensure_that_table_uses_vnodes(const keyspace_metadata& ksm, const schema& schema) { - locator::replication_strategy_params params(ksm.strategy_options()); + locator::replication_strategy_params params(ksm.strategy_options(), ksm.initial_tablets()); auto rs = locator::abstract_replication_strategy::create_replication_strategy(ksm.strategy_name(), params); if (rs->uses_tablets()) { throw exceptions::invalid_request_exception(format("Cannot create CDC log for a table {}.{}, because keyspace uses tablets. See issue #16317.", diff --git a/cql3/statements/create_keyspace_statement.cc b/cql3/statements/create_keyspace_statement.cc index f1bf62b9e7..4963ef2cdc 100644 --- a/cql3/statements/create_keyspace_statement.cc +++ b/cql3/statements/create_keyspace_statement.cc @@ -147,7 +147,7 @@ std::vector check_against_restricted_replication_strategies( std::vector warnings; locator::replication_strategy_config_options opts; - locator::replication_strategy_params params(opts); + locator::replication_strategy_params params(opts, std::nullopt); auto replication_strategy = locator::abstract_replication_strategy::create_replication_strategy( locator::abstract_replication_strategy::to_qualified_class_name( *attrs.get_replication_strategy_class()), params)->get_type(); diff --git a/data_dictionary/data_dictionary.cc b/data_dictionary/data_dictionary.cc index cd21a2ec9b..7c5a474af0 100644 --- a/data_dictionary/data_dictionary.cc +++ b/data_dictionary/data_dictionary.cc @@ -251,7 +251,7 @@ keyspace_metadata::keyspace_metadata(std::string_view name, void keyspace_metadata::validate(const gms::feature_service& fs, const locator::topology& topology) const { using namespace locator; - locator::replication_strategy_params params(strategy_options()); + locator::replication_strategy_params params(strategy_options(), initial_tablets()); abstract_replication_strategy::validate_replication_strategy(name(), strategy_name(), params, fs, topology); } diff --git a/locator/abstract_replication_strategy.hh b/locator/abstract_replication_strategy.hh index 7dd8666a8c..3480b846f5 100644 --- a/locator/abstract_replication_strategy.hh +++ b/locator/abstract_replication_strategy.hh @@ -51,7 +51,8 @@ using can_yield = utils::can_yield; using replication_strategy_config_options = std::map; struct replication_strategy_params { const replication_strategy_config_options& options; - explicit replication_strategy_params(const replication_strategy_config_options& o) noexcept : options(o) {} + std::optional initial_tablets; + explicit replication_strategy_params(const replication_strategy_config_options& o, std::optional it) noexcept : options(o), initial_tablets(it) {} }; using replication_map = std::unordered_map; diff --git a/replica/database.cc b/replica/database.cc index dbf90dc9e5..338b9cf393 100644 --- a/replica/database.cc +++ b/replica/database.cc @@ -1183,7 +1183,7 @@ future<> keyspace::create_replication_strategy(const locator::shared_token_metadata& stm) { using namespace locator; - locator::replication_strategy_params params(_metadata->strategy_options()); + locator::replication_strategy_params params(_metadata->strategy_options(), _metadata->initial_tablets()); _replication_strategy = abstract_replication_strategy::create_replication_strategy(_metadata->strategy_name(), params); rslogger.debug("replication strategy for keyspace {} is {}, opts={}", diff --git a/service/tablet_allocator.cc b/service/tablet_allocator.cc index ebd9719238..4d09607bf2 100644 --- a/service/tablet_allocator.cc +++ b/service/tablet_allocator.cc @@ -825,7 +825,7 @@ public: } void on_before_create_column_family(const keyspace_metadata& ksm, const schema& s, std::vector& muts, api::timestamp_type ts) override { - locator::replication_strategy_params params(ksm.strategy_options()); + locator::replication_strategy_params params(ksm.strategy_options(), ksm.initial_tablets()); auto rs = abstract_replication_strategy::create_replication_strategy(ksm.strategy_name(), params); if (auto&& tablet_rs = rs->maybe_as_tablet_aware()) { auto tm = _db.get_shared_token_metadata().get(); diff --git a/test/boost/network_topology_strategy_test.cc b/test/boost/network_topology_strategy_test.cc index 1b7e58ff15..e9c8995526 100644 --- a/test/boost/network_topology_strategy_test.cc +++ b/test/boost/network_topology_strategy_test.cc @@ -267,7 +267,7 @@ void simple_test() { {"101", "2"}, {"102", "3"} }; - locator::replication_strategy_params params323(options323); + locator::replication_strategy_params params323(options323, std::nullopt); auto ars_ptr = abstract_replication_strategy::create_replication_strategy( "NetworkTopologyStrategy", params323); @@ -281,7 +281,7 @@ void simple_test() { {"101", "2"}, {"102", "0"} }; - locator::replication_strategy_params params320(options320); + locator::replication_strategy_params params320(options320, std::nullopt); ars_ptr = abstract_replication_strategy::create_replication_strategy( "NetworkTopologyStrategy", params320); @@ -367,7 +367,7 @@ void heavy_origin_test() { } }).get(); - locator::replication_strategy_params params(config_options); + locator::replication_strategy_params params(config_options, std::nullopt); auto ars_ptr = abstract_replication_strategy::create_replication_strategy( "NetworkTopologyStrategy", params); @@ -435,7 +435,7 @@ SEASTAR_THREAD_TEST_CASE(NetworkTopologyStrategy_tablets_test) { {"102", "3"}, {"initial_tablets", "100"} }; - locator::replication_strategy_params params323(options323); + locator::replication_strategy_params params323(options323, std::nullopt); auto ars_ptr = abstract_replication_strategy::create_replication_strategy( "NetworkTopologyStrategy", params323); @@ -459,7 +459,7 @@ SEASTAR_THREAD_TEST_CASE(NetworkTopologyStrategy_tablets_test) { {"102", "0"}, {"initial_tablets", "100"} }; - locator::replication_strategy_params params320(options320); + locator::replication_strategy_params params320(options320, std::nullopt); ars_ptr = abstract_replication_strategy::create_replication_strategy( "NetworkTopologyStrategy", params320); @@ -476,7 +476,7 @@ SEASTAR_THREAD_TEST_CASE(NetworkTopologyStrategy_tablets_test) { {"102", "2"}, {"initial_tablets", "100"} }; - locator::replication_strategy_params params324(options324); + locator::replication_strategy_params params324(options324, std::nullopt); ars_ptr = abstract_replication_strategy::create_replication_strategy( "NetworkTopologyStrategy", params324); @@ -654,7 +654,7 @@ static void test_equivalence(const shared_token_metadata& stm, const locator::to | boost::adaptors::transformed( [](const std::pair& p) { return std::make_pair(p.first, to_sstring(p.second)); - })))); + })), std::nullopt)); const token_metadata& tm = *stm.get(); for (size_t i = 0; i < 1000; ++i) { diff --git a/test/boost/token_metadata_test.cc b/test/boost/token_metadata_test.cc index 3b1ce671c2..29317ae07d 100644 --- a/test/boost/token_metadata_test.cc +++ b/test/boost/token_metadata_test.cc @@ -41,7 +41,7 @@ namespace { mutable_vnode_erm_ptr create_erm(mutable_token_metadata_ptr tmptr, replication_strategy_config_options opts = {}) { dc_rack_fn get_dc_rack_fn = get_dc_rack; tmptr->update_topology_change_info(get_dc_rack_fn).get(); - auto strategy = seastar::make_shared(replication_strategy_params(opts)); + auto strategy = seastar::make_shared(replication_strategy_params(opts, std::nullopt)); return calculate_effective_replication_map(std::move(strategy), tmptr).get0(); } }