From a48881801a74a8ff940d932b2f3f403db55bc603 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Botond=20D=C3=A9nes?= Date: Fri, 19 Jan 2024 09:29:05 -0500 Subject: [PATCH] replica/tablets: drop keyspace_name from system.tablets partition-key The name of the keyspace being part of the partition key is not useful, the table_id already uniquely identifies the table. The keyspace name being part of the key, means that code wanting to interact with this table, often has to resolve the table id, just to be able to provide the keyspace name. This is counter productive, so make the keyspace_name just a static column instead, just like table_name already is. Fixes: #16377 Closes scylladb/scylladb#16881 --- replica/tablet_mutation_builder.hh | 7 +++---- replica/tablets.cc | 15 +++++++-------- replica/tablets.hh | 2 +- service/storage_service.cc | 8 +++----- service/tablet_allocator.cc | 4 ++-- test/boost/tablets_test.cc | 6 +++--- test/topology_experimental_raft/test_tablets.py | 1 - 7 files changed, 19 insertions(+), 24 deletions(-) diff --git a/replica/tablet_mutation_builder.hh b/replica/tablet_mutation_builder.hh index b75b1d1c1e..12bdf25a87 100644 --- a/replica/tablet_mutation_builder.hh +++ b/replica/tablet_mutation_builder.hh @@ -24,13 +24,12 @@ private: return clustering_key::from_single_value(*_s, data_value(dht::token::to_int64(last_token)).serialize_nonnull()); } public: - tablet_mutation_builder(api::timestamp_type ts, const sstring& keyspace_name, table_id table) + tablet_mutation_builder(api::timestamp_type ts, table_id table) : _ts(ts) , _s(db::system_keyspace::tablets()) - , _m(_s, partition_key::from_exploded(*_s, { - data_value(keyspace_name).serialize_nonnull(), + , _m(_s, partition_key::from_single_value(*_s, data_value(table.uuid()).serialize_nonnull() - })) + )) { } tablet_mutation_builder& set_new_replicas(dht::token last_token, locator::tablet_replica_set replicas); diff --git a/replica/tablets.cc b/replica/tablets.cc index 6f69647798..a6d6872716 100644 --- a/replica/tablets.cc +++ b/replica/tablets.cc @@ -40,9 +40,9 @@ schema_ptr make_tablets_schema() { // replica_set_type = frozen> auto id = generate_legacy_id(db::system_keyspace::NAME, db::system_keyspace::TABLETS); return schema_builder(db::system_keyspace::NAME, db::system_keyspace::TABLETS, id) - .with_column("keyspace_name", utf8_type, column_kind::partition_key) .with_column("table_id", uuid_type, column_kind::partition_key) .with_column("tablet_count", int32_type, column_kind::static_column) + .with_column("keyspace_name", utf8_type, column_kind::static_column) .with_column("table_name", utf8_type, column_kind::static_column) .with_column("last_token", long_type, column_kind::clustering_key) .with_column("replicas", replica_set_type) @@ -72,12 +72,12 @@ tablet_map_to_mutation(const tablet_map& tablets, table_id id, const sstring& ke auto gc_now = gc_clock::now(); auto tombstone_ts = ts - 1; - mutation m(s, partition_key::from_exploded(*s, { - data_value(keyspace_name).serialize_nonnull(), + mutation m(s, partition_key::from_single_value(*s, data_value(id.uuid()).serialize_nonnull() - })); + )); m.partition().apply(tombstone(tombstone_ts, gc_now)); m.set_static_cell("tablet_count", data_value(int(tablets.tablet_count())), ts); + m.set_static_cell("keyspace_name", data_value(keyspace_name), ts); m.set_static_cell("table_name", data_value(table_name), ts); tablet_id tid = tablets.first_tablet(); @@ -141,12 +141,11 @@ tablet_mutation_builder::del_transition(dht::token last_token) { return *this; } -mutation make_drop_tablet_map_mutation(const sstring& keyspace_name, table_id id, api::timestamp_type ts) { +mutation make_drop_tablet_map_mutation(table_id id, api::timestamp_type ts) { auto s = db::system_keyspace::tablets(); - mutation m(s, partition_key::from_exploded(*s, { - data_value(keyspace_name).serialize_nonnull(), + mutation m(s, partition_key::from_single_value(*s, data_value(id.uuid()).serialize_nonnull() - })); + )); m.partition().apply(tombstone(ts, gc_clock::now())); return m; } diff --git a/replica/tablets.hh b/replica/tablets.hh index 576a5a8fb9..9eed468f1f 100644 --- a/replica/tablets.hh +++ b/replica/tablets.hh @@ -50,7 +50,7 @@ future tablet_map_to_mutation(const locator::tablet_map&, const sstring& table_name, api::timestamp_type); -mutation make_drop_tablet_map_mutation(const sstring& keyspace_name, table_id, api::timestamp_type); +mutation make_drop_tablet_map_mutation(table_id, api::timestamp_type); /// Stores a given tablet_metadata in system.tablets. /// diff --git a/service/storage_service.cc b/service/storage_service.cc index 6ccfd2eb91..c4241d27f1 100644 --- a/service/storage_service.cc +++ b/service/storage_service.cc @@ -2036,7 +2036,6 @@ class topology_coordinator : public endpoint_lifecycle_subscriber { } void generate_migration_update(std::vector& out, const group0_guard& guard, const tablet_migration_info& mig) { - auto s = _db.find_schema(mig.tablet.table); auto& tmap = get_token_metadata_ptr()->tablets().get_tablet_map(mig.tablet.table); auto last_token = tmap.get_last_token(mig.tablet.tablet); if (tmap.get_tablet_transition_info(mig.tablet.tablet)) { @@ -2044,7 +2043,7 @@ class topology_coordinator : public endpoint_lifecycle_subscriber { return; } out.emplace_back( - replica::tablet_mutation_builder(guard.write_timestamp(), s->ks_name(), mig.tablet.table) + replica::tablet_mutation_builder(guard.write_timestamp(), mig.tablet.table) .set_new_replicas(last_token, replace_replica(tmap.get_tablet_info(mig.tablet.tablet).replicas, mig.src, mig.dst)) .set_stage(last_token, locator::tablet_transition_stage::allow_write_both_read_old) .build()); @@ -2090,7 +2089,7 @@ class topology_coordinator : public endpoint_lifecycle_subscriber { table_id table = s->id(); auto get_mutation_builder = [&] () { - return replica::tablet_mutation_builder(guard.write_timestamp(), s->ks_name(), table); + return replica::tablet_mutation_builder(guard.write_timestamp(), table); }; auto transition_to = [&] (locator::tablet_transition_stage stage) { @@ -7577,7 +7576,6 @@ future<> storage_service::move_tablet(table_id table, dht::token token, locator: } std::vector updates; - auto ks_name = _db.local().find_schema(table)->ks_name(); auto& tmap = get_token_metadata().tablets().get_tablet_map(table); auto tid = tmap.get_tablet_id(token); auto& tinfo = tmap.get_tablet_info(tid); @@ -7604,7 +7602,7 @@ future<> storage_service::move_tablet(table_id table, dht::token token, locator: co_return; } - updates.push_back(canonical_mutation(replica::tablet_mutation_builder(guard.write_timestamp(), ks_name, table) + updates.push_back(canonical_mutation(replica::tablet_mutation_builder(guard.write_timestamp(), table) .set_new_replicas(last_token, locator::replace_replica(tinfo.replicas, src, dst)) .set_stage(last_token, locator::tablet_transition_stage::allow_write_both_read_old) .build())); diff --git a/service/tablet_allocator.cc b/service/tablet_allocator.cc index 4167ff86da..9a8a755bd9 100644 --- a/service/tablet_allocator.cc +++ b/service/tablet_allocator.cc @@ -839,7 +839,7 @@ public: std::vector result; if (rs.uses_tablets()) { auto tm = _db.get_shared_token_metadata().get(); - muts.emplace_back(make_drop_tablet_map_mutation(s.keypace_name(), s.id(), ts)); + muts.emplace_back(make_drop_tablet_map_mutation(s.id(), ts)); } } @@ -849,7 +849,7 @@ public: if (rs.uses_tablets()) { auto tm = _db.get_shared_token_metadata().get(); for (auto&& [name, s] : ks.metadata()->cf_meta_data()) { - muts.emplace_back(make_drop_tablet_map_mutation(keyspace_name, s->id(), ts)); + muts.emplace_back(make_drop_tablet_map_mutation(s->id(), ts)); } } } diff --git a/test/boost/tablets_test.cc b/test/boost/tablets_test.cc index 0e64d15922..6bf399f801 100644 --- a/test/boost/tablets_test.cc +++ b/test/boost/tablets_test.cc @@ -319,7 +319,7 @@ SEASTAR_TEST_CASE(test_mutation_builder) { save_tablet_metadata(e.local_db(), tm, ts++).get(); { - tablet_mutation_builder b(ts++, "ks", table1); + tablet_mutation_builder b(ts++, table1); auto last_token = tm.get_tablet_map(table1).get_last_token(tid1); b.set_new_replicas(last_token, tablet_replica_set { tablet_replica {h1, 2}, @@ -359,7 +359,7 @@ SEASTAR_TEST_CASE(test_mutation_builder) { } { - tablet_mutation_builder b(ts++, "ks", table1); + tablet_mutation_builder b(ts++, table1); auto last_token = tm.get_tablet_map(table1).get_last_token(tid1); b.set_stage(last_token, tablet_transition_stage::use_new); e.local_db().apply({freeze(b.build())}, db::no_timeout).get(); @@ -395,7 +395,7 @@ SEASTAR_TEST_CASE(test_mutation_builder) { } { - tablet_mutation_builder b(ts++, "ks", table1); + tablet_mutation_builder b(ts++, table1); auto last_token = tm.get_tablet_map(table1).get_last_token(tid1); b.set_replicas(last_token, tablet_replica_set { tablet_replica {h1, 2}, diff --git a/test/topology_experimental_raft/test_tablets.py b/test/topology_experimental_raft/test_tablets.py index 6dde411306..cbcd18de27 100644 --- a/test/topology_experimental_raft/test_tablets.py +++ b/test/topology_experimental_raft/test_tablets.py @@ -50,7 +50,6 @@ async def get_tablet_replicas(manager: ManagerClient, server: ServerInfo, keyspa table_id = await manager.get_table_id(keyspace_name, table_name) rows = await manager.cql.run_async(f"SELECT last_token, replicas FROM system.tablets where " - f"keyspace_name = '{keyspace_name}' and " f"table_id = {table_id}", host=host) for row in rows: if row.last_token >= token: