From 3ef201d67a7521fc82b42e2ba992a16cc337ca5d Mon Sep 17 00:00:00 2001 From: Petr Gusev Date: Tue, 14 Mar 2023 19:26:05 +0400 Subject: [PATCH] schema.hh: use schema_static_props for wait_for_sync_to_commitlog This patch continues the refactoring, now we move wait_for_sync_to_commitlog property from schema_builder to schema_static_props. The patch replaces schema_builder::set_wait_for_sync_to_commitlog and is_extra_durable with two register_static_configurator, one in system_keyspace and another in system_distributed_keyspace. They correspond to the two parts of the original disjunction in schema_tables::is_extra_durable. --- db/schema_tables.cc | 10 ---------- db/system_distributed_keyspace.cc | 13 +++++++++---- db/system_distributed_keyspace.hh | 4 ---- db/system_keyspace.cc | 26 ++++++++++++++------------ db/system_keyspace.hh | 4 ---- schema/schema.hh | 6 ++---- schema/schema_builder.hh | 4 ---- 7 files changed, 25 insertions(+), 42 deletions(-) diff --git a/db/schema_tables.cc b/db/schema_tables.cc index 4aed185742..3e8c26f5ad 100644 --- a/db/schema_tables.cc +++ b/db/schema_tables.cc @@ -83,12 +83,6 @@ using namespace std::chrono_literals; static logging::logger diff_logger("schema_diff"); -static bool is_extra_durable(const sstring& ks_name, const sstring& cf_name) { - return (is_system_keyspace(ks_name) && db::system_keyspace::is_extra_durable(cf_name)) - || ((ks_name == db::system_distributed_keyspace::NAME || ks_name == db::system_distributed_keyspace::NAME_EVERYWHERE) - && db::system_distributed_keyspace::is_extra_durable(cf_name)); -} - /** system.schema_* tables used to store keyspace/table/type attributes prior to C* 3.0 */ namespace db { @@ -3093,10 +3087,6 @@ schema_ptr create_table_from_mutations(const schema_ctxt& ctxt, schema_mutations builder.with_sharder(smp::count, ctxt.murmur3_partitioner_ignore_msb_bits()); } - if (is_extra_durable(ks_name, cf_name)) { - builder.set_wait_for_sync_to_commitlog(true); - } - return builder.build(); } diff --git a/db/system_distributed_keyspace.cc b/db/system_distributed_keyspace.cc index 17f0c3e306..874db9a9d0 100644 --- a/db/system_distributed_keyspace.cc +++ b/db/system_distributed_keyspace.cc @@ -40,6 +40,15 @@ static logging::logger dlogger("system_distributed_keyspace"); extern logging::logger cdc_log; namespace db { +namespace { + const auto set_wait_for_sync_to_commitlog = schema_builder::register_static_configurator([](const sstring& ks_name, const sstring& cf_name, schema_static_props& props) { + if ((ks_name == system_distributed_keyspace::NAME_EVERYWHERE && cf_name == system_distributed_keyspace::CDC_GENERATIONS_V2) || + (ks_name == system_distributed_keyspace::NAME && cf_name == system_distributed_keyspace::CDC_TOPOLOGY_DESCRIPTION)) + { + props.wait_for_sync_to_commitlog = true; + } + }); +} thread_local data_type cdc_streams_set_type = set_type_impl::get_instance(bytes_type, false); @@ -185,10 +194,6 @@ static void check_exists(std::string_view ks_name, std::string_view cf_name, con } } -bool system_distributed_keyspace::is_extra_durable(const sstring& cf_name) { - return cf_name == CDC_TOPOLOGY_DESCRIPTION || cf_name == CDC_GENERATIONS_V2; -} - std::vector system_distributed_keyspace::all_distributed_tables() { return {view_build_status(), cdc_desc(), cdc_timestamps(), service_levels()}; } diff --git a/db/system_distributed_keyspace.hh b/db/system_distributed_keyspace.hh index 1590bdd63f..bac22acbed 100644 --- a/db/system_distributed_keyspace.hh +++ b/db/system_distributed_keyspace.hh @@ -77,10 +77,6 @@ private: bool _forced_cdc_timestamps_schema_sync = false; public: - /* Should writes to the given table always be synchronized by commitlog (flushed to disk) - * before being acknowledged? */ - static bool is_extra_durable(const sstring& cf_name); - static std::vector all_distributed_tables(); static std::vector all_everywhere_tables(); diff --git a/db/system_keyspace.cc b/db/system_keyspace.cc index cc6c1d61d0..bdf94ad088 100644 --- a/db/system_keyspace.cc +++ b/db/system_keyspace.cc @@ -83,6 +83,20 @@ namespace { props.use_null_sharder = true; } }); + const auto set_wait_for_sync_to_commitlog = schema_builder::register_static_configurator([](const sstring& ks_name, const sstring& cf_name, schema_static_props& props) { + static const std::unordered_set extra_durable_tables = { + system_keyspace::PAXOS, + system_keyspace::SCYLLA_LOCAL, + system_keyspace::RAFT, + system_keyspace::RAFT_SNAPSHOTS, + system_keyspace::RAFT_SNAPSHOT_CONFIG, + system_keyspace::DISCOVERY, + system_keyspace::BROADCAST_KV_STORE + }; + if (ks_name == system_keyspace::NAME && extra_durable_tables.contains(cf_name)) { + props.wait_for_sync_to_commitlog = true; + } + }); } std::unique_ptr qctx = {}; @@ -90,10 +104,6 @@ std::unique_ptr qctx = {}; static logging::logger slogger("system_keyspace"); static const api::timestamp_type creation_timestamp = api::new_timestamp(); -bool system_keyspace::is_extra_durable(const sstring& name) { - return boost::algorithm::any_of(extra_durable_tables, [name] (const char* table) { return name == table; }); -} - api::timestamp_type system_keyspace::schema_creation_timestamp() { return creation_timestamp; } @@ -198,7 +208,6 @@ schema_ptr system_keyspace::batchlog() { ); builder.set_gc_grace_seconds(0); builder.with_version(generate_schema_version(builder.uuid())); - builder.set_wait_for_sync_to_commitlog(true); return builder.build(schema_builder::compact_storage::no); }(); return paxos; @@ -222,7 +231,6 @@ schema_ptr system_keyspace::raft() { .set_comment("Persisted RAFT log, votes and snapshot info") .with_version(generate_schema_version(id)) - .set_wait_for_sync_to_commitlog(true) .build(); }(); return schema; @@ -243,7 +251,6 @@ schema_ptr system_keyspace::raft_snapshots() { .set_comment("Persisted RAFT snapshot descriptors info") .with_version(generate_schema_version(id)) - .set_wait_for_sync_to_commitlog(true) .build(); }(); return schema; @@ -260,7 +267,6 @@ schema_ptr system_keyspace::raft_snapshot_config() { .set_comment("RAFT configuration for the latest snapshot descriptor") .with_version(generate_schema_version(id)) - .set_wait_for_sync_to_commitlog(true) .build(); }(); return schema; @@ -654,8 +660,6 @@ schema_ptr system_keyspace::large_cells() { "Scylla specific information about the local node" ); builder.set_gc_grace_seconds(0); - // Raft Group id and server id updates must be sync - builder.set_wait_for_sync_to_commitlog(true); builder.with_version(generate_schema_version(builder.uuid())); return builder.build(schema_builder::compact_storage::no); }(); @@ -956,7 +960,6 @@ schema_ptr system_keyspace::discovery() { .with_column("raft_server_id", uuid_type) .set_comment("State of cluster discovery algorithm: the set of discovered peers") .with_version(generate_schema_version(id)) - .set_wait_for_sync_to_commitlog(true) .build(); }(); return schema; @@ -969,7 +972,6 @@ schema_ptr system_keyspace::broadcast_kv_store() { .with_column("key", utf8_type, column_kind::partition_key) .with_column("value", utf8_type) .with_version(generate_schema_version(id)) - .set_wait_for_sync_to_commitlog(true) .build(); }(); return schema; diff --git a/db/system_keyspace.hh b/db/system_keyspace.hh index 024fa07b28..c32b213fca 100644 --- a/db/system_keyspace.hh +++ b/db/system_keyspace.hh @@ -211,10 +211,6 @@ public: static schema_ptr batchlog(); }; - static constexpr const char* extra_durable_tables[] = { PAXOS, SCYLLA_LOCAL, RAFT, RAFT_SNAPSHOTS, RAFT_SNAPSHOT_CONFIG, DISCOVERY, BROADCAST_KV_STORE }; - - static bool is_extra_durable(const sstring& name); - // Partition estimates for a given range of tokens. struct range_estimates { schema_ptr schema; diff --git a/schema/schema.hh b/schema/schema.hh index e32101f876..d0e5eaa915 100644 --- a/schema/schema.hh +++ b/schema/schema.hh @@ -564,6 +564,7 @@ public: struct schema_static_props { bool use_null_sharder = false; // use a sharder that puts everything on shard 0 + bool wait_for_sync_to_commitlog = false; // true if all writes using this schema have to be synced immediately by commitlog }; /* @@ -625,9 +626,6 @@ private: std::unordered_map _dropped_columns; std::map _collections; std::unordered_map _indices_by_name; - // The flag is not stored in the schema mutation and does not affects schema digest. - // It is set locally on a system tables that should be extra durable - bool _wait_for_sync = false; // true if all writes using this schema have to be synced immediately by commitlog std::reference_wrapper _partitioner; // Sharding info is not stored in the schema mutation and does not affect // schema digest. It is also not set locally on a schema tables. @@ -962,7 +960,7 @@ public: bool is_synced() const; bool equal_columns(const schema&) const; bool wait_for_sync_to_commitlog() const { - return _raw._wait_for_sync; + return _static_props.wait_for_sync_to_commitlog; } public: const v3_columns& v3() const { diff --git a/schema/schema_builder.hh b/schema/schema_builder.hh index 87cd88b8e0..36ef96494b 100644 --- a/schema/schema_builder.hh +++ b/schema/schema_builder.hh @@ -236,10 +236,6 @@ public: return *this; } - schema_builder& set_wait_for_sync_to_commitlog(bool sync) { - _raw._wait_for_sync = sync; - return *this; - } schema_builder& with_partitioner(sstring name); schema_builder& with_sharder(unsigned shard_count, unsigned sharding_ignore_msb_bits); class default_names {