From ba512c52a505d5e1718794c725fd953121a2d45b Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Fri, 26 Apr 2024 16:36:47 +0300 Subject: [PATCH 1/2] sstable_directory: Use sstable location to initialize registry lister When populating sstables on boot a bunch of sstable_directory objects is created. For each sstable there come three -- one for normal, quarantine and staging state. Each is initialized with sstable location (which is now a datadir/ks_name/cf_name-and-uuid) and the desired state (a enum class). When created, the directory object wires up component lister, depending on which storage options are provided. For local sstables a legacy filesystem lister is created and it's initialized with a path where to search files for -- location + / + string(state). But for s3 sstables, that keep their entries in registry, the lister is errorneously initialized with the same location + / + string(state) value. The mistake is that sstables in registry keep location and state in different columns, so for any state lister should query registry with the same location value (then it filters entries by state on its own). Signed-off-by: Pavel Emelyanov --- sstables/sstable_directory.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sstables/sstable_directory.cc b/sstables/sstable_directory.cc index c35cb028d8..1de8eee510 100644 --- a/sstables/sstable_directory.cc +++ b/sstables/sstable_directory.cc @@ -58,7 +58,7 @@ sstable_directory::make_components_lister() { return std::make_unique(_sstable_dir); }, [this] (const data_dictionary::storage_options::s3& os) mutable -> std::unique_ptr { - return std::make_unique(_manager.sstables_registry(), _sstable_dir.native()); + return std::make_unique(_manager.sstables_registry(), _table_dir); } }, _storage_opts->value); } From 5e23493d255ea78debe9c71378e4a1266cb720ba Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Fri, 26 Apr 2024 16:45:49 +0300 Subject: [PATCH 2/2] test: Add test for how quarantined sstables registry entries are loaded Signed-off-by: Pavel Emelyanov --- test/object_store/test_basic.py | 36 +++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/test/object_store/test_basic.py b/test/object_store/test_basic.py index 1808e48877..4944562d97 100644 --- a/test/object_store/test_basic.py +++ b/test/object_store/test_basic.py @@ -156,6 +156,42 @@ async def test_garbage_collect(manager: ManagerClient, s3_server): assert not o.startswith(str(ent[1])), f'Sstable object not cleaned, found {o}' +@pytest.mark.asyncio +async def test_populate_from_quarantine(manager: ManagerClient, s3_server): + '''verify sstables are populated from quarantine state''' + + cfg = {'enable_user_defined_functions': False, + 'object_storage_config_file': str(s3_server.config_file), + 'experimental_features': ['keyspace-storage-options']} + server = await manager.server_add(config=cfg) + + cql = manager.get_cql() + + print(f'Create keyspace (minio listening at {s3_server.address})') + ks, cf = create_ks_and_cf(cql, s3_server) + + res = cql.execute(f"SELECT * FROM {ks}.{cf};") + rows = {x.name: x.value for x in res} + assert len(rows) > 0, 'Test table is empty' + + await manager.api.flush_keyspace(server.ip_addr, ks) + # Move the sstables into "quarantine" + res = cql.execute("SELECT * FROM system.sstables;") + assert len(list(res)) > 0, 'No entries in registry' + for row in res: + cql.execute("UPDATE system.sstables SET state = 'quarantine'" + f" WHERE location = '{row.location}' AND generation = {row.generation};") + + print('Restart scylla') + await manager.server_restart(server.server_id) + cql = await reconnect_driver(manager) + + res = cql.execute(f"SELECT * FROM {ks}.{cf};") + have_res = {x.name: x.value for x in res} + # Quarantine entries must have been processed normally + assert have_res == rows, f'Unexpected table content: {have_res}' + + @pytest.mark.asyncio async def test_misconfigured_storage(manager: ManagerClient, s3_server): '''creating keyspace with unknown endpoint is not allowed'''