diff --git a/db/snapshot-ctl.cc b/db/snapshot-ctl.cc index 1305b33155..66011d343b 100644 --- a/db/snapshot-ctl.cc +++ b/db/snapshot-ctl.cc @@ -104,10 +104,8 @@ future<> snapshot_ctl::do_take_column_family_snapshot(sstring ks_name, std::vect for (const auto& table_name : tables) { co_await _db.invoke_on_all([&] (replica::database &db) { auto& cf = db.find_column_family(ks_name, table_name); - // FIXME: need a better way. - // See https://github.com/scylladb/scylla/issues/10526 - if (table_name.find(".") != sstring::npos) { - throw std::invalid_argument("Cannot take a snapshot of a secondary index by itself. Run snapshot on the table that owns the index."); + if (cf.schema()->is_view()) { + throw std::invalid_argument("Do not take a snapshot of a materialized view or a secondary index by itself. Run snapshot on the base table instead."); } return cf.snapshot(db, tag, bool(sf)); }); diff --git a/test/cql-pytest/util.py b/test/cql-pytest/util.py index 80d001893a..13a9982fd3 100644 --- a/test/cql-pytest/util.py +++ b/test/cql-pytest/util.py @@ -107,6 +107,19 @@ def new_materialized_view(cql, table, select, pk, where): finally: cql.execute(f"DROP MATERIALIZED VIEW {mv}") +# A utility function for creating a new temporary secondary index of +# an existing table. +@contextmanager +def new_secondary_index(cql, table, column, name='', extra=''): + keyspace = table.split('.')[0] + if not name: + name = unique_name() + cql.execute(f"CREATE INDEX {name} ON {table} ({column}) {extra}") + try: + yield f"{keyspace}.{name}" + finally: + cql.execute(f"DROP INDEX {keyspace}.{name}") + def project(column_name_string, rows): """Returns a list of column values from each of the rows.""" return [getattr(r, column_name_string) for r in rows] diff --git a/test/rest_api/test_storage_service.py b/test/rest_api/test_storage_service.py index 6364824c0c..8124bcdf27 100644 --- a/test/rest_api/test_storage_service.py +++ b/test/rest_api/test_storage_service.py @@ -10,7 +10,7 @@ import time # Use the util.py library from ../cql-pytest: sys.path.insert(1, sys.path[0] + '/../cql-pytest') -from util import unique_name, new_test_table, new_test_keyspace +from util import unique_name, new_test_table, new_test_keyspace, new_materialized_view, new_secondary_index from rest_util import new_test_snapshot # "keyspace" function: Creates and returns a temporary keyspace to be @@ -321,3 +321,25 @@ def test_storage_service_snapshot(cql, this_dc, rest_api): {'ks': ks1, 'cf': cf10, 'total': 0, 'live': 0} ] }) + +# Verify that snapshots of materialized views and secondary indexes are disallowed. +def test_storage_service_snapshot_mv_si(cql, this_dc, rest_api): + resp = rest_api.send("GET", "storage_service/snapshots") + resp.raise_for_status() + + with new_test_keyspace(cql, f"WITH REPLICATION = {{ 'class' : 'NetworkTopologyStrategy', '{this_dc}' : 1 }}") as keyspace: + schema = 'p int, v text, primary key (p)' + with new_test_table(cql, keyspace, schema) as table: + with new_materialized_view(cql, table, '*', 'v, p', 'v is not null and p is not null') as mv: + try: + with new_test_snapshot(rest_api, keyspace, mv.split('.')[1]) as snap: + pytest.fail(f"Snapshot of materialized view {mv} should have failed") + except requests.HTTPError: + pass + + with new_secondary_index(cql, table, 'v') as si: + try: + with new_test_snapshot(rest_api, keyspace, si.split('.')[1]) as snap: + pytest.fail(f"Snapshot of secondary index {si} should have failed") + except requests.HTTPError: + pass