index: Set tombstone_gc when creating secondary index

Before this commit, when the underlying materialized view was created,
it didn't have the property `tombstone_gc` set to any value. That
was a bug and we fix it now.

Two reproducer tests is added for validation. They reproduce the problem
and don't pass before this commit.

Fixes scylladb/scylladb#26542
This commit is contained in:
Dawid Mędrek
2025-10-13 19:04:49 +02:00
committed by Michael Litvak
parent 0e7be65bb1
commit ecc969ca23
5 changed files with 33 additions and 5 deletions

View File

@@ -1911,7 +1911,7 @@ static void make_update_indices_mutations(
if (!view_should_exist(index)) {
return view_ptr(nullptr);
}
auto view = cf.get_index_manager().create_view_for_index(index);
auto view = cf.get_index_manager().create_view_for_index(index, db.as_data_dictionary());
auto view_mutations = make_view_mutations(view, timestamp, true);
view_mutations.copy_to(mutations);
return view;

View File

@@ -26,6 +26,7 @@
#include "db/view/view.hh"
#include "types/concrete_types.hh"
#include "db/tags/extension.hh"
#include "tombstone_gc_extension.hh"
#include "utils/histogram_metrics_helper.hh"
seastar::metrics::label idx{"idx"};
@@ -234,7 +235,7 @@ static data_type type_for_computed_column(cql3::statements::index_target::target
}
}
view_ptr secondary_index_manager::create_view_for_index(const index_metadata& im) const {
view_ptr secondary_index_manager::create_view_for_index(const index_metadata& im, const data_dictionary::database& db) const {
auto schema = _cf.schema();
sstring index_target_name = im.options().at(cql3::statements::index_target::target_option_name);
schema_builder builder{schema->ks_name(), index_table_name(im.name())};
@@ -318,6 +319,10 @@ view_ptr secondary_index_manager::create_view_for_index(const index_metadata& im
format("{} IS NOT NULL", index_target->name_as_cql_string()) :
"";
builder.with_view_info(schema, false, where_clause);
auto tombstone_gc_ext = seastar::make_shared<tombstone_gc_extension>(get_default_tombstone_gc_mode(db, schema->ks_name()));
builder.add_extension(tombstone_gc_extension::NAME, std::move(tombstone_gc_ext));
// A local secondary index should be backed by a *synchronous* view,
// see #16371. A view is marked synchronous with a tag. Non-local indexes
// do not need the tags schema extension at all.

View File

@@ -127,7 +127,7 @@ class secondary_index_manager {
public:
secondary_index_manager(data_dictionary::table cf);
void reload();
view_ptr create_view_for_index(const index_metadata& index) const;
view_ptr create_view_for_index(const index_metadata& index, const data_dictionary::database&) const;
std::vector<index_metadata> get_dependent_indices(const column_definition& cdef) const;
std::vector<index> list_indexes() const;
bool is_index(view_ptr) const;

View File

@@ -2113,4 +2113,27 @@ def test_index_metrics(cql, test_keyspace, scylla_only):
cql.execute(f"SELECT * FROM {table} WHERE v=1")
current_metrics = ScyllaMetrics.query(cql)
current_count = current_metrics.get(f'scylla_index_query_latencies_count', {"idx": index_name, "ks": test_keyspace})
assert current_count - initial_count == 1
assert current_count - initial_count == 1
# Verify that a newly created secondary index has
# the property `tombstone_gc` set to some value.
#
# Reproducer of scylladb/scylladb#26542
def test_tombstone_gc_property(cql, test_keyspace, scylla_only):
with new_test_table(cql, test_keyspace, "p int PRIMARY KEY, v int") as table:
index_name = unique_name()
cql.execute(f"CREATE INDEX {index_name} ON {table}(v)")
desc_row = cql.execute(f"DESCRIBE MATERIALIZED VIEW {test_keyspace}.{index_name}_index").one()
assert "tombstone_gc = {" in desc_row.create_statement
# Verify that a newly created unnamed secondary index has
# the property `tombstone_gc` set to some value.
#
# Reproducer of scylladb/scylladb#26542
def test_tombstone_gc_property_unnamed_index(cql, test_keyspace, scylla_only):
with new_test_table(cql, test_keyspace, "p int PRIMARY KEY, v int") as table:
cql.execute(f"CREATE INDEX ON {table}(v)")
desc_row = cql.execute(f"DESCRIBE MATERIALIZED VIEW {table}_v_idx_index").one()
assert "tombstone_gc = {" in desc_row.create_statement

View File

@@ -333,7 +333,7 @@ std::vector<schema_ptr> do_load_schemas(const db::config& cfg, std::string_view
}
it->schema = std::move(new_base_schema);
it->secondary_idx_man.reload();
auto view = it->secondary_idx_man.create_view_for_index(index);
auto view = it->secondary_idx_man.create_view_for_index(index, db);
real_db.tables.emplace_back(dd_impl, dd_impl.unwrap(ks), view, true);
} else if (auto p = dynamic_cast<cql3::statements::update_statement*>(statement)) {
if (p->keyspace() != db::schema_tables::NAME && p->column_family() != db::schema_tables::DROPPED_COLUMNS) {