mirrors/scylladb
1
0
Fork 0
mirror of https://github.com/scylladb/scylladb.git synced 2026-05-12 19:02:12 +00:00
Code Issues Packages Projects Releases Wiki Activity

sstables/storage_manager: Fix configured endpoints observer

Browse Source 
On start the manager creates observer for object_storage_endpoints
config parameter. The goal is to refresh the maintained set of endpoint
parameters and client upon config change. The observer is created on
shard 0 only, and when kicked it calls manager.invoke-on-all to update
manager on all shards.

However, there's a race here. The thing is that db::config values are
implicitly "sharded" under the hood with the help of plain array. When
any code tries to read a value from db::config::something, the reading
code secretly gets the value from this inner array indexed by the
current shard id.

Next, when the config is updated, it first assigns new values to [0]
element of the hidden array, then calls broadcast_to_all_shards() helper
that copies the valaues from zeroth slot to all the others. But the
manager's observer is triggered when the new value is assigned on zero
index, and if the invoke-on-all lambda (mentioned above) happens to be
faster than broadcast_to_all_shards, the non-zero shards will read old
values from db::config's inner array.

The fix is to instantiate observer on all shards and update only local
shard, whenever this update is triggered.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
This commit is contained in:
Pavel Emelyanov
2025-12-16 13:56:34 +03:00
parent f902eb1632
commit 132aa753da
2 changed files with 2 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
sstables/sstables_manager.cc
View File

@@ -79,7 +79,7 @@ storage_manager::object_storage_endpoint::object_storage_endpoint(db::object_sto
storage_manager::storage_manager(const db::config& cfg, config stm_cfg)
: _object_storage_clients_memory(stm_cfg.object_storage_clients_memory)
, _config_updater(this_shard_id() == 0 ? std::make_unique<config_updater>(cfg, *this) : nullptr)
, _config_updater(std::make_unique<config_updater>(cfg, *this))
{
for (auto& e : cfg.object_storage_endpoints()) {
_object_storage_endpoints.emplace(std::make_pair(e.key(), e));
@@ -170,9 +170,7 @@ std::vector<sstring> storage_manager::endpoints(sstring type) const noexcept {
storage_manager::config_updater::config_updater(const db::config& cfg, storage_manager& sstm)
: action([&sstm, &cfg] () mutable {
return sstm.container().invoke_on_all([&cfg](auto& sstm) -> future<> {
co_await sstm.update_config(cfg);
});
return sstm.update_config(cfg);
})
, observer(cfg.object_storage_endpoints.observe(action.make_observer()))
{}

1
test/cluster/object_store/test_backup.py
View File

@@ -166,7 +166,6 @@ async def test_backup_with_non_existing_parameters(manager: ManagerClient, objec
@pytest.mark.asyncio
@pytest.mark.xfail
async def test_backup_endpoint_config_is_live_updateable(manager: ManagerClient, object_storage):
'''backup should fail if the endpoint is invalid/inaccessible
after updating the config, it should succeed'''