From c61d8552500bac65dda9348f6ec15ac8a9fcff5d Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Mon, 26 Jan 2026 18:11:13 +0300 Subject: [PATCH] hints: Provide explicit scheduling group for hint_sender Currently it grabs one from database, but it's not nice to use database as config/sched-groups provider. This PR passes the scheduling group to use for sending hints via manager which, in turn, gets one from proxy via its config (proxy config already carries configuration for hints manager). The group is initialized in main.cc code and is set to the maintenance one (nowadays it's the same as streaming group). This will help splitting the streaming scheduling group into more elaborated groups under the maintenance supergroup: SCYLLADB-351 Signed-off-by: Pavel Emelyanov Closes scylladb/scylladb#28358 --- db/hints/internal/hint_endpoint_manager.cc | 4 ++-- db/hints/internal/hint_endpoint_manager.hh | 2 +- db/hints/internal/hint_sender.cc | 4 ++-- db/hints/internal/hint_sender.hh | 2 +- db/hints/manager.cc | 5 +++-- db/hints/manager.hh | 3 ++- main.cc | 1 + service/storage_proxy.cc | 4 ++-- service/storage_proxy.hh | 1 + 9 files changed, 15 insertions(+), 11 deletions(-) diff --git a/db/hints/internal/hint_endpoint_manager.cc b/db/hints/internal/hint_endpoint_manager.cc index 4954999cd7..c53c164062 100644 --- a/db/hints/internal/hint_endpoint_manager.cc +++ b/db/hints/internal/hint_endpoint_manager.cc @@ -158,7 +158,7 @@ void hint_endpoint_manager::cancel_draining() noexcept { _sender.cancel_draining(); } -hint_endpoint_manager::hint_endpoint_manager(const endpoint_id& key, fs::path hint_directory, manager& shard_manager) +hint_endpoint_manager::hint_endpoint_manager(const endpoint_id& key, fs::path hint_directory, manager& shard_manager, scheduling_group send_sg) : _key(key) , _shard_manager(shard_manager) , _store_gate("hint_endpoint_manager") @@ -169,7 +169,7 @@ hint_endpoint_manager::hint_endpoint_manager(const endpoint_id& key, fs::path hi // Approximate the position of the last written hint by using the same formula as for segment id calculation in commitlog // TODO: Should this logic be deduplicated with what is in the commitlog? , _last_written_rp(this_shard_id(), std::chrono::duration_cast(runtime::get_boot_time().time_since_epoch()).count()) - , _sender(*this, _shard_manager.local_storage_proxy(), _shard_manager.local_db(), _shard_manager.local_gossiper()) + , _sender(*this, _shard_manager.local_storage_proxy(), _shard_manager.local_db(), _shard_manager.local_gossiper(), send_sg) {} hint_endpoint_manager::hint_endpoint_manager(hint_endpoint_manager&& other) diff --git a/db/hints/internal/hint_endpoint_manager.hh b/db/hints/internal/hint_endpoint_manager.hh index 271a435e8f..72eb97e2a5 100644 --- a/db/hints/internal/hint_endpoint_manager.hh +++ b/db/hints/internal/hint_endpoint_manager.hh @@ -63,7 +63,7 @@ private: hint_sender _sender; public: - hint_endpoint_manager(const endpoint_id& key, std::filesystem::path hint_directory, manager& shard_manager); + hint_endpoint_manager(const endpoint_id& key, std::filesystem::path hint_directory, manager& shard_manager, scheduling_group send_sg); hint_endpoint_manager(hint_endpoint_manager&&); ~hint_endpoint_manager(); diff --git a/db/hints/internal/hint_sender.cc b/db/hints/internal/hint_sender.cc index a4a55bcf0a..20b520aab4 100644 --- a/db/hints/internal/hint_sender.cc +++ b/db/hints/internal/hint_sender.cc @@ -122,7 +122,7 @@ const column_mapping& hint_sender::get_column_mapping(lw_shared_ptrsecond; } -hint_sender::hint_sender(hint_endpoint_manager& parent, service::storage_proxy& local_storage_proxy,replica::database& local_db, const gms::gossiper& local_gossiper) noexcept +hint_sender::hint_sender(hint_endpoint_manager& parent, service::storage_proxy& local_storage_proxy,replica::database& local_db, const gms::gossiper& local_gossiper, scheduling_group sg) noexcept : _stopped(make_ready_future<>()) , _ep_key(parent.end_point_key()) , _ep_manager(parent) @@ -130,7 +130,7 @@ hint_sender::hint_sender(hint_endpoint_manager& parent, service::storage_proxy& , _resource_manager(_shard_manager._resource_manager) , _proxy(local_storage_proxy) , _db(local_db) - , _hints_cpu_sched_group(_db.get_streaming_scheduling_group()) + , _hints_cpu_sched_group(sg) , _gossiper(local_gossiper) , _file_update_mutex(_ep_manager.file_update_mutex()) {} diff --git a/db/hints/internal/hint_sender.hh b/db/hints/internal/hint_sender.hh index 28ee062fc3..a84106e8a4 100644 --- a/db/hints/internal/hint_sender.hh +++ b/db/hints/internal/hint_sender.hh @@ -120,7 +120,7 @@ private: std::multimap>>> _replay_waiters; public: - hint_sender(hint_endpoint_manager& parent, service::storage_proxy& local_storage_proxy, replica::database& local_db, const gms::gossiper& local_gossiper) noexcept; + hint_sender(hint_endpoint_manager& parent, service::storage_proxy& local_storage_proxy, replica::database& local_db, const gms::gossiper& local_gossiper, scheduling_group sg) noexcept; ~hint_sender(); /// \brief A constructor that should be called from the copy/move-constructor of hint_endpoint_manager. diff --git a/db/hints/manager.cc b/db/hints/manager.cc index bd3bf19d17..15a0911cce 100644 --- a/db/hints/manager.cc +++ b/db/hints/manager.cc @@ -142,7 +142,7 @@ future<> directory_initializer::ensure_rebalanced() { } manager::manager(service::storage_proxy& proxy, sstring hints_directory, host_filter filter, int64_t max_hint_window_ms, - resource_manager& res_manager, sharded& db) + resource_manager& res_manager, sharded& db, scheduling_group sg) : _hints_dir(fs::path(hints_directory) / fmt::to_string(this_shard_id())) , _host_filter(std::move(filter)) , _proxy(proxy) @@ -150,6 +150,7 @@ manager::manager(service::storage_proxy& proxy, sstring hints_directory, host_fi , _local_db(db.local()) , _draining_eps_gate(seastar::format("hints::manager::{}", _hints_dir.native())) , _resource_manager(res_manager) + , _hints_sending_sched_group(sg) { if (utils::get_local_injector().enter("decrease_hints_flush_period")) { hints_flush_period = std::chrono::seconds{1}; @@ -415,7 +416,7 @@ hint_endpoint_manager& manager::get_ep_manager(const endpoint_id& host_id, const try { std::filesystem::path hint_directory = hints_dir() / (_uses_host_id ? fmt::to_string(host_id) : fmt::to_string(ip)); - auto [it, _] = _ep_managers.emplace(host_id, hint_endpoint_manager{host_id, std::move(hint_directory), *this}); + auto [it, _] = _ep_managers.emplace(host_id, hint_endpoint_manager{host_id, std::move(hint_directory), *this, _hints_sending_sched_group}); hint_endpoint_manager& ep_man = it->second; manager_logger.trace("Created an endpoint manager for {}", host_id); diff --git a/db/hints/manager.hh b/db/hints/manager.hh index 59fadacd10..5abc37af19 100644 --- a/db/hints/manager.hh +++ b/db/hints/manager.hh @@ -133,6 +133,7 @@ private: hint_stats _stats; seastar::metrics::metric_groups _metrics; + scheduling_group _hints_sending_sched_group; // We need to keep a variant here. Before migrating hinted handoff to using host ID, hint directories will // still represent IP addresses. But after the migration, they will start representing host IDs. @@ -155,7 +156,7 @@ private: public: manager(service::storage_proxy& proxy, sstring hints_directory, host_filter filter, - int64_t max_hint_window_ms, resource_manager& res_manager, sharded& db); + int64_t max_hint_window_ms, resource_manager& res_manager, sharded& db, scheduling_group sg); manager(const manager&) = delete; manager& operator=(const manager&) = delete; diff --git a/main.cc b/main.cc index 603f837a01..99972d5331 100644 --- a/main.cc +++ b/main.cc @@ -1306,6 +1306,7 @@ To start the scylla server proper, simply invoke as: scylla server (or just scyl checkpoint(stop_signal, "starting storage proxy"); service::storage_proxy::config spcfg { .hints_directory_initializer = hints_dir_initializer, + .hints_sched_group = maintenance_scheduling_group, }; spcfg.hinted_handoff_enabled = hinted_handoff_enabled; spcfg.available_memory = memory::stats().total_memory(); diff --git a/service/storage_proxy.cc b/service/storage_proxy.cc index 1821988e97..14c8144857 100644 --- a/service/storage_proxy.cc +++ b/service/storage_proxy.cc @@ -3221,9 +3221,9 @@ storage_proxy::storage_proxy(sharded& db, storage_proxy::conf , _write_ack_smp_service_group(cfg.write_ack_smp_service_group) , _next_response_id(std::chrono::system_clock::now().time_since_epoch()/1ms) , _hints_resource_manager(*this, cfg.available_memory / 10, _db.local().get_config().max_hinted_handoff_concurrency) - , _hints_manager(*this, _db.local().get_config().hints_directory(), cfg.hinted_handoff_enabled, _db.local().get_config().max_hint_window_in_ms(), _hints_resource_manager, _db) + , _hints_manager(*this, _db.local().get_config().hints_directory(), cfg.hinted_handoff_enabled, _db.local().get_config().max_hint_window_in_ms(), _hints_resource_manager, _db, cfg.hints_sched_group) , _hints_directory_initializer(std::move(cfg.hints_directory_initializer)) - , _hints_for_views_manager(*this, _db.local().get_config().view_hints_directory(), {}, _db.local().get_config().max_hint_window_in_ms(), _hints_resource_manager, _db) + , _hints_for_views_manager(*this, _db.local().get_config().view_hints_directory(), {}, _db.local().get_config().max_hint_window_in_ms(), _hints_resource_manager, _db, cfg.hints_sched_group) , _stats_key(stats_key) , _features(feat) , _background_write_throttle_threahsold(cfg.available_memory / 10) diff --git a/service/storage_proxy.hh b/service/storage_proxy.hh index 4123ebb77f..867805f6cc 100644 --- a/service/storage_proxy.hh +++ b/service/storage_proxy.hh @@ -195,6 +195,7 @@ public: // they need a separate smp_service_group to prevent an ABBA deadlock // with writes. smp_service_group write_ack_smp_service_group = default_smp_service_group(); + scheduling_group hints_sched_group; }; private: