From 66f637e477c0d3feeb320d6c8d5330ad1e1c1ef1 Mon Sep 17 00:00:00 2001 From: Casey Bodley Date: Mon, 24 Oct 2022 14:09:10 -0400 Subject: [PATCH] rgw/main: add RGWProcessEnv in AppMain instead of constructing a separate RGWProcessEnv for each frontend, initialize a single instance in AppMain and share a reference with each frontend RGWRealmReloader now takes a reference to RGWProcessEnv instead of a reference to the Store pointer itself, and updates RGWProcessEnv::store on realm reload because RGWRealmReloader may mutate RGWProcessEnv::store, we no longer have a separate AppMain::store pointer, so don't worry about keeping that consistent over reloads Signed-off-by: Casey Bodley --- src/rgw/rgw_appmain.cc | 63 ++++++++++++++++++----------------- src/rgw/rgw_main.h | 4 +-- src/rgw/rgw_realm_reloader.cc | 33 +++++++++--------- src/rgw/rgw_realm_reloader.h | 7 ++-- 4 files changed, 55 insertions(+), 52 deletions(-) diff --git a/src/rgw/rgw_appmain.cc b/src/rgw/rgw_appmain.cc index 0ce240b6a9a25..8ca2dfe6aa8d7 100644 --- a/src/rgw/rgw_appmain.cc +++ b/src/rgw/rgw_appmain.cc @@ -212,7 +212,7 @@ void rgw::AppMain::init_storage() ((!nfs) || (nfs && g_conf()->rgw_nfs_run_sync_thread))); DriverManager::Config cfg = DriverManager::get_config(false, g_ceph_context); - driver = DriverManager::get_storage(dpp, dpp->get_cct(), + env.driver = DriverManager::get_storage(dpp, dpp->get_cct(), cfg, run_gc, run_lc, @@ -238,7 +238,7 @@ void rgw::AppMain::init_http_clients() void rgw::AppMain::cond_init_apis() { - rgw_rest_init(g_ceph_context, driver->get_zone()->get_zonegroup()); + rgw_rest_init(g_ceph_context, env.driver->get_zone()->get_zonegroup()); if (have_http_frontend) { std::vector apis; @@ -272,7 +272,7 @@ void rgw::AppMain::cond_init_apis() if (apis_map.count("s3") > 0 || s3website_enabled) { if (!swift_at_root) { rest.register_default_mgr(set_logging( - rest_filter(driver, RGW_REST_S3, + rest_filter(env.driver, RGW_REST_S3, new RGWRESTMgr_S3(s3website_enabled, sts_enabled, iam_enabled, pubsub_enabled)))); } else { @@ -297,10 +297,10 @@ void rgw::AppMain::cond_init_apis() if (! swift_at_root) { rest.register_resource(g_conf()->rgw_swift_url_prefix, - set_logging(rest_filter(driver, RGW_REST_SWIFT, + set_logging(rest_filter(env.driver, RGW_REST_SWIFT, swift_resource))); } else { - if (driver->get_zone()->get_zonegroup().get_zone_count() > 1) { + if (env.driver->get_zone()->get_zonegroup().get_zone_count() > 1) { derr << "Placing Swift API in the root of URL hierarchy while running" << " multi-site configuration requires another instance of RadosGW" << " with S3 API enabled!" << dendl; @@ -320,7 +320,7 @@ void rgw::AppMain::cond_init_apis() admin_resource->register_resource("info", new RGWRESTMgr_Info); admin_resource->register_resource("usage", new RGWRESTMgr_Usage); /* Register driver-specific admin APIs */ - driver->register_admin_apis(admin_resource); + env.driver->register_admin_apis(admin_resource); rest.register_resource(g_conf()->rgw_admin_entry, admin_resource); } } /* have_http_frontend */ @@ -328,12 +328,13 @@ void rgw::AppMain::cond_init_apis() void rgw::AppMain::init_ldap() { - const string &ldap_uri = driver->ctx()->_conf->rgw_ldap_uri; - const string &ldap_binddn = driver->ctx()->_conf->rgw_ldap_binddn; - const string &ldap_searchdn = driver->ctx()->_conf->rgw_ldap_searchdn; - const string &ldap_searchfilter = driver->ctx()->_conf->rgw_ldap_searchfilter; - const string &ldap_dnattr = driver->ctx()->_conf->rgw_ldap_dnattr; - std::string ldap_bindpw = parse_rgw_ldap_bindpw(driver->ctx()); + CephContext* cct = env.driver->ctx(); + const string &ldap_uri = cct->_conf->rgw_ldap_uri; + const string &ldap_binddn = cct->_conf->rgw_ldap_binddn; + const string &ldap_searchdn = cct->_conf->rgw_ldap_searchdn; + const string &ldap_searchfilter = cct->_conf->rgw_ldap_searchfilter; + const string &ldap_dnattr = cct->_conf->rgw_ldap_dnattr; + std::string ldap_bindpw = parse_rgw_ldap_bindpw(cct); ldh.reset(new rgw::LDAPHelper(ldap_uri, ldap_binddn, ldap_bindpw.c_str(), ldap_searchdn, ldap_searchfilter, ldap_dnattr)); @@ -343,7 +344,7 @@ void rgw::AppMain::init_ldap() void rgw::AppMain::init_opslog() { - rgw_log_usage_init(dpp->get_cct(), driver); + rgw_log_usage_init(dpp->get_cct(), env.driver); OpsLogManifold *olog_manifold = new OpsLogManifold(); if (!g_conf()->rgw_ops_log_socket_path.empty()) { @@ -359,7 +360,7 @@ void rgw::AppMain::init_opslog() ops_log_file->start(); olog_manifold->add_sink(ops_log_file); } - olog_manifold->add_sink(new OpsLogRados(driver)); + olog_manifold->add_sink(new OpsLogRados(env.driver)); olog = olog_manifold; } /* init_opslog */ @@ -390,7 +391,7 @@ int rgw::AppMain::init_frontends2(RGWLib* rgwlib) implicit_tenant_context.reset(new rgw::auth::ImplicitTenants{g_conf()}); g_conf().add_observer(implicit_tenant_context.get()); auto auth_registry = - rgw::auth::StrategyRegistry::create(dpp->get_cct(), *(implicit_tenant_context.get()), driver); + rgw::auth::StrategyRegistry::create(dpp->get_cct(), *implicit_tenant_context, env.driver); /* allocate a mime table (you'd never guess that from the name) */ rgw_tools_init(dpp, dpp->get_cct()); @@ -402,6 +403,13 @@ int rgw::AppMain::init_frontends2(RGWLib* rgwlib) ratelimiter.reset(new ActiveRateLimiter{dpp->get_cct()}); ratelimiter->start(); + // initialize RGWProcessEnv + env.rest = &rest; + env.olog = olog; + env.auth_registry = auth_registry; + env.ratelimiting = ratelimiter.get(); + env.lua_background = lua_background.get(); + int fe_count = 0; for (multimap::iterator fiter = fe_map.begin(); fiter != fe_map.end(); ++fiter, ++fe_count) { @@ -416,20 +424,12 @@ int rgw::AppMain::init_frontends2(RGWLib* rgwlib) RGWFrontend* fe = nullptr; if (framework == "loadgen") { - RGWProcessEnv env = {driver, &rest, olog, - auth_registry, ratelimiter.get(), lua_background.get()}; - fe = new RGWLoadGenFrontend(env, config); } else if (framework == "beast") { - int port; - config->get_val("port", 80, &port); - RGWProcessEnv env{driver, &rest, olog, - auth_registry, ratelimiter.get(), lua_background.get()}; - fe = new RGWAsioFrontend(env, config, *(sched_ctx.get())); + fe = new RGWAsioFrontend(env, config, *sched_ctx); } else if (framework == "rgw-nfs") { - RGWProcessEnv env = { driver, &rest, olog }; fe = new RGWLibFrontend(env, config); if (rgwlib) { rgwlib->set_fe(static_cast(fe)); @@ -460,24 +460,24 @@ int rgw::AppMain::init_frontends2(RGWLib* rgwlib) } std::string daemon_type = (nfs) ? "rgw-nfs" : "rgw"; - r = driver->register_to_service_map(dpp, daemon_type, service_map_meta); + r = env.driver->register_to_service_map(dpp, daemon_type, service_map_meta); if (r < 0) { derr << "ERROR: failed to register to service map: " << cpp_strerror(-r) << dendl; /* ignore error */ } - if (driver->get_name() == "rados") { + if (env.driver->get_name() == "rados") { // add a watcher to respond to realm configuration changes - pusher = std::make_unique(dpp, driver, null_yield); + pusher = std::make_unique(dpp, env.driver, null_yield); fe_pauser = std::make_unique(fes, *(implicit_tenant_context.get()), pusher.get()); rgw_pauser = std::make_unique(); rgw_pauser->add_pauser(fe_pauser.get()); if (lua_background) { rgw_pauser->add_pauser(lua_background.get()); } - reloader = std::make_unique(driver, service_map_meta, rgw_pauser.get()); + reloader = std::make_unique(env, service_map_meta, rgw_pauser.get()); realm_watcher = std::make_unique(dpp, g_ceph_context, - static_cast(driver)->svc()->zone->get_realm()); + static_cast(env.driver)->svc()->zone->get_realm()); realm_watcher->add_watcher(RGWRealmNotify::Reload, *reloader); realm_watcher->add_watcher(RGWRealmNotify::ZonesNeedPeriod, *pusher.get()); } @@ -508,6 +508,7 @@ void rgw::AppMain::init_notification_endpoints() void rgw::AppMain::init_lua() { + rgw::sal::Driver* driver = env.driver; int r{0}; const auto &luarocks_path = g_conf().get_val("rgw_luarocks_location"); @@ -543,7 +544,7 @@ void rgw::AppMain::init_lua() void rgw::AppMain::shutdown(std::function finalize_async_signals) { - if (driver->get_name() == "rados") { + if (env.driver->get_name() == "rados") { reloader.reset(); // stop the realm reloader } @@ -570,7 +571,7 @@ void rgw::AppMain::shutdown(std::function finalize_async_signals) lua_background->shutdown(); } - DriverManager::close_storage(driver); + DriverManager::close_storage(env.driver); rgw_tools_cleanup(); rgw_shutdown_resolver(); diff --git a/src/rgw/rgw_main.h b/src/rgw/rgw_main.h index ca95319866424..d6e6de37a8c29 100644 --- a/src/rgw/rgw_main.h +++ b/src/rgw/rgw_main.h @@ -75,8 +75,8 @@ class AppMain { std::unique_ptr fe_pauser; std::unique_ptr realm_watcher; std::unique_ptr rgw_pauser; - rgw::sal::Driver* driver; DoutPrefixProvider* dpp; + RGWProcessEnv env; public: AppMain(DoutPrefixProvider* dpp) @@ -87,7 +87,7 @@ public: = []() { /* nada */}); rgw::sal::Driver* get_driver() { - return driver; + return env.driver; } rgw::LDAPHelper* get_ldh() { diff --git a/src/rgw/rgw_realm_reloader.cc b/src/rgw/rgw_realm_reloader.cc index e82b01fe27609..c79ad73672a70 100644 --- a/src/rgw/rgw_realm_reloader.cc +++ b/src/rgw/rgw_realm_reloader.cc @@ -7,6 +7,7 @@ #include "rgw_log.h" #include "rgw_rest.h" #include "rgw_user.h" +#include "rgw_process.h" #include "rgw_sal.h" #include "rgw_sal_rados.h" @@ -26,12 +27,12 @@ static constexpr bool USE_SAFE_TIMER_CALLBACKS = false; -RGWRealmReloader::RGWRealmReloader(rgw::sal::Driver*& driver, std::map& service_map_meta, +RGWRealmReloader::RGWRealmReloader(RGWProcessEnv& env, std::map& service_map_meta, Pauser* frontends) - : driver(driver), + : env(env), service_map_meta(service_map_meta), frontends(frontends), - timer(driver->ctx(), mutex, USE_SAFE_TIMER_CALLBACKS), + timer(env.driver->ctx(), mutex, USE_SAFE_TIMER_CALLBACKS), mutex(ceph::make_mutex("RGWRealmReloader")), reload_scheduled(nullptr) { @@ -54,12 +55,12 @@ class RGWRealmReloader::C_Reload : public Context { void RGWRealmReloader::handle_notify(RGWRealmNotify type, bufferlist::const_iterator& p) { - if (!driver) { + if (!env.driver) { /* we're in the middle of reload */ return; } - CephContext *const cct = driver->ctx(); + CephContext *const cct = env.driver->ctx(); std::lock_guard lock{mutex}; if (reload_scheduled) { @@ -79,7 +80,7 @@ void RGWRealmReloader::handle_notify(RGWRealmNotify type, void RGWRealmReloader::reload() { - CephContext *const cct = driver->ctx(); + CephContext *const cct = env.driver->ctx(); const DoutPrefix dp(cct, dout_subsys, "rgw realm reloader: "); ldpp_dout(&dp, 1) << "Pausing frontends for realm update..." << dendl; @@ -91,8 +92,8 @@ void RGWRealmReloader::reload() rgw_log_usage_finalize(); // destroy the existing driver - DriverManager::close_storage(driver); - driver = nullptr; + DriverManager::close_storage(env.driver); + env.driver = nullptr; ldpp_dout(&dp, 1) << "driver closed" << dendl; { @@ -103,12 +104,12 @@ void RGWRealmReloader::reload() } - while (!driver) { + while (!env.driver) { // recreate and initialize a new driver DriverManager::Config cfg; cfg.store_name = "rados"; cfg.filter_name = "none"; - driver = + env.driver = DriverManager::get_storage(&dp, cct, cfg, cct->_conf->rgw_enable_gc_threads, @@ -128,7 +129,7 @@ void RGWRealmReloader::reload() // don't want to assert or abort the entire cluster. instead, just // sleep until we get another notification, and retry until we get // a working configuration - if (driver == nullptr) { + if (env.driver == nullptr) { ldpp_dout(&dp, -1) << "Failed to reinitialize RGWRados after a realm " "configuration update. Waiting for a new update." << dendl; @@ -145,7 +146,7 @@ void RGWRealmReloader::reload() // if we successfully created a driver, clean it up outside of the lock, // then continue to loop and recreate another - std::swap(driver, store_cleanup); + std::swap(env.driver, store_cleanup); } } @@ -157,7 +158,7 @@ void RGWRealmReloader::reload() } } - int r = driver->register_to_service_map(&dp, "rgw", service_map_meta); + int r = env.driver->register_to_service_map(&dp, "rgw", service_map_meta); if (r < 0) { ldpp_dout(&dp, -1) << "ERROR: failed to register to service map: " << cpp_strerror(-r) << dendl; @@ -167,11 +168,11 @@ void RGWRealmReloader::reload() ldpp_dout(&dp, 1) << "Finishing initialization of new driver" << dendl; // finish initializing the new driver ldpp_dout(&dp, 1) << " - REST subsystem init" << dendl; - rgw_rest_init(cct, driver->get_zone()->get_zonegroup()); + rgw_rest_init(cct, env.driver->get_zone()->get_zonegroup()); ldpp_dout(&dp, 1) << " - usage subsystem init" << dendl; - rgw_log_usage_init(cct, driver); + rgw_log_usage_init(cct, env.driver); ldpp_dout(&dp, 1) << "Resuming frontends with new realm configuration." << dendl; - frontends->resume(driver); + frontends->resume(env.driver); } diff --git a/src/rgw/rgw_realm_reloader.h b/src/rgw/rgw_realm_reloader.h index 20538ad65bdf8..b07be97b99c73 100644 --- a/src/rgw/rgw_realm_reloader.h +++ b/src/rgw/rgw_realm_reloader.h @@ -8,6 +8,8 @@ #include "common/Cond.h" #include "rgw_sal_fwd.h" +struct RGWProcessEnv; + /** * RGWRealmReloader responds to new period notifications by recreating RGWRados * with the updated realm configuration. @@ -31,7 +33,7 @@ class RGWRealmReloader : public RGWRealmWatcher::Watcher { virtual void resume(rgw::sal::Driver* driver) = 0; }; - RGWRealmReloader(rgw::sal::Driver*& driver, std::map& service_map_meta, + RGWRealmReloader(RGWProcessEnv& env, std::map& service_map_meta, Pauser* frontends); ~RGWRealmReloader() override; @@ -44,8 +46,7 @@ class RGWRealmReloader : public RGWRealmWatcher::Watcher { class C_Reload; //< Context that calls reload() - /// main()'s driver pointer as a reference, modified by reload() - rgw::sal::Driver*& driver; + RGWProcessEnv& env; std::map& service_map_meta; Pauser *const frontends; -- 2.39.5