From 5afcec4a3d04ec1d5ca6c6ccf133bd74db25eef9 Mon Sep 17 00:00:00 2001 From: Andrzej Jackowski Date: Wed, 10 Dec 2025 15:36:09 +0100 Subject: [PATCH] Revert "auth: move passwords::check call to alien thread" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The alien thread was a solution for reactor stalls caused by indivisible password‑hashing tasks (scylladb/scylladb#24524). However, because there is only one alien thread, overall hashing throughput was reduced (see, e.g., scylladb/scylla-enterprise#5711). To address this, the alien‑thread solution is reverted, and a hashing implementation with yielding will be introduced later in this patch series. This reverts commit 9574513ec1b5106ff17a60ce1cad964e94ad622a. --- auth/allow_all_authenticator.cc | 4 +--- auth/allow_all_authenticator.hh | 3 +-- auth/certificate_authenticator.cc | 5 ++--- auth/certificate_authenticator.hh | 3 +-- auth/password_authenticator.cc | 10 +++------- auth/password_authenticator.hh | 4 +--- auth/saslauthd_authenticator.cc | 5 ++--- auth/saslauthd_authenticator.hh | 3 +-- auth/service.cc | 5 ++--- auth/service.hh | 4 +--- auth/transitional.cc | 7 +++---- main.cc | 9 +++------ test/lib/cql_test_env.cc | 5 +---- 13 files changed, 22 insertions(+), 45 deletions(-) diff --git a/auth/allow_all_authenticator.cc b/auth/allow_all_authenticator.cc index cab3762273..335d227994 100644 --- a/auth/allow_all_authenticator.cc +++ b/auth/allow_all_authenticator.cc @@ -9,7 +9,6 @@ #include "auth/allow_all_authenticator.hh" #include "service/migration_manager.hh" -#include "utils/alien_worker.hh" #include "utils/class_registrator.hh" namespace auth { @@ -23,7 +22,6 @@ static const class_registrator< cql3::query_processor&, ::service::raft_group0_client&, ::service::migration_manager&, - cache&, - utils::alien_worker&> registration("org.apache.cassandra.auth.AllowAllAuthenticator"); + cache&> registration("org.apache.cassandra.auth.AllowAllAuthenticator"); } diff --git a/auth/allow_all_authenticator.hh b/auth/allow_all_authenticator.hh index c534ca9a7b..3710a0cfb7 100644 --- a/auth/allow_all_authenticator.hh +++ b/auth/allow_all_authenticator.hh @@ -14,7 +14,6 @@ #include "auth/authenticator.hh" #include "auth/cache.hh" #include "auth/common.hh" -#include "utils/alien_worker.hh" namespace cql3 { class query_processor; @@ -30,7 +29,7 @@ extern const std::string_view allow_all_authenticator_name; class allow_all_authenticator final : public authenticator { public: - allow_all_authenticator(cql3::query_processor&, ::service::raft_group0_client&, ::service::migration_manager&, cache&, utils::alien_worker&) { + allow_all_authenticator(cql3::query_processor&, ::service::raft_group0_client&, ::service::migration_manager&, cache&) { } virtual future<> start() override { diff --git a/auth/certificate_authenticator.cc b/auth/certificate_authenticator.cc index 9757817342..de811ef90d 100644 --- a/auth/certificate_authenticator.cc +++ b/auth/certificate_authenticator.cc @@ -35,14 +35,13 @@ static const class_registrator cert_auth_reg(CERT_AUTH_NAME); + , auth::cache&> cert_auth_reg(CERT_AUTH_NAME); enum class auth::certificate_authenticator::query_source { subject, altname }; -auth::certificate_authenticator::certificate_authenticator(cql3::query_processor& qp, ::service::raft_group0_client&, ::service::migration_manager&, auth::cache&, utils::alien_worker&) +auth::certificate_authenticator::certificate_authenticator(cql3::query_processor& qp, ::service::raft_group0_client&, ::service::migration_manager&, auth::cache&) : _queries([&] { auto& conf = qp.db().get_config(); auto queries = conf.auth_certificate_role_queries(); diff --git a/auth/certificate_authenticator.hh b/auth/certificate_authenticator.hh index 28ebfbdd81..192d509d45 100644 --- a/auth/certificate_authenticator.hh +++ b/auth/certificate_authenticator.hh @@ -10,7 +10,6 @@ #pragma once #include "auth/authenticator.hh" -#include "utils/alien_worker.hh" #include // IWYU pragma: keep namespace cql3 { @@ -34,7 +33,7 @@ class certificate_authenticator : public authenticator { enum class query_source; std::vector> _queries; public: - certificate_authenticator(cql3::query_processor&, ::service::raft_group0_client&, ::service::migration_manager&, cache&, utils::alien_worker&); + certificate_authenticator(cql3::query_processor&, ::service::raft_group0_client&, ::service::migration_manager&, cache&); ~certificate_authenticator(); future<> start() override; diff --git a/auth/password_authenticator.cc b/auth/password_authenticator.cc index d6e36f8f61..4868ae55ad 100644 --- a/auth/password_authenticator.cc +++ b/auth/password_authenticator.cc @@ -49,8 +49,7 @@ static const class_registrator< cql3::query_processor&, ::service::raft_group0_client&, ::service::migration_manager&, - cache&, - utils::alien_worker&> password_auth_reg("org.apache.cassandra.auth.PasswordAuthenticator"); + cache&> password_auth_reg("org.apache.cassandra.auth.PasswordAuthenticator"); static thread_local auto rng_for_salt = std::default_random_engine(std::random_device{}()); @@ -64,14 +63,13 @@ std::string password_authenticator::default_superuser(const db::config& cfg) { password_authenticator::~password_authenticator() { } -password_authenticator::password_authenticator(cql3::query_processor& qp, ::service::raft_group0_client& g0, ::service::migration_manager& mm, cache& cache, utils::alien_worker& hashing_worker) +password_authenticator::password_authenticator(cql3::query_processor& qp, ::service::raft_group0_client& g0, ::service::migration_manager& mm, cache& cache) : _qp(qp) , _group0_client(g0) , _migration_manager(mm) , _cache(cache) , _stopped(make_ready_future<>()) , _superuser(default_superuser(qp.db().get_config())) - , _hashing_worker(hashing_worker) {} static bool has_salted_hash(const cql3::untyped_result_set_row& row) { @@ -330,9 +328,7 @@ future password_authenticator::authenticate( } salted_hash = role->salted_hash; } - const bool password_match = co_await _hashing_worker.submit([password = std::move(password), salted_hash] { - return passwords::check(password, *salted_hash); - }); + const bool password_match = passwords::check(password, *salted_hash); if (!password_match) { throw exceptions::authentication_exception("Username and/or password are incorrect"); } diff --git a/auth/password_authenticator.hh b/auth/password_authenticator.hh index 70975fe442..a5a7577ebe 100644 --- a/auth/password_authenticator.hh +++ b/auth/password_authenticator.hh @@ -18,7 +18,6 @@ #include "auth/passwords.hh" #include "auth/cache.hh" #include "service/raft/raft_group0_client.hh" -#include "utils/alien_worker.hh" namespace db { class config; @@ -49,13 +48,12 @@ class password_authenticator : public authenticator { shared_promise<> _superuser_created_promise; // We used to also support bcrypt, SHA-256, and MD5 (ref. scylladb#24524). constexpr static auth::passwords::scheme _scheme = passwords::scheme::sha_512; - utils::alien_worker& _hashing_worker; public: static db::consistency_level consistency_for_user(std::string_view role_name); static std::string default_superuser(const db::config&); - password_authenticator(cql3::query_processor&, ::service::raft_group0_client&, ::service::migration_manager&, cache&, utils::alien_worker&); + password_authenticator(cql3::query_processor&, ::service::raft_group0_client&, ::service::migration_manager&, cache&); ~password_authenticator(); diff --git a/auth/saslauthd_authenticator.cc b/auth/saslauthd_authenticator.cc index d56aa2d558..1e47fe7de7 100644 --- a/auth/saslauthd_authenticator.cc +++ b/auth/saslauthd_authenticator.cc @@ -35,10 +35,9 @@ static const class_registrator< cql3::query_processor&, ::service::raft_group0_client&, ::service::migration_manager&, - cache&, - utils::alien_worker&> saslauthd_auth_reg("com.scylladb.auth.SaslauthdAuthenticator"); + cache&> saslauthd_auth_reg("com.scylladb.auth.SaslauthdAuthenticator"); -saslauthd_authenticator::saslauthd_authenticator(cql3::query_processor& qp, ::service::raft_group0_client&, ::service::migration_manager&, cache&, utils::alien_worker&) +saslauthd_authenticator::saslauthd_authenticator(cql3::query_processor& qp, ::service::raft_group0_client&, ::service::migration_manager&, cache&) : _socket_path(qp.db().get_config().saslauthd_socket_path()) {} diff --git a/auth/saslauthd_authenticator.hh b/auth/saslauthd_authenticator.hh index de590b53f9..38b3d21110 100644 --- a/auth/saslauthd_authenticator.hh +++ b/auth/saslauthd_authenticator.hh @@ -12,7 +12,6 @@ #include "auth/authenticator.hh" #include "auth/cache.hh" -#include "utils/alien_worker.hh" namespace cql3 { class query_processor; @@ -30,7 +29,7 @@ namespace auth { class saslauthd_authenticator : public authenticator { sstring _socket_path; ///< Path to the domain socket on which saslauthd is listening. public: - saslauthd_authenticator(cql3::query_processor&, ::service::raft_group0_client&, ::service::migration_manager&, cache&,utils::alien_worker&); + saslauthd_authenticator(cql3::query_processor&, ::service::raft_group0_client&, ::service::migration_manager&, cache&); future<> start() override; diff --git a/auth/service.cc b/auth/service.cc index 87cd0cda3f..fd2152d4ee 100644 --- a/auth/service.cc +++ b/auth/service.cc @@ -191,8 +191,7 @@ service::service( ::service::migration_manager& mm, const service_config& sc, maintenance_socket_enabled used_by_maintenance_socket, - cache& cache, - utils::alien_worker& hashing_worker) + cache& cache) : service( std::move(c), cache, @@ -200,7 +199,7 @@ service::service( g0, mn, create_object(sc.authorizer_java_name, qp, g0, mm), - create_object(sc.authenticator_java_name, qp, g0, mm, cache, hashing_worker), + create_object(sc.authenticator_java_name, qp, g0, mm, cache), create_object(sc.role_manager_java_name, qp, g0, mm, cache), used_by_maintenance_socket) { } diff --git a/auth/service.hh b/auth/service.hh index 73c7ae04e8..6c5829608b 100644 --- a/auth/service.hh +++ b/auth/service.hh @@ -27,7 +27,6 @@ #include "cql3/description.hh" #include "seastarx.hh" #include "service/raft/raft_group0_client.hh" -#include "utils/alien_worker.hh" #include "utils/observable.hh" #include "utils/serialized_action.hh" #include "service/maintenance_mode.hh" @@ -131,8 +130,7 @@ public: ::service::migration_manager&, const service_config&, maintenance_socket_enabled, - cache&, - utils::alien_worker&); + cache&); future<> start(::service::migration_manager&, db::system_keyspace&); diff --git a/auth/transitional.cc b/auth/transitional.cc index d3eb3aceb3..706d6fc297 100644 --- a/auth/transitional.cc +++ b/auth/transitional.cc @@ -38,8 +38,8 @@ class transitional_authenticator : public authenticator { public: static const sstring PASSWORD_AUTHENTICATOR_NAME; - transitional_authenticator(cql3::query_processor& qp, ::service::raft_group0_client& g0, ::service::migration_manager& mm, cache& cache, utils::alien_worker& hashing_worker) - : transitional_authenticator(std::make_unique(qp, g0, mm, cache, hashing_worker)) { + transitional_authenticator(cql3::query_processor& qp, ::service::raft_group0_client& g0, ::service::migration_manager& mm, cache& cache) + : transitional_authenticator(std::make_unique(qp, g0, mm, cache)) { } transitional_authenticator(std::unique_ptr a) : _authenticator(std::move(a)) { @@ -241,8 +241,7 @@ static const class_registrator< cql3::query_processor&, ::service::raft_group0_client&, ::service::migration_manager&, - auth::cache&, - utils::alien_worker&> transitional_authenticator_reg(auth::PACKAGE_NAME + "TransitionalAuthenticator"); + auth::cache&> transitional_authenticator_reg(auth::PACKAGE_NAME + "TransitionalAuthenticator"); static const class_registrator< auth::authorizer, diff --git a/main.cc b/main.cc index ebdc70c068..9bd7e04b94 100644 --- a/main.cc +++ b/main.cc @@ -748,8 +748,6 @@ To start the scylla server proper, simply invoke as: scylla server (or just scyl // inherit Seastar's CPU affinity masks. We want this thread to be free // to migrate between CPUs; we think that's what makes the most sense. auto rpc_dict_training_worker = utils::alien_worker(startlog, 19, "rpc-dict"); - // niceness=10 is ~10% of normal process time - auto hashing_worker = utils::alien_worker(startlog, 10, "pwd-hash"); return app.run(ac, av, [&] () -> future { @@ -779,8 +777,7 @@ To start the scylla server proper, simply invoke as: scylla server (or just scyl return seastar::async([&app, cfg, ext, &disk_space_monitor_shard0, &cm, &sstm, &db, &qp, &bm, &proxy, &mapreduce_service, &mm, &mm_notifier, &ctx, &opts, &dirs, &prometheus_server, &cf_cache_hitrate_calculator, &load_meter, &feature_service, &gossiper, &snitch, &token_metadata, &erm_factory, &snapshot_ctl, &messaging, &sst_dir_semaphore, &raft_gr, &service_memory_limiter, - &repair, &sst_loader, &auth_cache, &ss, &lifecycle_notifier, &stream_manager, &task_manager, &rpc_dict_training_worker, - &hashing_worker, &vector_store_client] { + &repair, &sst_loader, &auth_cache, &ss, &lifecycle_notifier, &stream_manager, &task_manager, &rpc_dict_training_worker, &vector_store_client] { try { if (opts.contains("relabel-config-file") && !opts["relabel-config-file"].as().empty()) { // calling update_relabel_config_from_file can cause an exception that would stop startup @@ -2060,7 +2057,7 @@ To start the scylla server proper, simply invoke as: scylla server (or just scyl maintenance_auth_config.authenticator_java_name = sstring{auth::allow_all_authenticator_name}; maintenance_auth_config.role_manager_java_name = sstring{auth::maintenance_socket_role_manager_name}; - maintenance_auth_service.start(perm_cache_config, std::ref(qp), std::ref(group0_client), std::ref(mm_notifier), std::ref(mm), maintenance_auth_config, maintenance_socket_enabled::yes, std::ref(auth_cache), std::ref(hashing_worker)).get(); + maintenance_auth_service.start(perm_cache_config, std::ref(qp), std::ref(group0_client), std::ref(mm_notifier), std::ref(mm), maintenance_auth_config, maintenance_socket_enabled::yes, std::ref(auth_cache)).get(); cql_maintenance_server_ctl.emplace(maintenance_auth_service, mm_notifier, gossiper, qp, service_memory_limiter, sl_controller, lifecycle_notifier, *cfg, maintenance_cql_sg_stats_key, maintenance_socket_enabled::yes, dbcfg.statement_scheduling_group); @@ -2336,7 +2333,7 @@ To start the scylla server proper, simply invoke as: scylla server (or just scyl auth_config.authenticator_java_name = qualified_authenticator_name; auth_config.role_manager_java_name = qualified_role_manager_name; - auth_service.start(std::move(perm_cache_config), std::ref(qp), std::ref(group0_client), std::ref(mm_notifier), std::ref(mm), auth_config, maintenance_socket_enabled::no, std::ref(auth_cache), std::ref(hashing_worker)).get(); + auth_service.start(std::move(perm_cache_config), std::ref(qp), std::ref(group0_client), std::ref(mm_notifier), std::ref(mm), auth_config, maintenance_socket_enabled::no, std::ref(auth_cache)).get(); std::any stop_auth_service; // Has to be called after node joined the cluster (join_cluster()) diff --git a/test/lib/cql_test_env.cc b/test/lib/cql_test_env.cc index e93014b64a..e6770e8fb0 100644 --- a/test/lib/cql_test_env.cc +++ b/test/lib/cql_test_env.cc @@ -1128,11 +1128,8 @@ private: auth_config.authenticator_java_name = qualified_authenticator_name; auth_config.role_manager_java_name = qualified_role_manager_name; + _auth_service.start(perm_cache_config, std::ref(_qp), std::ref(group0_client), std::ref(_mnotifier), std::ref(_mm), auth_config, maintenance_socket_enabled::no, std::ref(_auth_cache)).get(); - - const uint64_t niceness = 19; - auto hashing_worker = utils::alien_worker(startlog, niceness, "pwd-hash"); - _auth_service.start(perm_cache_config, std::ref(_qp), std::ref(group0_client), std::ref(_mnotifier), std::ref(_mm), auth_config, maintenance_socket_enabled::no, std::ref(_auth_cache), std::ref(hashing_worker)).get(); _auth_service.invoke_on_all([this] (auth::service& auth) { return auth.start(_mm.local(), _sys_ks.local()); }).get();