snapshot-ctl: run_snapshot_modify_operation: reject views and secondary index using the schema

Detecting a secondary index by checking for a dot
in the table name is wrong as tables generated by Alternator
may contain a dot in their name.

Instead detect bot hmaterialized view and secondary indexes
using the schema()->is_view() method.

Fixes #10526

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
This commit is contained in:
Benny Halevy
2022-05-09 16:42:33 +03:00
parent 1fbcdbd2e8
commit aa127a2dbb
3 changed files with 38 additions and 5 deletions

View File

@@ -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));
});

View File

@@ -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]

View File

@@ -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