From e70d9328fc7e272d5061686253dd3266c912b6e1 Mon Sep 17 00:00:00 2001 From: Casey Bodley Date: Thu, 8 May 2025 15:01:48 -0400 Subject: [PATCH] rgw: move rados-specific parts of RGWRealmWatcher into ConfigStore split RGWRealmWatcher in two halves, moving the rados-specific stuff into a RadosRealmWatcher that inherits from RGWRealmWatcher add factory function ConfigStore::create_realm_watcher() that returns a generic RGWRealmWatcher. ConfigStore backends that don't support watch/notify can return nullptr rgw::AppMain uses this to avoid relying on checking for a "rados" driver name, which doesn't work correctly when there are filters on top of the RadosStore Signed-off-by: Casey Bodley --- src/rgw/CMakeLists.txt | 3 +- src/rgw/driver/dbstore/config/sqlite.cc | 9 + src/rgw/driver/dbstore/config/sqlite.h | 4 + src/rgw/driver/immutable_config/store.cc | 10 +- src/rgw/driver/immutable_config/store.h | 5 + src/rgw/driver/rados/config/realm_watcher.cc | 163 +++++++++++++++++++ src/rgw/driver/rados/config/realm_watcher.h | 42 +++++ src/rgw/driver/rados/config/store.h | 4 + src/rgw/rgw_appmain.cc | 23 ++- src/rgw/rgw_realm_reloader.h | 1 + src/rgw/rgw_realm_watcher.cc | 135 --------------- src/rgw/rgw_realm_watcher.h | 37 +---- src/rgw/rgw_sal_config.h | 7 + 13 files changed, 268 insertions(+), 175 deletions(-) create mode 100644 src/rgw/driver/rados/config/realm_watcher.cc create mode 100644 src/rgw/driver/rados/config/realm_watcher.h diff --git a/src/rgw/CMakeLists.txt b/src/rgw/CMakeLists.txt index 35cf3960e32..1a39111015a 100644 --- a/src/rgw/CMakeLists.txt +++ b/src/rgw/CMakeLists.txt @@ -160,6 +160,7 @@ set(librgw_common_srcs rgw_tracer.cc rgw_lua_background.cc rgw_data_access.cc + rgw_realm_watcher.cc driver/rados/account.cc driver/rados/buckets.cc rgw_bucket_logging.cc @@ -225,6 +226,7 @@ list(APPEND librgw_common_srcs driver/rados/config/period.cc driver/rados/config/period_config.cc driver/rados/config/realm.cc + driver/rados/config/realm_watcher.cc driver/rados/config/store.cc driver/rados/config/zone.cc driver/rados/config/zonegroup.cc) @@ -395,7 +397,6 @@ set(rgw_a_srcs rgw_period_pusher.cc rgw_process.cc rgw_realm_reloader.cc - rgw_realm_watcher.cc rgw_rest_config.cc rgw_rest_info.cc rgw_rest_metadata.cc diff --git a/src/rgw/driver/dbstore/config/sqlite.cc b/src/rgw/driver/dbstore/config/sqlite.cc index 9f497751509..68ae8f0f239 100644 --- a/src/rgw/driver/dbstore/config/sqlite.cc +++ b/src/rgw/driver/dbstore/config/sqlite.cc @@ -25,6 +25,7 @@ #include "include/encoding.h" #include "common/dout.h" #include "common/random_string.h" +#include "rgw_realm_watcher.h" #include "driver/rados/rgw_zone.h" // FIXME: subclass dependency @@ -676,6 +677,14 @@ int SQLiteConfigStore::realm_notify_new_period(const DoutPrefixProvider* dpp, return -ENOTSUP; } +auto SQLiteConfigStore::create_realm_watcher(const DoutPrefixProvider* dpp, + optional_yield y, + const RGWRealm& realm) + -> std::unique_ptr +{ + return nullptr; +} + int SQLiteConfigStore::list_realm_names(const DoutPrefixProvider* dpp, optional_yield y, const std::string& marker, std::span entries, diff --git a/src/rgw/driver/dbstore/config/sqlite.h b/src/rgw/driver/dbstore/config/sqlite.h index bdc05fadeb0..74cb9d7231a 100644 --- a/src/rgw/driver/dbstore/config/sqlite.h +++ b/src/rgw/driver/dbstore/config/sqlite.h @@ -61,6 +61,10 @@ class SQLiteConfigStore : public sal::ConfigStore { int realm_notify_new_period(const DoutPrefixProvider* dpp, optional_yield y, const RGWPeriod& period) override; + auto create_realm_watcher(const DoutPrefixProvider* dpp, + optional_yield y, + const RGWRealm& realm) + -> std::unique_ptr override; int list_realm_names(const DoutPrefixProvider* dpp, optional_yield y, const std::string& marker, std::span entries, diff --git a/src/rgw/driver/immutable_config/store.cc b/src/rgw/driver/immutable_config/store.cc index f691728dd99..4a768064b14 100644 --- a/src/rgw/driver/immutable_config/store.cc +++ b/src/rgw/driver/immutable_config/store.cc @@ -12,8 +12,8 @@ * */ -#include "rgw_zone.h" #include "store.h" +#include "rgw_realm_watcher.h" namespace rgw::sal { @@ -94,6 +94,14 @@ int ImmutableConfigStore::realm_notify_new_period(const DoutPrefixProvider* dpp, return -ENOTSUP; } +auto ImmutableConfigStore::create_realm_watcher(const DoutPrefixProvider* dpp, + optional_yield y, + const RGWRealm& realm) + -> std::unique_ptr +{ + return nullptr; +} + int ImmutableConfigStore::list_realm_names(const DoutPrefixProvider* dpp, optional_yield y, const std::string& marker, std::span entries, diff --git a/src/rgw/driver/immutable_config/store.h b/src/rgw/driver/immutable_config/store.h index 758630848a3..f73eab67749 100644 --- a/src/rgw/driver/immutable_config/store.h +++ b/src/rgw/driver/immutable_config/store.h @@ -15,6 +15,7 @@ #pragma once #include "rgw_sal_config.h" +#include "rgw_zone.h" namespace rgw::sal { @@ -59,6 +60,10 @@ class ImmutableConfigStore : public ConfigStore { virtual int realm_notify_new_period(const DoutPrefixProvider* dpp, optional_yield y, const RGWPeriod& period) override; + virtual auto create_realm_watcher(const DoutPrefixProvider* dpp, + optional_yield y, + const RGWRealm& realm) + -> std::unique_ptr override; virtual int list_realm_names(const DoutPrefixProvider* dpp, optional_yield y, const std::string& marker, std::span entries, diff --git a/src/rgw/driver/rados/config/realm_watcher.cc b/src/rgw/driver/rados/config/realm_watcher.cc new file mode 100644 index 00000000000..e7dbd1d7828 --- /dev/null +++ b/src/rgw/driver/rados/config/realm_watcher.cc @@ -0,0 +1,163 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab ft=cpp + +#include "realm_watcher.h" + +#include "common/errno.h" +#include "include/ceph_assert.h" + +#include "rgw_tools.h" +#include "rgw_zone.h" + +#include "impl.h" +#include "store.h" + +#define dout_subsys ceph_subsys_rgw + +#undef dout_prefix +#define dout_prefix (*_dout << "rgw realm watcher: ") + + +namespace rgw::rados { + +RadosRealmWatcher::RadosRealmWatcher(const DoutPrefixProvider* dpp, CephContext* cct, + librados::Rados& rados, const RGWRealm& realm) + : cct(cct) +{ + // no default realm, nothing to watch + if (realm.get_id().empty()) { + ldpp_dout(dpp, 4) << "No realm, disabling dynamic reconfiguration." << dendl; + return; + } + + // establish the watch on RGWRealm + int r = watch_start(dpp, rados, realm); + if (r < 0) { + ldpp_dout(dpp, -1) << "Failed to establish a watch on RGWRealm, " + "disabling dynamic reconfiguration." << dendl; + return; + } +} + +RadosRealmWatcher::~RadosRealmWatcher() +{ + watch_stop(); +} + +void RadosRealmWatcher::handle_notify(uint64_t notify_id, uint64_t cookie, + uint64_t notifier_id, bufferlist& bl) +{ + if (cookie != watch_handle) + return; + + // send an empty notify ack + bufferlist reply; + pool_ctx.notify_ack(watch_oid, notify_id, cookie, reply); + + try { + auto p = bl.cbegin(); + while (!p.end()) { + RGWRealmNotify notify; + decode(notify, p); + auto watcher = watchers.find(notify); + if (watcher == watchers.end()) { + lderr(cct) << "Failed to find a watcher for notify type " + << static_cast(notify) << dendl; + break; + } + watcher->second.handle_notify(notify, p); + } + } catch (const buffer::error &e) { + lderr(cct) << "Failed to decode realm notifications." << dendl; + } +} + +void RadosRealmWatcher::handle_error(uint64_t cookie, int err) +{ + lderr(cct) << "RadosRealmWatcher::handle_error oid=" << watch_oid << " err=" << err << dendl; + if (cookie != watch_handle) + return; + + watch_restart(); +} + +int RadosRealmWatcher::watch_start(const DoutPrefixProvider* dpp, + librados::Rados& rados, + const RGWRealm& realm) +{ + // initialize a Rados client + int r = rados.init_with_context(cct); + if (r < 0) { + ldpp_dout(dpp, -1) << "Rados client initialization failed with " + << cpp_strerror(-r) << dendl; + return r; + } + r = rados.connect(); + if (r < 0) { + ldpp_dout(dpp, -1) << "Rados client connection failed with " + << cpp_strerror(-r) << dendl; + return r; + } + + // open an IoCtx for the realm's pool + rgw_pool pool(realm.get_pool(cct)); + r = rgw_init_ioctx(dpp, &rados, pool, pool_ctx); + if (r < 0) { + ldpp_dout(dpp, -1) << "Failed to open pool " << pool + << " with " << cpp_strerror(-r) << dendl; + rados.shutdown(); + return r; + } + + // register a watch on the realm's control object + auto oid = realm.get_control_oid(); + r = pool_ctx.watch2(oid, &watch_handle, this); + if (r < 0) { + ldpp_dout(dpp, -1) << "Failed to watch " << oid + << " with " << cpp_strerror(-r) << dendl; + pool_ctx.close(); + rados.shutdown(); + return r; + } + + ldpp_dout(dpp, 10) << "Watching " << oid << dendl; + std::swap(watch_oid, oid); + return 0; +} + +int RadosRealmWatcher::watch_restart() +{ + ceph_assert(!watch_oid.empty()); + int r = pool_ctx.unwatch2(watch_handle); + if (r < 0) { + lderr(cct) << "Failed to unwatch on " << watch_oid + << " with " << cpp_strerror(-r) << dendl; + } + r = pool_ctx.watch2(watch_oid, &watch_handle, this); + if (r < 0) { + lderr(cct) << "Failed to restart watch on " << watch_oid + << " with " << cpp_strerror(-r) << dendl; + pool_ctx.close(); + watch_oid.clear(); + } + return r; +} + +void RadosRealmWatcher::watch_stop() +{ + if (!watch_oid.empty()) { + pool_ctx.unwatch2(watch_handle); + pool_ctx.close(); + watch_oid.clear(); + } +} + +auto RadosConfigStore::create_realm_watcher(const DoutPrefixProvider* dpp, + optional_yield y, + const RGWRealm& realm) + -> std::unique_ptr +{ + return std::make_unique(dpp, dpp->get_cct(), impl->rados, realm); +} + +} // namespace rgw::rados diff --git a/src/rgw/driver/rados/config/realm_watcher.h b/src/rgw/driver/rados/config/realm_watcher.h new file mode 100644 index 00000000000..0d04447dfbf --- /dev/null +++ b/src/rgw/driver/rados/config/realm_watcher.h @@ -0,0 +1,42 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab ft=cpp + +#pragma once + +#include "rgw_realm_watcher.h" +#include "include/rados/librados.hpp" + +class DoutPrefixProvider; +class RGWRealm; + +namespace rgw::rados { + +class RadosRealmWatcher : public RGWRealmWatcher, + public librados::WatchCtx2 { + public: + RadosRealmWatcher(const DoutPrefixProvider* dpp, CephContext* cct, + librados::Rados& rados, const RGWRealm& realm); + ~RadosRealmWatcher() override; + + /// respond to realm notifications by calling the appropriate watcher + void handle_notify(uint64_t notify_id, uint64_t cookie, + uint64_t notifier_id, bufferlist& bl) override; + + /// reestablish the watch if it gets disconnected + void handle_error(uint64_t cookie, int err) override; + + private: + CephContext *const cct; + + librados::IoCtx pool_ctx; + uint64_t watch_handle = 0; + std::string watch_oid; + + int watch_start(const DoutPrefixProvider* dpp, + librados::Rados& rados, + const RGWRealm& realm); + int watch_restart(); + void watch_stop(); +}; + +} // namespace rgw::rados diff --git a/src/rgw/driver/rados/config/store.h b/src/rgw/driver/rados/config/store.h index 463701788f7..715a176e04b 100644 --- a/src/rgw/driver/rados/config/store.h +++ b/src/rgw/driver/rados/config/store.h @@ -66,6 +66,10 @@ class RadosConfigStore : public sal::ConfigStore { virtual int realm_notify_new_period(const DoutPrefixProvider* dpp, optional_yield y, const RGWPeriod& period) override; + virtual auto create_realm_watcher(const DoutPrefixProvider* dpp, + optional_yield y, + const RGWRealm& realm) + -> std::unique_ptr override; virtual int list_realm_names(const DoutPrefixProvider* dpp, optional_yield y, const std::string& marker, std::span entries, diff --git a/src/rgw/rgw_appmain.cc b/src/rgw/rgw_appmain.cc index f2b54f18b8f..218a395279b 100644 --- a/src/rgw/rgw_appmain.cc +++ b/src/rgw/rgw_appmain.cc @@ -520,22 +520,25 @@ int rgw::AppMain::init_frontends2(RGWLib* rgwlib) /* ignore error */ } - if (env.driver->get_name() == "rados") { - // add a watcher to respond to realm configuration changes + // if we're part of a realm, add a watcher to respond to configuration changes + if (const auto& realm = env.site->get_realm(); realm) { + realm_watcher = env.cfgstore->create_realm_watcher(dpp, null_yield, *realm); + } + if (realm_watcher) { pusher = std::make_unique(dpp, env.driver, env.cfgstore, null_yield); + realm_watcher->add_watcher(RGWRealmNotify::ZonesNeedPeriod, *pusher); + fe_pauser = std::make_unique(fes, pusher.get()); rgw_pauser = std::make_unique(); rgw_pauser->add_pauser(fe_pauser.get()); if (env.lua.background) { rgw_pauser->add_pauser(env.lua.background); } + need_context_pool(); reloader = std::make_unique( - env, *implicit_tenant_context, service_map_meta, rgw_pauser.get(), *context_pool); - realm_watcher = std::make_unique(dpp, g_ceph_context, - static_cast(env.driver)->svc()->zone->get_realm()); + env, *implicit_tenant_context, service_map_meta, rgw_pauser.get(), *context_pool); realm_watcher->add_watcher(RGWRealmNotify::Reload, *reloader); - realm_watcher->add_watcher(RGWRealmNotify::ZonesNeedPeriod, *pusher.get()); } return r; @@ -582,8 +585,14 @@ void rgw::AppMain::init_lua() void rgw::AppMain::shutdown(std::function finalize_async_signals) { + // stop the realm reloader + rgw_pauser.reset(); + fe_pauser.reset(); + realm_watcher.reset(); + pusher.reset(); + reloader.reset(); + if (env.driver->get_name() == "rados") { - reloader.reset(); // stop the realm reloader if (g_conf().get_val("rgw_lua_enable")) static_cast(env.lua.manager.get())-> unwatch_reload(dpp); diff --git a/src/rgw/rgw_realm_reloader.h b/src/rgw/rgw_realm_reloader.h index 6cf969da55a..954cc2773a5 100644 --- a/src/rgw/rgw_realm_reloader.h +++ b/src/rgw/rgw_realm_reloader.h @@ -7,6 +7,7 @@ #include "rgw_realm_watcher.h" #include "common/Cond.h" +#include "common/Timer.h" #include "rgw_sal_fwd.h" struct RGWProcessEnv; diff --git a/src/rgw/rgw_realm_watcher.cc b/src/rgw/rgw_realm_watcher.cc index f6cd3475985..9f6f7d83f8d 100644 --- a/src/rgw/rgw_realm_watcher.cc +++ b/src/rgw/rgw_realm_watcher.cc @@ -1,148 +1,13 @@ // -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- // vim: ts=8 sw=2 smarttab ft=cpp -#include "common/errno.h" - #include "rgw_realm_watcher.h" -#include "rgw_tools.h" -#include "rgw_zone.h" - -#define dout_subsys ceph_subsys_rgw - -#undef dout_prefix -#define dout_prefix (*_dout << "rgw realm watcher: ") - - -RGWRealmWatcher::RGWRealmWatcher(const DoutPrefixProvider *dpp, CephContext* cct, const RGWRealm& realm) - : cct(cct) -{ - // no default realm, nothing to watch - if (realm.get_id().empty()) { - ldpp_dout(dpp, 4) << "No realm, disabling dynamic reconfiguration." << dendl; - return; - } - - // establish the watch on RGWRealm - int r = watch_start(dpp, realm); - if (r < 0) { - ldpp_dout(dpp, -1) << "Failed to establish a watch on RGWRealm, " - "disabling dynamic reconfiguration." << dendl; - return; - } -} RGWRealmWatcher::~RGWRealmWatcher() { - watch_stop(); } void RGWRealmWatcher::add_watcher(RGWRealmNotify type, Watcher& watcher) { watchers.emplace(type, watcher); } - -void RGWRealmWatcher::handle_notify(uint64_t notify_id, uint64_t cookie, - uint64_t notifier_id, bufferlist& bl) -{ - if (cookie != watch_handle) - return; - - // send an empty notify ack - bufferlist reply; - pool_ctx.notify_ack(watch_oid, notify_id, cookie, reply); - - try { - auto p = bl.cbegin(); - while (!p.end()) { - RGWRealmNotify notify; - decode(notify, p); - auto watcher = watchers.find(notify); - if (watcher == watchers.end()) { - lderr(cct) << "Failed to find a watcher for notify type " - << static_cast(notify) << dendl; - break; - } - watcher->second.handle_notify(notify, p); - } - } catch (const buffer::error &e) { - lderr(cct) << "Failed to decode realm notifications." << dendl; - } -} - -void RGWRealmWatcher::handle_error(uint64_t cookie, int err) -{ - lderr(cct) << "RGWRealmWatcher::handle_error oid=" << watch_oid << " err=" << err << dendl; - if (cookie != watch_handle) - return; - - watch_restart(); -} - -int RGWRealmWatcher::watch_start(const DoutPrefixProvider *dpp, const RGWRealm& realm) -{ - // initialize a Rados client - int r = rados.init_with_context(cct); - if (r < 0) { - ldpp_dout(dpp, -1) << "Rados client initialization failed with " - << cpp_strerror(-r) << dendl; - return r; - } - r = rados.connect(); - if (r < 0) { - ldpp_dout(dpp, -1) << "Rados client connection failed with " - << cpp_strerror(-r) << dendl; - return r; - } - - // open an IoCtx for the realm's pool - rgw_pool pool(realm.get_pool(cct)); - r = rgw_init_ioctx(dpp, &rados, pool, pool_ctx); - if (r < 0) { - ldpp_dout(dpp, -1) << "Failed to open pool " << pool - << " with " << cpp_strerror(-r) << dendl; - rados.shutdown(); - return r; - } - - // register a watch on the realm's control object - auto oid = realm.get_control_oid(); - r = pool_ctx.watch2(oid, &watch_handle, this); - if (r < 0) { - ldpp_dout(dpp, -1) << "Failed to watch " << oid - << " with " << cpp_strerror(-r) << dendl; - pool_ctx.close(); - rados.shutdown(); - return r; - } - - ldpp_dout(dpp, 10) << "Watching " << oid << dendl; - std::swap(watch_oid, oid); - return 0; -} - -int RGWRealmWatcher::watch_restart() -{ - ceph_assert(!watch_oid.empty()); - int r = pool_ctx.unwatch2(watch_handle); - if (r < 0) { - lderr(cct) << "Failed to unwatch on " << watch_oid - << " with " << cpp_strerror(-r) << dendl; - } - r = pool_ctx.watch2(watch_oid, &watch_handle, this); - if (r < 0) { - lderr(cct) << "Failed to restart watch on " << watch_oid - << " with " << cpp_strerror(-r) << dendl; - pool_ctx.close(); - watch_oid.clear(); - } - return r; -} - -void RGWRealmWatcher::watch_stop() -{ - if (!watch_oid.empty()) { - pool_ctx.unwatch2(watch_handle); - pool_ctx.close(); - watch_oid.clear(); - } -} diff --git a/src/rgw/rgw_realm_watcher.h b/src/rgw/rgw_realm_watcher.h index 2a0c0d07699..6af33166457 100644 --- a/src/rgw/rgw_realm_watcher.h +++ b/src/rgw/rgw_realm_watcher.h @@ -3,13 +3,9 @@ #pragma once -#include "include/rados/librados.hpp" -#include "include/ceph_assert.h" -#include "common/Timer.h" -#include "common/Cond.h" - -class RGWRados; -class RGWRealm; +#include +#include "include/buffer.h" +#include "include/encoding.h" enum class RGWRealmNotify { Reload, @@ -21,7 +17,7 @@ WRITE_RAW_ENCODER(RGWRealmNotify); * RGWRealmWatcher establishes a watch on the current RGWRealm's control object, * and forwards notifications to registered observers. */ -class RGWRealmWatcher : public librados::WatchCtx2 { +class RGWRealmWatcher { public: /** * Watcher is an interface that allows the RGWRealmWatcher to pass @@ -35,32 +31,11 @@ class RGWRealmWatcher : public librados::WatchCtx2 { bufferlist::const_iterator& p) = 0; }; - RGWRealmWatcher(const DoutPrefixProvider *dpp, CephContext* cct, const RGWRealm& realm); - ~RGWRealmWatcher() override; + virtual ~RGWRealmWatcher(); /// register a watcher for the given notification type void add_watcher(RGWRealmNotify type, Watcher& watcher); - /// respond to realm notifications by calling the appropriate watcher - void handle_notify(uint64_t notify_id, uint64_t cookie, - uint64_t notifier_id, bufferlist& bl) override; - - /// reestablish the watch if it gets disconnected - void handle_error(uint64_t cookie, int err) override; - - private: - CephContext *const cct; - - /// keep a separate Rados client whose lifetime is independent of RGWRados - /// so that we don't miss notifications during realm reconfiguration - librados::Rados rados; - librados::IoCtx pool_ctx; - uint64_t watch_handle = 0; - std::string watch_oid; - - int watch_start(const DoutPrefixProvider *dpp, const RGWRealm& realm); - int watch_restart(); - void watch_stop(); - + protected: std::map watchers; }; diff --git a/src/rgw/rgw_sal_config.h b/src/rgw/rgw_sal_config.h index 2f973e2058e..ffc63de4d85 100644 --- a/src/rgw/rgw_sal_config.h +++ b/src/rgw/rgw_sal_config.h @@ -26,6 +26,7 @@ class optional_yield; struct RGWPeriod; struct RGWPeriodConfig; struct RGWRealm; +struct RGWRealmWatcher; struct RGWZoneGroup; struct RGWZoneParams; @@ -91,6 +92,12 @@ class ConfigStore { virtual int realm_notify_new_period(const DoutPrefixProvider* dpp, optional_yield y, const RGWPeriod& period) = 0; + /// Create a RGWRealmWatcher that dispatches cluster notifications from + /// realm_notify_new_period(). May return nullptr if not supported. + virtual auto create_realm_watcher(const DoutPrefixProvider* dpp, + optional_yield y, + const RGWRealm& realm) + -> std::unique_ptr = 0; /// List up to 'entries.size()' realm names starting from the given marker virtual int list_realm_names(const DoutPrefixProvider* dpp, optional_yield y, const std::string& marker, -- 2.39.5