From 724b9e66ea6eb38bbf1198a050da64d6176a8a48 Mon Sep 17 00:00:00 2001 From: Marcin Maliszkiewicz Date: Mon, 20 Apr 2026 15:06:13 +0200 Subject: [PATCH 1/2] audit: enable heterogeneous lookup on audited keyspaces/tables Replace the bare std::set/std::map> member types with named aliases that use std::less<> as the comparator. The transparent comparator enables heterogeneous lookup with string_view keys. This commit is a pure refactor with no behavioral change: the parser return types, constructor parameters, observer template instantiations, and start_audit() locals are all updated to use the aliases. --- audit/audit.cc | 20 ++++++++++---------- audit/audit.hh | 15 ++++++++++----- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/audit/audit.cc b/audit/audit.cc index 852d44b511..5d3744781c 100644 --- a/audit/audit.cc +++ b/audit/audit.cc @@ -113,8 +113,8 @@ static category_set parse_audit_categories(const sstring& data) { return result; } -static std::map> parse_audit_tables(const sstring& data) { - std::map> result; +static audit::audited_tables_t parse_audit_tables(const sstring& data) { + audit::audited_tables_t result; if (!data.empty()) { std::vector tokens; boost::split(tokens, data, boost::is_any_of(",")); @@ -139,8 +139,8 @@ static std::map> parse_audit_tables(const sstring& da return result; } -static std::set parse_audit_keyspaces(const sstring& data) { - std::set result; +static audit::audited_keyspaces_t parse_audit_keyspaces(const sstring& data) { + audit::audited_keyspaces_t result; if (!data.empty()) { std::vector tokens; boost::split(tokens, data, boost::is_any_of(",")); @@ -156,8 +156,8 @@ audit::audit(locator::shared_token_metadata& token_metadata, cql3::query_processor& qp, service::migration_manager& mm, std::set&& audit_modes, - std::set&& audited_keyspaces, - std::map>&& audited_tables, + audited_keyspaces_t&& audited_keyspaces, + audited_tables_t&& audited_tables, category_set&& audited_categories, const db::config& cfg) : _token_metadata(token_metadata) @@ -165,8 +165,8 @@ audit::audit(locator::shared_token_metadata& token_metadata, , _audited_tables(std::move(audited_tables)) , _audited_categories(std::move(audited_categories)) , _cfg(cfg) - , _cfg_keyspaces_observer(cfg.audit_keyspaces.observe([this] (sstring const& new_value){ update_config>(new_value, parse_audit_keyspaces, _audited_keyspaces); })) - , _cfg_tables_observer(cfg.audit_tables.observe([this] (sstring const& new_value){ update_config>>(new_value, parse_audit_tables, _audited_tables); })) + , _cfg_keyspaces_observer(cfg.audit_keyspaces.observe([this] (sstring const& new_value){ update_config(new_value, parse_audit_keyspaces, _audited_keyspaces); })) + , _cfg_tables_observer(cfg.audit_tables.observe([this] (sstring const& new_value){ update_config(new_value, parse_audit_tables, _audited_tables); })) , _cfg_categories_observer(cfg.audit_categories.observe([this] (sstring const& new_value){ update_config(new_value, parse_audit_categories, _audited_categories); })) { _storage_helper_ptr = create_storage_helper(std::move(audit_modes), qp, mm); @@ -181,8 +181,8 @@ future<> audit::start_audit(const db::config& cfg, sharded(); } category_set audited_categories = parse_audit_categories(cfg.audit_categories()); - std::map> audited_tables = parse_audit_tables(cfg.audit_tables()); - std::set audited_keyspaces = parse_audit_keyspaces(cfg.audit_keyspaces()); + audit::audited_tables_t audited_tables = parse_audit_tables(cfg.audit_tables()); + audit::audited_keyspaces_t audited_keyspaces = parse_audit_keyspaces(cfg.audit_keyspaces()); logger.info("Audit is enabled. Auditing to: \"{}\", with the following categories: \"{}\", keyspaces: \"{}\", and tables: \"{}\"", cfg.audit(), cfg.audit_categories(), cfg.audit_keyspaces(), cfg.audit_tables()); diff --git a/audit/audit.hh b/audit/audit.hh index 3663949153..bf81cdd607 100644 --- a/audit/audit.hh +++ b/audit/audit.hh @@ -129,10 +129,15 @@ public: class storage_helper; class audit final : public seastar::async_sharded_service { +public: + // Transparent comparator (std::less<>) enables heterogeneous lookup with + // string_view keys. + using audited_keyspaces_t = std::set>; + using audited_tables_t = std::map>, std::less<>>; +private: locator::shared_token_metadata& _token_metadata; - std::set _audited_keyspaces; - // Maps keyspace name to set of table names in that keyspace - std::map> _audited_tables; + audited_keyspaces_t _audited_keyspaces; + audited_tables_t _audited_tables; category_set _audited_categories; std::unique_ptr _storage_helper_ptr; @@ -164,8 +169,8 @@ public: cql3::query_processor& qp, service::migration_manager& mm, std::set&& audit_modes, - std::set&& audited_keyspaces, - std::map>&& audited_tables, + audited_keyspaces_t&& audited_keyspaces, + audited_tables_t&& audited_tables, category_set&& audited_categories, const db::config& cfg); ~audit(); From c136b2e64055c5a4e659e5b901a729a64611cd6f Mon Sep 17 00:00:00 2001 From: Marcin Maliszkiewicz Date: Mon, 20 Apr 2026 15:06:57 +0200 Subject: [PATCH 2/2] audit: drop sstring temporaries on the will_log() fast path audit::will_log() is called for every CQL/Alternator request. With non-empty keyspace it does: _audited_keyspaces.find(sstring(keyspace)) should_log_table(sstring(keyspace), sstring(table)) constructing three temporary sstrings from the std::string_view arguments on every call. Now that the underlying associative containers use std::less<> as comparator (previous commit), find() accepts the string_view directly. Switch should_log_table() to take string_view as well so the temporaries disappear entirely. For short keyspace names the temporaries stay in SSO so allocs/op is unchanged at 58.1, but each construction still costs ~60 instructions. perf-simple-query --smp 1 --duration 15 --audit "table" --audit-keyspaces "ks-non-existing" --audit-categories "DCL,DDL,AUTH,DML,QUERY" build: --mode=release --use-profile="" (no PGO) Before (regression introduced in 9646ee05bd): instructions_per_op: 36952 After: instructions_per_op: 36768 Brings insns/op back to the pre-regression baseline 3d0582d51e (insns/op ~36777) within the per-run noise of ~15 insns standard deviation, eliminating the ~180 insns/op regression. Fixes https://scylladb.atlassian.net/browse/SCYLLADB-1616 --- audit/audit.cc | 6 +++--- audit/audit.hh | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/audit/audit.cc b/audit/audit.cc index 5d3744781c..9b014a810e 100644 --- a/audit/audit.cc +++ b/audit/audit.cc @@ -304,7 +304,7 @@ future<> inspect_login(const sstring& username, socket_address client_ip, bool e return audit::local_audit_instance().log_login(username, client_ip, error); } -bool audit::should_log_table(const sstring& keyspace, const sstring& name) const { +bool audit::should_log_table(std::string_view keyspace, std::string_view name) const { auto keyspace_it = _audited_tables.find(keyspace); return keyspace_it != _audited_tables.cend() && keyspace_it->second.find(name) != keyspace_it->second.cend(); } @@ -319,8 +319,8 @@ bool audit::will_log(statement_category cat, std::string_view keyspace, std::str // so it is logged whenever the category matches. return _audited_categories.contains(cat) && (keyspace.empty() - || _audited_keyspaces.find(sstring(keyspace)) != _audited_keyspaces.cend() - || should_log_table(sstring(keyspace), sstring(table)) + || _audited_keyspaces.find(keyspace) != _audited_keyspaces.cend() + || should_log_table(keyspace, table) || cat == statement_category::AUTH || cat == statement_category::ADMIN || cat == statement_category::DCL); diff --git a/audit/audit.hh b/audit/audit.hh index bf81cdd607..2b482d2047 100644 --- a/audit/audit.hh +++ b/audit/audit.hh @@ -150,7 +150,7 @@ private: template void update_config(const sstring & new_value, std::function parse_func, T& cfg_parameter); - bool should_log_table(const sstring& keyspace, const sstring& name) const; + bool should_log_table(std::string_view keyspace, std::string_view name) const; public: static seastar::sharded& audit_instance() { // FIXME: leaked intentionally to avoid shutdown problems, see #293