From 26f55345245479c86fc1ce891cc0196b6f1fed51 Mon Sep 17 00:00:00 2001 From: "Adam C. Emerson" Date: Mon, 31 Mar 2025 23:10:06 -0400 Subject: [PATCH] rgw/admin: Don't start background tasks in radosgw-admin Signed-off-by: Adam C. Emerson --- src/rgw/driver/rados/rgw_datalog.cc | 18 +++++++++++------- src/rgw/driver/rados/rgw_datalog.h | 13 +++++++------ src/rgw/driver/rados/rgw_rados.cc | 6 ++++-- src/rgw/driver/rados/rgw_rados.h | 4 ++-- src/rgw/driver/rados/rgw_service.cc | 7 ++++--- src/rgw/driver/rados/rgw_service.h | 12 ++++++------ src/rgw/radosgw-admin/radosgw-admin.cc | 1 + src/rgw/rgw_appmain.cc | 2 +- src/rgw/rgw_object_expirer.cc | 2 +- src/rgw/rgw_realm_reloader.cc | 2 +- src/rgw/rgw_sal.cc | 12 +++++++----- src/rgw/rgw_sal.h | 10 +++++++--- src/test/rgw/rgw_cr_test.cc | 2 +- 13 files changed, 53 insertions(+), 38 deletions(-) diff --git a/src/rgw/driver/rados/rgw_datalog.cc b/src/rgw/driver/rados/rgw_datalog.cc index f3a2b59fcbb4b..e9a14050e11d5 100644 --- a/src/rgw/driver/rados/rgw_datalog.cc +++ b/src/rgw/driver/rados/rgw_datalog.cc @@ -435,16 +435,18 @@ void DataLogBackends::handle_empty_to(uint64_t new_tail) { int RGWDataChangesLog::start(const DoutPrefixProvider *dpp, const RGWZone* zone, const RGWZoneParams& zoneparams, - rgw::sal::RadosStore* store) + rgw::sal::RadosStore* store, + bool background_tasks) { log_data = zone->log_data; rados = &store->get_neorados(); - try { // Blocking in startup code, not ideal, but won't hurt anything. std::exception_ptr eptr = asio::co_spawn(store->get_io_context(), - start(dpp, zoneparams.log_pool), + start(dpp, zoneparams.log_pool, + background_tasks, background_tasks, + background_tasks), async::use_blocked); if (eptr) { std::rethrow_exception(eptr); @@ -472,6 +474,7 @@ RGWDataChangesLog::start(const DoutPrefixProvider *dpp, { down_flag = false; cancel_strand = asio::make_strand(rados->get_executor()); + ran_background = (recovery || watch || renew); auto defbacking = to_log_type( cct->_conf.get_val("rgw_default_data_log_backing")); @@ -1250,7 +1253,8 @@ asio::awaitable DataLogBackends::trim_entries( std::unique_lock l(m); const auto head_gen = (end() - 1)->second->gen_id; const auto tail_gen = begin()->first; - if (target_gen < tail_gen) + if (target_gen < tail_gen) + co_return; auto r = 0; for (auto be = lower_bound(0)->second; @@ -1344,7 +1348,7 @@ bool RGWDataChangesLog::going_down() const asio::awaitable RGWDataChangesLog::shutdown() { DoutPrefix dp{cct, ceph_subsys_rgw, "Datalog Shutdown"}; - if (down_flag) { + if (down_flag || !ran_background) { co_return; } down_flag = true; @@ -1370,7 +1374,7 @@ asio::awaitable RGWDataChangesLog::shutdown() { watchcookie = 0; co_await rados->unwatch(wc, loc, asio::use_awaitable); } - co_await renew_entries(&dp); + co_return; } asio::awaitable RGWDataChangesLog::shutdown_or_timeout() { @@ -1391,7 +1395,7 @@ asio::awaitable RGWDataChangesLog::shutdown_or_timeout() { RGWDataChangesLog::~RGWDataChangesLog() { if (log_data && !down_flag) { lderr(cct) << __PRETTY_FUNCTION__ << ":" << __LINE__ - << ": RGWDataChangesLog destructed without dhutdown." << dendl; + << ": RGWDataChangesLog destructed without shutdown." << dendl; } } diff --git a/src/rgw/driver/rados/rgw_datalog.h b/src/rgw/driver/rados/rgw_datalog.h index 588a79fb76c8d..1b6306eb8f6a7 100644 --- a/src/rgw/driver/rados/rgw_datalog.h +++ b/src/rgw/driver/rados/rgw_datalog.h @@ -366,6 +366,7 @@ class RGWDataChangesLog { bc::flat_map> modified_shards; std::atomic down_flag = { true }; + bool ran_background = false; struct ChangeStatus { std::shared_ptr sync_policy; @@ -416,14 +417,14 @@ public: asio::awaitable start(const DoutPrefixProvider* dpp, const rgw_pool& log_pool, - // For testing - bool recovery = true, - bool watch = true, - bool renew = true); + // Broken out for testing, in use + // they're either all on (radosgw) or + // all off (radosgw-admin) + bool recovery, bool watch, bool renew); int start(const DoutPrefixProvider *dpp, const RGWZone* _zone, - const RGWZoneParams& zoneparams, - rgw::sal::RadosStore* store); + const RGWZoneParams& zoneparams, rgw::sal::RadosStore* store, + bool background_tasks); asio::awaitable establish_watch(const DoutPrefixProvider* dpp, std::string_view oid); asio::awaitable process_notification(const DoutPrefixProvider* dpp, diff --git a/src/rgw/driver/rados/rgw_rados.cc b/src/rgw/driver/rados/rgw_rados.cc index 1e69536033661..1faae9e4c36be 100644 --- a/src/rgw/driver/rados/rgw_rados.cc +++ b/src/rgw/driver/rados/rgw_rados.cc @@ -1418,13 +1418,14 @@ int RGWRados::init_complete(const DoutPrefixProvider *dpp, optional_yield y) } int RGWRados::init_svc(bool raw, const DoutPrefixProvider *dpp, + bool background_tasks, // Ignored when `raw` const rgw::SiteConfig& site) { if (raw) { return svc.init_raw(cct, driver, use_cache, null_yield, dpp, site); } - return svc.init(cct, driver, use_cache, run_sync_thread, null_yield, dpp, site); + return svc.init(cct, driver, use_cache, run_sync_thread, background_tasks, null_yield, dpp, site); } /** @@ -1432,6 +1433,7 @@ int RGWRados::init_svc(bool raw, const DoutPrefixProvider *dpp, * Returns 0 on success, -ERR# on failure. */ int RGWRados::init_begin(CephContext* _cct, const DoutPrefixProvider *dpp, + bool background_tasks, const rgw::SiteConfig& site) { set_context(_cct); @@ -1446,7 +1448,7 @@ int RGWRados::init_begin(CephContext* _cct, const DoutPrefixProvider *dpp, return ret; } - ret = init_svc(false, dpp, site); + ret = init_svc(false, dpp, background_tasks, site); if (ret < 0) { ldpp_dout(dpp, 0) << "ERROR: failed to init services (ret=" << cpp_strerror(-ret) << ")" << dendl; return ret; diff --git a/src/rgw/driver/rados/rgw_rados.h b/src/rgw/driver/rados/rgw_rados.h index ea8b6916e837e..709c2a8b213b9 100644 --- a/src/rgw/driver/rados/rgw_rados.h +++ b/src/rgw/driver/rados/rgw_rados.h @@ -595,9 +595,9 @@ public: CephContext *ctx() { return cct; } /** do all necessary setup of the storage device */ int init_begin(CephContext *_cct, const DoutPrefixProvider *dpp, - const rgw::SiteConfig& site); + bool background_tasks, const rgw::SiteConfig& site); /** Initialize the RADOS instance and prepare to do other ops */ - int init_svc(bool raw, const DoutPrefixProvider *dpp, const rgw::SiteConfig& site); + int init_svc(bool raw, const DoutPrefixProvider *dpp, bool background_tasks, const rgw::SiteConfig& site); virtual int init_rados(); int init_complete(const DoutPrefixProvider *dpp, optional_yield y); void finalize(); diff --git a/src/rgw/driver/rados/rgw_service.cc b/src/rgw/driver/rados/rgw_service.cc index 7de23f223410c..f5207a8a07516 100644 --- a/src/rgw/driver/rados/rgw_service.cc +++ b/src/rgw/driver/rados/rgw_service.cc @@ -51,6 +51,7 @@ int RGWServices_Def::init(CephContext *cct, bool have_cache, bool raw, bool run_sync, + bool background_tasks, optional_yield y, const DoutPrefixProvider *dpp) { @@ -136,7 +137,7 @@ int RGWServices_Def::init(CephContext *cct, r = datalog_rados->start(dpp, &zone->get_zone(), zone->get_zone_params(), - driver); + driver, background_tasks); if (r < 0) { ldpp_dout(dpp, 0) << "ERROR: failed to start datalog_rados service (" << cpp_strerror(-r) << dendl; return r; @@ -261,12 +262,12 @@ void RGWServices_Def::shutdown() has_shutdown = true; } -int RGWServices::do_init(CephContext *_cct, rgw::sal::RadosStore* driver, bool have_cache, bool raw, bool run_sync, optional_yield y, const DoutPrefixProvider *dpp, const rgw::SiteConfig& _site) +int RGWServices::do_init(CephContext *_cct, rgw::sal::RadosStore* driver, bool have_cache, bool raw, bool run_sync, bool background_tasks, optional_yield y, const DoutPrefixProvider *dpp, const rgw::SiteConfig& _site) { cct = _cct; site = &_site; - int r = _svc.init(cct, driver, have_cache, raw, run_sync, y, dpp); + int r = _svc.init(cct, driver, have_cache, raw, run_sync, background_tasks, y, dpp); if (r < 0) { return r; } diff --git a/src/rgw/driver/rados/rgw_service.h b/src/rgw/driver/rados/rgw_service.h index 202f40d20b31e..c9cb71c3e308e 100644 --- a/src/rgw/driver/rados/rgw_service.h +++ b/src/rgw/driver/rados/rgw_service.h @@ -105,8 +105,8 @@ struct RGWServices_Def ~RGWServices_Def(); int init(CephContext *cct, rgw::sal::RadosStore* store, bool have_cache, - bool raw_storage, bool run_sync, optional_yield y, - const DoutPrefixProvider *dpp); + bool raw_storage, bool run_sync, bool background_tasks, + optional_yield y, const DoutPrefixProvider *dpp); void shutdown(); }; @@ -144,20 +144,20 @@ struct RGWServices RGWAsyncRadosProcessor* async_processor; int do_init(CephContext *cct, rgw::sal::RadosStore* store, bool have_cache, - bool raw_storage, bool run_sync, optional_yield y, + bool raw_storage, bool run_sync, bool background_tasks, optional_yield y, const DoutPrefixProvider *dpp, const rgw::SiteConfig& site); int init(CephContext *cct, rgw::sal::RadosStore* store, bool have_cache, - bool run_sync, optional_yield y, const DoutPrefixProvider *dpp, + bool run_sync, bool background_tasks, optional_yield y, const DoutPrefixProvider *dpp, const rgw::SiteConfig& site) { - return do_init(cct, store, have_cache, false, run_sync, y, dpp, site); + return do_init(cct, store, have_cache, false, run_sync, background_tasks, y, dpp, site); } int init_raw(CephContext *cct, rgw::sal::RadosStore* store, bool have_cache, optional_yield y, const DoutPrefixProvider *dpp, const rgw::SiteConfig& site) { - return do_init(cct, store, have_cache, true, false, y, dpp, site); + return do_init(cct, store, have_cache, true, false, false, y, dpp, site); } void shutdown() { _svc.shutdown(); diff --git a/src/rgw/radosgw-admin/radosgw-admin.cc b/src/rgw/radosgw-admin/radosgw-admin.cc index 466e030c15f7f..d59cc98cdc047 100644 --- a/src/rgw/radosgw-admin/radosgw-admin.cc +++ b/src/rgw/radosgw-admin/radosgw-admin.cc @@ -4557,6 +4557,7 @@ int main(int argc, const char **argv) false, false, false, + false, // No background tasks! null_yield, need_cache && g_conf()->rgw_cache_enabled, need_gc); diff --git a/src/rgw/rgw_appmain.cc b/src/rgw/rgw_appmain.cc index ea0b130347f5c..cea8d73c39b33 100644 --- a/src/rgw/rgw_appmain.cc +++ b/src/rgw/rgw_appmain.cc @@ -252,7 +252,7 @@ int rgw::AppMain::init_storage() run_quota, run_sync, g_conf().get_val("rgw_dynamic_resharding"), - true, null_yield, // run notification thread + true, true, null_yield, // run notification thread g_conf()->rgw_cache_enabled); if (!env.driver) { return -EIO; diff --git a/src/rgw/rgw_object_expirer.cc b/src/rgw/rgw_object_expirer.cc index 58723503c5e91..aa71315aa766e 100644 --- a/src/rgw/rgw_object_expirer.cc +++ b/src/rgw/rgw_object_expirer.cc @@ -105,7 +105,7 @@ int main(const int argc, const char **argv) exit(1); } - driver = DriverManager::get_storage(&dp, g_ceph_context, cfg, context_pool, site, false, false, false, false, false, false, null_yield); + driver = DriverManager::get_storage(&dp, g_ceph_context, cfg, context_pool, site, false, false, false, false, false, false, true, null_yield); if (!driver) { std::cerr << "couldn't init storage provider" << std::endl; return EIO; diff --git a/src/rgw/rgw_realm_reloader.cc b/src/rgw/rgw_realm_reloader.cc index 7b2a90b5ae7da..32c35b2bb435e 100644 --- a/src/rgw/rgw_realm_reloader.cc +++ b/src/rgw/rgw_realm_reloader.cc @@ -128,7 +128,7 @@ void RGWRealmReloader::reload() cct->_conf->rgw_enable_quota_threads, cct->_conf->rgw_run_sync_thread, cct->_conf.get_val("rgw_dynamic_resharding"), - true, null_yield, // run notification thread + true, true, null_yield, // run notification thread cct->_conf->rgw_cache_enabled); } diff --git a/src/rgw/rgw_sal.cc b/src/rgw/rgw_sal.cc index e2a6c78b4f436..03aa37e43436a 100644 --- a/src/rgw/rgw_sal.cc +++ b/src/rgw/rgw_sal.cc @@ -79,7 +79,9 @@ rgw::sal::Driver* DriverManager::init_storage_provider(const DoutPrefixProvider* bool run_reshard_thread, bool run_notification_thread, bool use_cache, - bool use_gc, optional_yield y) + bool use_gc, + bool background_tasks, + optional_yield y) { rgw::sal::Driver* driver{nullptr}; @@ -96,7 +98,7 @@ rgw::sal::Driver* DriverManager::init_storage_provider(const DoutPrefixProvider* .set_run_sync_thread(run_sync_thread) .set_run_reshard_thread(run_reshard_thread) .set_run_notification_thread(run_notification_thread) - .init_begin(cct, dpp, site_config) < 0) { + .init_begin(cct, dpp, background_tasks, site_config) < 0) { delete driver; return nullptr; } @@ -123,7 +125,7 @@ rgw::sal::Driver* DriverManager::init_storage_provider(const DoutPrefixProvider* .set_run_sync_thread(run_sync_thread) .set_run_reshard_thread(run_reshard_thread) .set_run_notification_thread(run_notification_thread) - .init_begin(cct, dpp, site_config) < 0) { + .init_begin(cct, dpp, background_tasks, site_config) < 0) { delete driver; return nullptr; } @@ -246,7 +248,7 @@ rgw::sal::Driver* DriverManager::init_raw_storage_provider(const DoutPrefixProvi return nullptr; } - int ret = rados->init_svc(true, dpp, site_config); + int ret = rados->init_svc(true, dpp, false, site_config); if (ret < 0) { ldout(cct, 0) << "ERROR: failed to init services (ret=" << cpp_strerror(-ret) << ")" << dendl; delete driver; @@ -449,4 +451,4 @@ std::string_view rgw_restore_type_dump(rgw::sal::RGWRestoreType type) } } -} // namespace rgw::sal \ No newline at end of file +} // namespace rgw::sal diff --git a/src/rgw/rgw_sal.h b/src/rgw/rgw_sal.h index 1cf8c6cf3eb1f..33a176ebefde8 100644 --- a/src/rgw/rgw_sal.h +++ b/src/rgw/rgw_sal.h @@ -1879,7 +1879,9 @@ public: bool quota_threads, bool run_sync_thread, bool run_reshard_thread, - bool run_notification_thread, optional_yield y, + bool run_notification_thread, + bool background_tasks, + optional_yield y, bool use_cache = true, bool use_gc = true) { rgw::sal::Driver* driver = init_storage_provider(dpp, cct, cfg, io_context, @@ -1890,7 +1892,8 @@ public: run_sync_thread, run_reshard_thread, run_notification_thread, - use_cache, use_gc, y); + use_cache, use_gc, + background_tasks, y); return driver; } /** Get a stripped down driver by service name */ @@ -1916,7 +1919,8 @@ public: bool run_reshard_thread, bool run_notification_thread, bool use_metadata_cache, - bool use_gc, optional_yield y); + bool use_gc, bool background_tasks, + optional_yield y); /** Initialize a new raw Driver */ static rgw::sal::Driver* init_raw_storage_provider(const DoutPrefixProvider* dpp, CephContext* cct, diff --git a/src/test/rgw/rgw_cr_test.cc b/src/test/rgw/rgw_cr_test.cc index d75f776eb7499..3adfda594eb2b 100644 --- a/src/test/rgw/rgw_cr_test.cc +++ b/src/test/rgw/rgw_cr_test.cc @@ -350,7 +350,7 @@ int main(int argc, const char **argv) false, false, false, - true, null_yield, + true, true, null_yield, false)); if (!store) { std::cerr << "couldn't init storage provider" << std::endl; -- 2.39.5