From dcd520f6cfd9b77d66b071ff2fc3ecc5f29a1bdf Mon Sep 17 00:00:00 2001 From: Piotr Dulikowski Date: Wed, 12 Apr 2023 11:53:39 +0200 Subject: [PATCH 1/8] db/system_keyspace: add storage for cluster features managed in group 0 The `system.topology` table is extended with two new columns that will be used to manage cluster features: - `supported_features set` is a new clustering column which holds the features that given node advertises as supported. It will be first initialized when the node joins the cluster, and then updated every time the node reboots and its supported features set changes. - `enabled_features set` is a new static column which holds the features that are considered enabled by the cluster. Unlike in the current gossip-based implementation the features will not be enabled implicitly when all nodes support a feature, but rather when via an explicit action of the topology coordinator. --- db/system_keyspace.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/db/system_keyspace.cc b/db/system_keyspace.cc index 67b6ba5446..433a3ecf76 100644 --- a/db/system_keyspace.cc +++ b/db/system_keyspace.cc @@ -256,11 +256,13 @@ schema_ptr system_keyspace::topology() { .with_column("num_tokens", int32_type) .with_column("shard_count", int32_type) .with_column("ignore_msb", int32_type) + .with_column("supported_features", set_type_impl::get_instance(utf8_type, true)) .with_column("new_cdc_generation_data_uuid", uuid_type, column_kind::static_column) .with_column("transition_state", utf8_type, column_kind::static_column) .with_column("current_cdc_generation_uuid", uuid_type, column_kind::static_column) .with_column("current_cdc_generation_timestamp", timestamp_type, column_kind::static_column) .with_column("global_topology_request", utf8_type, column_kind::static_column) + .with_column("enabled_features", set_type_impl::get_instance(utf8_type, true), column_kind::static_column) .set_comment("Current state of topology change machine") .with_version(generate_schema_version(id)) .build(); From e527e63abce123ec6a0b09bf3cf78b11cc353787 Mon Sep 17 00:00:00 2001 From: Piotr Dulikowski Date: Fri, 16 Jun 2023 16:38:34 +0200 Subject: [PATCH 2/8] system_keyspace: introduce deserialize_set_column There are three places in system_keyspace.cc which deserialize a column holding a set of tokens and convert it to an unordered set of dht::token. The deserialization process involves a small number of steps that are the same in all of those places, therefore they can be abstracted away. This commit adds `deserialize_set_column` function which takes care of deserializing the column to `set_type_impl::native_type` which can be then passed to `decode_tokens`. The new function will also be useful for decoding set columns with cluster features, which will be handled in the next commit. --- db/system_keyspace.cc | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/db/system_keyspace.cc b/db/system_keyspace.cc index 433a3ecf76..b9f20b886d 100644 --- a/db/system_keyspace.cc +++ b/db/system_keyspace.cc @@ -1555,6 +1555,13 @@ future system_keyspace::get_truncated_at(table_id cf_id) { }); } +static set_type_impl::native_type deserialize_set_column(const schema& s, const cql3::untyped_result_set_row& row, const char* name) { + auto blob = row.get_blob(name); + auto cdef = s.get_column_definition(name); + auto deserialized = cdef->type->deserialize(blob); + return value_cast(deserialized); +} + static set_type_impl::native_type prepare_tokens(const std::unordered_set& tokens) { set_type_impl::native_type tset; for (auto& t: tokens) { @@ -1563,7 +1570,7 @@ static set_type_impl::native_type prepare_tokens(const std::unordered_set decode_tokens(set_type_impl::native_type& tokens) { +std::unordered_set decode_tokens(const set_type_impl::native_type& tokens) { std::unordered_set tset; for (auto& t: tokens) { auto str = value_cast(t); @@ -1594,11 +1601,7 @@ future>> sy for (auto& row : *cql_result) { auto peer = gms::inet_address(row.get_as("peer")); if (row.has("tokens")) { - auto blob = row.get_blob("tokens"); - auto cdef = peers()->get_column_definition("tokens"); - auto deserialized = cdef->type->deserialize(blob); - auto tokens = value_cast(deserialized); - ret.emplace(peer, decode_tokens(tokens)); + ret.emplace(peer, decode_tokens(deserialize_set_column(*peers(), row, "tokens"))); } } return ret; @@ -1792,12 +1795,8 @@ future> system_keyspace::get_saved_tokens() { return make_ready_future>(); } - auto blob = msg->one().get_blob("tokens"); - auto cdef = local()->get_column_definition("tokens"); - auto deserialized = cdef->type->deserialize(blob); - auto tokens = value_cast(deserialized); - - return make_ready_future>(decode_tokens(tokens)); + auto decoded_tokens = decode_tokens(deserialize_set_column(*local(), msg->one(), "tokens")); + return make_ready_future>(std::move(decoded_tokens)); }); } @@ -3495,11 +3494,7 @@ future system_keyspace::load_topology_state() { std::optional ring_slice; if (row.has("tokens")) { - auto blob = row.get_blob("tokens"); - auto cdef = topology()->get_column_definition("tokens"); - auto deserialized = cdef->type->deserialize(blob); - auto ts = value_cast(deserialized); - auto tokens = decode_tokens(ts); + auto tokens = decode_tokens(deserialize_set_column(*topology(), row, "tokens")); if (tokens.empty()) { on_fatal_internal_error(slogger, format( From bc84d59665420d380b8af0f6eb138f3223e4c124 Mon Sep 17 00:00:00 2001 From: Piotr Dulikowski Date: Thu, 27 Apr 2023 16:22:20 +0200 Subject: [PATCH 3/8] topology_state_machine: include information about features Now, the newly added `supported_features` and `enabled_features` columns are reflected in the `topology_state_machine` structure. --- db/system_keyspace.cc | 19 ++++++++++++++++++- service/topology_state_machine.hh | 4 ++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/db/system_keyspace.cc b/db/system_keyspace.cc index b9f20b886d..54b47c9f7e 100644 --- a/db/system_keyspace.cc +++ b/db/system_keyspace.cc @@ -3470,6 +3470,14 @@ future<> system_keyspace::set_must_synchronize_topology(bool value) { return set_scylla_local_param_as(MUST_SYNCHRONIZE_TOPOLOGY_KEY, value); } +static std::set decode_features(const set_type_impl::native_type& features) { + std::set fset; + for (auto& f : features) { + fset.insert(value_cast(std::move(f))); + } + return fset; +} + future system_keyspace::load_topology_state() { auto rs = co_await qctx->execute_cql( format("SELECT * FROM system.{} WHERE key = '{}'", TOPOLOGY, TOPOLOGY)); @@ -3517,6 +3525,11 @@ future system_keyspace::load_topology_state() { rebuild_option = row.get_as("rebuild_option"); } + std::set supported_features; + if (row.has("supported_features")) { + supported_features = decode_features(deserialize_set_column(*topology(), row, "supported_features")); + } + if (row.has("topology_request")) { auto req = service::topology_request_from_string(row.get_as("topology_request")); ret.requests.emplace(host_id, req); @@ -3583,7 +3596,7 @@ future system_keyspace::load_topology_state() { if (map) { map->emplace(host_id, service::replica_state{ nstate, std::move(datacenter), std::move(rack), std::move(release_version), - ring_slice, shard_count, ignore_msb}); + ring_slice, shard_count, ignore_msb, std::move(supported_features)}); } } @@ -3646,6 +3659,10 @@ future system_keyspace::load_topology_state() { some_row.get_as("global_topology_request")); ret.global_request.emplace(req); } + + if (some_row.has("enabled_features")) { + ret.enabled_features = decode_features(deserialize_set_column(*topology(), some_row, "enabled_features")); + } } co_return ret; diff --git a/service/topology_state_machine.hh b/service/topology_state_machine.hh index 86c1880f63..afc3d1f9f3 100644 --- a/service/topology_state_machine.hh +++ b/service/topology_state_machine.hh @@ -63,6 +63,7 @@ struct replica_state { std::optional ring; // if engaged contain the set of tokens the node owns together with their state size_t shard_count; uint8_t ignore_msb; + std::set supported_features; }; struct topology { @@ -103,6 +104,9 @@ struct topology { // It's used as partition key in CDC_GENERATIONS_V3 table. std::optional new_cdc_generation_data_uuid; + // Features that are considered enabled by the cluster + std::set enabled_features; + // Find only nodes in non 'left' state const std::pair* find(raft::server_id id) const; // Return true if node exists in any state including 'left' one From ee12192125d3fab1d5482119549830457f7b4039 Mon Sep 17 00:00:00 2001 From: Piotr Dulikowski Date: Fri, 16 Jun 2023 13:15:25 +0200 Subject: [PATCH 4/8] storage_service: introduce topology_mutation_builder_base Introduces `topology_mutation_builder_base` which will be a base class for both topology mutation builder and topology node mutation builder. Its purpose is to abstract away some detail about setting/deleting/etc. column in the mutation, the actual topology (node) mutation builder will only have to care about converting types and/or allowing only particular columns to be set. The class is using CRTP: derived classes provide access to the row being modified, schema and the timestamp. For the sake of commit diff readability, this commt only introduces this class and changes the builders to derive from it but no setter implementations are modified - this will be done in the next commit. --- service/storage_service.cc | 117 ++++++++++++++++++++++++++++++++++++- 1 file changed, 115 insertions(+), 2 deletions(-) diff --git a/service/storage_service.cc b/service/storage_service.cc index 36b2704ec7..3fc13fcddf 100644 --- a/service/storage_service.cc +++ b/service/storage_service.cc @@ -499,12 +499,43 @@ future<> storage_service::merge_topology_snapshot(raft_topology_snapshot snp) { co_await _db.local().apply(freeze(muts), db::no_timeout); } +template +class topology_mutation_builder_base { +private: + Builder& self() { + return *static_cast(this); + } + +protected: + enum class collection_apply_mode { + overwrite, + update, + }; + + using builder_base = topology_mutation_builder_base; + + Builder& apply_atomic(const char* cell, const data_value& value); + template + requires std::convertible_to, data_value> + Builder& apply_set(const char* cell, collection_apply_mode apply_mode, const C& c); + Builder& del(const char* cell); +}; + class topology_mutation_builder; -class topology_node_mutation_builder { +class topology_node_mutation_builder + : public topology_mutation_builder_base { + + friend builder_base; + topology_mutation_builder& _builder; deletable_row& _r; +private: + row& row(); + api::timestamp_type timestamp() const; + const schema& schema() const; + public: topology_node_mutation_builder(topology_mutation_builder&, raft::server_id); @@ -521,7 +552,10 @@ public: canonical_mutation build(); }; -class topology_mutation_builder { +class topology_mutation_builder + : public topology_mutation_builder_base { + + friend builder_base; friend class topology_node_mutation_builder; schema_ptr _s; @@ -529,6 +563,12 @@ class topology_mutation_builder { api::timestamp_type _ts; std::optional _node_builder; + +private: + row& row(); + api::timestamp_type timestamp() const; + const schema& schema() const; + public: topology_mutation_builder(api::timestamp_type ts); topology_mutation_builder& set_transition_state(topology::transition_state); @@ -553,6 +593,67 @@ topology_node_mutation_builder::topology_node_mutation_builder(topology_mutation _r.apply(row_marker(_builder._ts)); } +template +Builder& topology_mutation_builder_base::apply_atomic(const char* cell, const data_value& value) { + const column_definition* cdef = self().schema().get_column_definition(cell); + assert(cdef); + self().row().apply(*cdef, atomic_cell::make_live(*cdef->type, self().timestamp(), cdef->type->decompose(value))); + return self(); +} + +template +template +requires std::convertible_to, data_value> +Builder& topology_mutation_builder_base::apply_set(const char* cell, collection_apply_mode apply_mode, const C& c) { + const column_definition* cdef = self().schema().get_column_definition(cell); + assert(cdef); + auto vtype = static_pointer_cast(cdef->type)->get_elements_type(); + + std::set cset(vtype->as_less_comparator()); + for (const auto& v : c) { + cset.insert(vtype->decompose(data_value(v))); + } + + collection_mutation_description cm; + cm.cells.reserve(cset.size()); + for (const bytes& raw : cset) { + cm.cells.emplace_back(raw, atomic_cell::make_live(*bytes_type, self().timestamp(), bytes_view())); + } + + if (apply_mode == collection_apply_mode::overwrite) { + cm.tomb = tombstone(self().timestamp() - 1, gc_clock::now()); + } + + self().row().apply(*cdef, cm.serialize(*cdef->type)); + return self(); +} + +template +Builder& topology_mutation_builder_base::del(const char* cell) { + auto cdef = self().schema().get_column_definition(cell); + assert(cdef); + if (!cdef->type->is_multi_cell()) { + self().row().apply(*cdef, atomic_cell::make_dead(self().timestamp(), gc_clock::now())); + } else { + collection_mutation_description cm; + cm.tomb = tombstone{self().timestamp(), gc_clock::now()}; + self().row().apply(*cdef, cm.serialize(*cdef->type)); + } + return self(); +} + +row& topology_node_mutation_builder::row() { + return _r.cells(); +} + +api::timestamp_type topology_node_mutation_builder::timestamp() const { + return _builder._ts; +} + +const schema& topology_node_mutation_builder::schema() const { + return *_builder._s; +} + topology_node_mutation_builder& topology_node_mutation_builder::set(const char* cell, const sstring& value) { auto cdef = _builder._s->get_column_definition(cell); assert(cdef); @@ -620,6 +721,18 @@ canonical_mutation topology_node_mutation_builder::build() { return canonical_mutation{std::move(_builder._m)}; } +row& topology_mutation_builder::row() { + return _m.partition().static_row().maybe_create(); +} + +api::timestamp_type topology_mutation_builder::timestamp() const { + return _ts; +} + +const schema& topology_mutation_builder::schema() const { + return *_s; +} + topology_mutation_builder& topology_mutation_builder::set_transition_state(topology::transition_state value) { _m.set_static_cell("transition_state", ::format("{}", value), _ts); return *this; From a8aaeabfaccef15dcea01c637cc85c72ef57e47e Mon Sep 17 00:00:00 2001 From: Piotr Dulikowski Date: Fri, 16 Jun 2023 13:17:43 +0200 Subject: [PATCH 5/8] storage_service: reimplement mutation builder setters As promised in the previous commit which introduced topology_mutation_builder_base, this commit adjusts existing setters of topology mutation builder and topology node mutation builder to use helper methods defined in the base class. Note that the `::set` method for the unordered set of tokens now does not delete the column in case an empty value is set, instead it just writes an empty set. This semantic is arguably more clear given that we have an explicit `::del` method and it shouldn't affect the existing implementation - we never intentionally insert an empty set of tokens. --- service/storage_service.cc | 73 +++++++------------------------------- 1 file changed, 13 insertions(+), 60 deletions(-) diff --git a/service/storage_service.cc b/service/storage_service.cc index 3fc13fcddf..94b9a8e845 100644 --- a/service/storage_service.cc +++ b/service/storage_service.cc @@ -655,66 +655,28 @@ const schema& topology_node_mutation_builder::schema() const { } topology_node_mutation_builder& topology_node_mutation_builder::set(const char* cell, const sstring& value) { - auto cdef = _builder._s->get_column_definition(cell); - assert(cdef); - _r.cells().apply(*cdef, atomic_cell::make_live(*cdef->type, _builder._ts, cdef->type->decompose(value))); - return *this; + return apply_atomic(cell, value); } topology_node_mutation_builder& topology_node_mutation_builder::set(const char* cell, const raft::server_id& value) { - auto cdef = _builder._s->get_column_definition(cell); - assert(cdef); - _r.cells().apply(*cdef, atomic_cell::make_live(*cdef->type, _builder._ts, cdef->type->decompose(value.uuid()))); - return *this; + return apply_atomic(cell, value.uuid()); } topology_node_mutation_builder& topology_node_mutation_builder::set(const char* cell, const uint32_t& value) { - auto cdef = _builder._s->get_column_definition(cell); - assert(cdef); - _r.cells().apply(*cdef, atomic_cell::make_live(*cdef->type, _builder._ts, cdef->type->decompose(int32_t(value)))); - return *this; + return apply_atomic(cell, int32_t(value)); } topology_node_mutation_builder& topology_node_mutation_builder::set( const char* cell, const utils::UUID& value) { - auto cdef = _builder._s->get_column_definition(cell); - assert(cdef); - _r.cells().apply(*cdef, atomic_cell::make_live(*cdef->type, _builder._ts, cdef->type->decompose(value))); - return *this; + return apply_atomic(cell, value); } topology_node_mutation_builder& topology_node_mutation_builder::del(const char* cell) { - auto cdef = _builder._s->get_column_definition(cell); - assert(cdef); - if (!cdef->type->is_multi_cell()) { - _r.cells().apply(*cdef, atomic_cell::make_dead(_builder._ts, gc_clock::now())); - } else { - collection_mutation_description cm; - cm.tomb = tombstone{_builder._ts, gc_clock::now()}; - _r.cells().apply(*cdef, cm.serialize(*cdef->type)); - } - return *this; + return builder_base::del(cell); } topology_node_mutation_builder& topology_node_mutation_builder::set(const char* cell, const std::unordered_set& tokens) { - auto cdef = _builder._s->get_column_definition(cell); - assert(cdef); - collection_mutation_description cm; - if (tokens.size()) { - auto vtype = static_pointer_cast(cdef->type)->get_elements_type(); - - cm.cells.reserve(tokens.size()); - - for (auto&& value : tokens) { - cm.cells.emplace_back(vtype->decompose(value.to_sstring()), atomic_cell::make_live(*bytes_type, _builder._ts, bytes_view())); - } - - cm.tomb = tombstone(_builder._ts - 1, gc_clock::now()); - _r.cells().apply(*cdef, cm.serialize(*cdef->type)); - } else { - del(cell); - } - return *this; + return apply_set(cell, collection_apply_mode::overwrite, tokens | boost::adaptors::transformed([] (const auto& t) { return t.to_sstring(); })); } canonical_mutation topology_node_mutation_builder::build() { @@ -734,40 +696,31 @@ const schema& topology_mutation_builder::schema() const { } topology_mutation_builder& topology_mutation_builder::set_transition_state(topology::transition_state value) { - _m.set_static_cell("transition_state", ::format("{}", value), _ts); - return *this; + return apply_atomic("transition_state", ::format("{}", value)); } topology_mutation_builder& topology_mutation_builder::del_transition_state() { - auto cdef = _s->get_column_definition("transition_state"); - assert(cdef); - _m.partition().static_row().apply(*cdef, atomic_cell::make_dead(_ts, gc_clock::now())); - return *this; + return del("transition_state"); } topology_mutation_builder& topology_mutation_builder::set_current_cdc_generation_id( const cdc::generation_id_v2& value) { - _m.set_static_cell("current_cdc_generation_timestamp", value.ts, _ts); - _m.set_static_cell("current_cdc_generation_uuid", value.id, _ts); + apply_atomic("current_cdc_generation_timestamp", value.ts); + apply_atomic("current_cdc_generation_uuid", value.id); return *this; } topology_mutation_builder& topology_mutation_builder::set_new_cdc_generation_data_uuid( const utils::UUID& value) { - _m.set_static_cell("new_cdc_generation_data_uuid", value, _ts); - return *this; + return apply_atomic("new_cdc_generation_data_uuid", value); } topology_mutation_builder& topology_mutation_builder::set_global_topology_request(global_topology_request value) { - _m.set_static_cell("global_topology_request", ::format("{}", value), _ts); - return *this; + return apply_atomic("global_topology_request", ::format("{}", value)); } topology_mutation_builder& topology_mutation_builder::del_global_topology_request() { - auto cdef = _s->get_column_definition("global_topology_request"); - assert(cdef); - _m.partition().static_row().apply(*cdef, atomic_cell::make_dead(_ts, gc_clock::now())); - return *this; + return del("global_topology_request"); } topology_node_mutation_builder& topology_mutation_builder::with_node(raft::server_id n) { From 2a4462a01f5d8ec5ba1650caa7ba86774b847c01 Mon Sep 17 00:00:00 2001 From: Piotr Dulikowski Date: Fri, 16 Jun 2023 18:19:30 +0200 Subject: [PATCH 6/8] storage_service: use explicit ::set overload instead of a template The `topology_node_mutation_builder::set` function has an overload which accepts any type which can be converted to string via `::format`. Its presence can lead to easy mistakes which can only be detected at runtime rather at compile time. A concrete example: I wrote a function that accepts an std::set where S is convertible to sstring; it turns out that std::string_view is not std::convertible_to sstring and overload resolution falled back to the catch-all overload. This commit gets rid of the catch-all overload and replaces it with explicit ones. Fortunately, it was used for only two enums, so it wasn't much work. --- service/storage_service.cc | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/service/storage_service.cc b/service/storage_service.cc index 94b9a8e845..704bfec170 100644 --- a/service/storage_service.cc +++ b/service/storage_service.cc @@ -539,10 +539,8 @@ private: public: topology_node_mutation_builder(topology_mutation_builder&, raft::server_id); - template - topology_node_mutation_builder& set(const char* cell, const T& value) { - return set(cell, sstring{::format("{}", value)}); - } + topology_node_mutation_builder& set(const char* cell, node_state value); + topology_node_mutation_builder& set(const char* cell, topology_request value); topology_node_mutation_builder& set(const char* cell, const sstring& value); topology_node_mutation_builder& set(const char* cell, const raft::server_id& value); topology_node_mutation_builder& set(const char* cell, const std::unordered_set& value); @@ -654,6 +652,14 @@ const schema& topology_node_mutation_builder::schema() const { return *_builder._s; } +topology_node_mutation_builder& topology_node_mutation_builder::set(const char* cell, node_state value) { + return apply_atomic(cell, sstring{::format("{}", value)}); +} + +topology_node_mutation_builder& topology_node_mutation_builder::set(const char* cell, topology_request value) { + return apply_atomic(cell, sstring{::format("{}", value)}); +} + topology_node_mutation_builder& topology_node_mutation_builder::set(const char* cell, const sstring& value) { return apply_atomic(cell, value); } From 707e929831e57c1e282ba1e9876f558ed9deed6a Mon Sep 17 00:00:00 2001 From: Piotr Dulikowski Date: Tue, 13 Jun 2023 15:09:22 +0200 Subject: [PATCH 7/8] storage_service: add methods for features to topology mutation builder The newly added `supported_features` and `enabled_features` columns can now be modified via topology mutation builders: - `supported_features` can now be overwritten via a new overload of `topology_node_mutation_builder::set`. - `enabled_features` can now be extended (i.e. more elements can be added to it) via `topology_mutation_builder::add_enabled_features`. As the set of enabled features only grows, this should be sufficient. --- service/storage_service.cc | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/service/storage_service.cc b/service/storage_service.cc index 704bfec170..da7ff74314 100644 --- a/service/storage_service.cc +++ b/service/storage_service.cc @@ -544,6 +544,9 @@ public: topology_node_mutation_builder& set(const char* cell, const sstring& value); topology_node_mutation_builder& set(const char* cell, const raft::server_id& value); topology_node_mutation_builder& set(const char* cell, const std::unordered_set& value); + template + requires std::constructible_from + topology_node_mutation_builder& set(const char* cell, const std::set& value); topology_node_mutation_builder& set(const char* cell, const uint32_t& value); topology_node_mutation_builder& set(const char* cell, const utils::UUID& value); topology_node_mutation_builder& del(const char* cell); @@ -573,6 +576,9 @@ public: topology_mutation_builder& set_current_cdc_generation_id(const cdc::generation_id_v2&); topology_mutation_builder& set_new_cdc_generation_data_uuid(const utils::UUID& value); topology_mutation_builder& set_global_topology_request(global_topology_request); + template + requires std::constructible_from + topology_mutation_builder& add_enabled_features(const std::set& value); topology_mutation_builder& del_transition_state(); topology_mutation_builder& del_global_topology_request(); topology_node_mutation_builder& with_node(raft::server_id); @@ -685,6 +691,12 @@ topology_node_mutation_builder& topology_node_mutation_builder::set(const char* return apply_set(cell, collection_apply_mode::overwrite, tokens | boost::adaptors::transformed([] (const auto& t) { return t.to_sstring(); })); } +template +requires std::constructible_from +topology_node_mutation_builder& topology_node_mutation_builder::set(const char* cell, const std::set& features) { + return apply_set(cell, collection_apply_mode::overwrite, features | boost::adaptors::transformed([] (const auto& f) { return sstring(f); })); +} + canonical_mutation topology_node_mutation_builder::build() { return canonical_mutation{std::move(_builder._m)}; } @@ -725,6 +737,12 @@ topology_mutation_builder& topology_mutation_builder::set_global_topology_reques return apply_atomic("global_topology_request", ::format("{}", value)); } +template +requires std::constructible_from +topology_mutation_builder& topology_mutation_builder::add_enabled_features(const std::set& features) { + return apply_set("enabled_features", collection_apply_mode::update, features | boost::adaptors::transformed([] (const auto& f) { return sstring(f); })); +} + topology_mutation_builder& topology_mutation_builder::del_global_topology_request() { return del("global_topology_request"); } From 3e955945de459461bb1fbac0515800b4c25660af Mon Sep 17 00:00:00 2001 From: Piotr Dulikowski Date: Tue, 13 Jun 2023 15:10:05 +0200 Subject: [PATCH 8/8] storage_service: update supported cluster features in group0 on start Now, when a node starts, it will update its `supported_features` row in `system.topology` via `update_topology_with_local_metadata`. At this point, the functionality behind cluster features on raft is mostly incomplete and the state of the `supported_features` column does not influence anything so it's safe to update this column unconditionally. In the future, the node will only join / start group0 server if it is sure that it supports all enabled features and it can safely update the `supported_features` parameter. --- service/storage_service.cc | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/service/storage_service.cc b/service/storage_service.cc index da7ff74314..fc433ddc8b 100644 --- a/service/storage_service.cc +++ b/service/storage_service.cc @@ -1530,7 +1530,8 @@ future<> storage_service::raft_replace(raft::server& raft_server, raft::server_i .set("replaced_id", replaced_id) .set("num_tokens", _db.local().get_config().num_tokens()) .set("shard_count", smp::count) - .set("ignore_msb", _db.local().get_config().murmur3_partitioner_ignore_msb_bits()); + .set("ignore_msb", _db.local().get_config().murmur3_partitioner_ignore_msb_bits()) + .set("supported_features", _feature_service.supported_feature_set()); topology_change change{{builder.build()}}; group0_command g0_cmd = _group0->client().prepare_command(std::move(change), guard, ::format("replace {}/{}: add myself ({}) to topology", replaced_id, replaced_ip, raft_server.id())); try { @@ -1566,7 +1567,8 @@ future<> storage_service::raft_bootstrap(raft::server& raft_server) { .set("topology_request", topology_request::join) .set("num_tokens", _db.local().get_config().num_tokens()) .set("shard_count", smp::count) - .set("ignore_msb", _db.local().get_config().murmur3_partitioner_ignore_msb_bits()); + .set("ignore_msb", _db.local().get_config().murmur3_partitioner_ignore_msb_bits()) + .set("supported_features", _feature_service.supported_feature_set()); topology_change change{{builder.build()}}; group0_command g0_cmd = _group0->client().prepare_command(std::move(change), guard, "bootstrap: add myself to topology"); try { @@ -1582,6 +1584,7 @@ future<> storage_service::update_topology_with_local_metadata(raft::server& raft auto local_shard_count = smp::count; auto local_ignore_msb = _db.local().get_config().murmur3_partitioner_ignore_msb_bits(); auto local_release_version = version::release(); + auto local_supported_features = boost::copy_range>(_feature_service.supported_feature_set()); auto synchronized = [&] () { auto it = _topology_state_machine._topology.find(raft_server.id()); @@ -1593,7 +1596,8 @@ future<> storage_service::update_topology_with_local_metadata(raft::server& raft return replica_state.shard_count == local_shard_count && replica_state.ignore_msb == local_ignore_msb - && replica_state.release_version == local_release_version; + && replica_state.release_version == local_release_version + && replica_state.supported_features == local_supported_features; }; // We avoid performing a read barrier if we're sure that our metadata stored in topology @@ -1629,7 +1633,8 @@ future<> storage_service::update_topology_with_local_metadata(raft::server& raft builder.with_node(raft_server.id()) .set("shard_count", local_shard_count) .set("ignore_msb", local_ignore_msb) - .set("release_version", local_release_version); + .set("release_version", local_release_version) + .set("supported_features", local_supported_features); topology_change change{{builder.build()}}; group0_command g0_cmd = _group0->client().prepare_command( std::move(change), guard, ::format("{}: update topology with local metadata", raft_server.id()));