-
Notifications
You must be signed in to change notification settings - Fork 655
feat(info): report command statistics per namespace #3557
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: unstable
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -66,21 +66,7 @@ Server::Server(engine::Storage *storage, Config *config) | |
| config_(config), | ||
| namespace_(storage) { | ||
| // init commands stats here to prevent concurrent insert, and cause core | ||
| auto commands = redis::CommandTable::GetOriginal(); | ||
|
|
||
| for (const auto &iter : *commands) { | ||
| stats.commands_stats[iter.first].calls = 0; | ||
| stats.commands_stats[iter.first].latency = 0; | ||
|
|
||
| if (stats.bucket_boundaries.size() > 0) { | ||
| // NB: Extra index for the last bucket (Inf) | ||
| for (std::size_t i{0}; i <= stats.bucket_boundaries.size(); ++i) { | ||
| stats.commands_histogram[iter.first].buckets.push_back(std::make_unique<std::atomic<uint64_t>>(0)); | ||
| } | ||
| stats.commands_histogram[iter.first].calls = 0; | ||
| stats.commands_histogram[iter.first].sum = 0; | ||
| } | ||
| } | ||
| initCommandStats(&stats); | ||
|
|
||
| // init cursor_dict_ | ||
| cursor_dict_ = std::make_unique<CursorDictType>(); | ||
|
|
@@ -872,7 +858,18 @@ uint64_t Server::GetClientID() { return client_id_.fetch_add(1, std::memory_orde | |
|
|
||
| void Server::recordInstantaneousMetrics() { | ||
| auto rocksdb_stats = storage->GetDB()->GetDBOptions().statistics; | ||
| stats.TrackInstantaneousMetric(STATS_METRIC_COMMAND, stats.total_calls); | ||
| // Sample each namespace's command metric, and feed the sum into the global metric so the | ||
| // admin/default view reports aggregate ops/sec without keeping a global command counter on the hot path. | ||
| uint64_t total_calls = 0; | ||
| { | ||
| std::shared_lock<std::shared_mutex> lock(ns_stats_mu_); | ||
| for (const auto &[ns, ns_stats] : ns_stats_) { | ||
| auto calls = ns_stats->total_calls.load(); | ||
| ns_stats->TrackInstantaneousMetric(STATS_METRIC_COMMAND, calls); | ||
| total_calls += calls; | ||
| } | ||
| } | ||
| stats.TrackInstantaneousMetric(STATS_METRIC_COMMAND, total_calls); | ||
|
git-hulk marked this conversation as resolved.
|
||
| stats.TrackInstantaneousMetric(STATS_METRIC_NET_INPUT, stats.in_bytes); | ||
| stats.TrackInstantaneousMetric(STATS_METRIC_NET_OUTPUT, stats.out_bytes); | ||
| stats.TrackInstantaneousMetric(STATS_METRIC_ROCKSDB_PUT, | ||
|
|
@@ -1392,11 +1389,75 @@ int64_t Server::GetLastBgsaveTime() { | |
| return last_bgsave_timestamp_secs_ == -1 ? start_time_secs_ : last_bgsave_timestamp_secs_; | ||
| } | ||
|
|
||
| Server::InfoEntries Server::GetStatsInfo() { | ||
| void Server::initCommandStats(Stats *stats) { | ||
| auto commands = redis::CommandTable::GetOriginal(); | ||
| for (const auto &iter : *commands) { | ||
| stats->commands_stats[iter.first].calls = 0; | ||
| stats->commands_stats[iter.first].latency = 0; | ||
|
|
||
| if (stats->bucket_boundaries.size() > 0) { | ||
| // NB: Extra index for the last bucket (Inf) | ||
| for (std::size_t i{0}; i <= stats->bucket_boundaries.size(); ++i) { | ||
| stats->commands_histogram[iter.first].buckets.push_back(std::make_unique<std::atomic<uint64_t>>(0)); | ||
| } | ||
| stats->commands_histogram[iter.first].calls = 0; | ||
| stats->commands_histogram[iter.first].sum = 0; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| std::shared_ptr<Stats> Server::GetOrCreateNamespaceStats(const std::string &ns) { | ||
| { | ||
| std::shared_lock<std::shared_mutex> lock(ns_stats_mu_); | ||
| if (auto it = ns_stats_.find(ns); it != ns_stats_.end()) { | ||
| return it->second; | ||
| } | ||
| } | ||
|
|
||
| std::unique_lock<std::shared_mutex> lock(ns_stats_mu_); | ||
| if (auto it = ns_stats_.find(ns); it != ns_stats_.end()) { | ||
| return it->second; | ||
| } | ||
| auto ns_stats = std::make_shared<Stats>(config_->histogram_bucket_boundaries); | ||
| initCommandStats(ns_stats.get()); | ||
| ns_stats_[ns] = ns_stats; | ||
| return ns_stats; | ||
| } | ||
|
|
||
| std::shared_ptr<Stats> Server::AggregateNamespaceStats() { | ||
| auto agg = std::make_shared<Stats>(config_->histogram_bucket_boundaries); | ||
| initCommandStats(agg.get()); | ||
|
|
||
| std::shared_lock<std::shared_mutex> lock(ns_stats_mu_); | ||
| for (const auto &[ns, ns_stats] : ns_stats_) { | ||
| agg->total_calls.fetch_add(ns_stats->total_calls.load(), std::memory_order_relaxed); | ||
| for (const auto &[cmd, stat] : ns_stats->commands_stats) { | ||
| agg->commands_stats[cmd].calls.fetch_add(stat.calls.load(), std::memory_order_relaxed); | ||
| agg->commands_stats[cmd].latency.fetch_add(stat.latency.load(), std::memory_order_relaxed); | ||
| } | ||
| for (const auto &[cmd, hist] : ns_stats->commands_histogram) { | ||
| auto &agg_hist = agg->commands_histogram[cmd]; | ||
| agg_hist.calls.fetch_add(hist.calls.load(), std::memory_order_relaxed); | ||
| agg_hist.sum.fetch_add(hist.sum.load(), std::memory_order_relaxed); | ||
| for (std::size_t i = 0; i < hist.buckets.size(); ++i) { | ||
| agg_hist.buckets[i]->fetch_add(hist.buckets[i]->load(), std::memory_order_relaxed); | ||
| } | ||
| } | ||
| } | ||
| return agg; | ||
| } | ||
|
Comment on lines
+1427
to
+1448
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Every INFO or latency query from an admin connection rebuilds a full statistics snapshot from scratch A complete statistics object for every registered command is allocated and zero-initialized ( Cost breakdown and duplicated work
This runs once for Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
| Server::InfoEntries Server::GetStatsInfo(const std::string &ns) { | ||
| // Command stats are per namespace; the admin/default namespace sees the aggregate across all of them. | ||
| auto cmd_stats_ptr = ns == kDefaultNamespace ? AggregateNamespaceStats() : GetOrCreateNamespaceStats(ns); | ||
| const Stats &cmd_stats = *cmd_stats_ptr; | ||
|
Comment on lines
+1450
to
+1453
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Command statistics become per-database when Redis SELECT compatibility mode is enabled Command counters are looked up by the connection's namespace ( How database indexes become namespaces and split the statsWhen A possible fix is to treat all Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
| Server::InfoEntries entries; | ||
| entries.emplace_back("total_connections_received", total_clients_.load()); | ||
| entries.emplace_back("total_commands_processed", stats.total_calls.load()); | ||
| entries.emplace_back("instantaneous_ops_per_sec", stats.GetInstantaneousMetric(STATS_METRIC_COMMAND)); | ||
| entries.emplace_back("total_commands_processed", cmd_stats.total_calls.load()); | ||
| auto ops_per_sec = ns == kDefaultNamespace ? stats.GetInstantaneousMetric(STATS_METRIC_COMMAND) | ||
| : cmd_stats.GetInstantaneousMetric(STATS_METRIC_COMMAND); | ||
| entries.emplace_back("instantaneous_ops_per_sec", ops_per_sec); | ||
|
git-hulk marked this conversation as resolved.
|
||
| entries.emplace_back("total_net_input_bytes", stats.in_bytes.load()); | ||
| entries.emplace_back("total_net_output_bytes", stats.out_bytes.load()); | ||
| entries.emplace_back("instantaneous_input_kbps", | ||
|
|
@@ -1420,10 +1481,13 @@ Server::InfoEntries Server::GetStatsInfo() { | |
| return entries; | ||
| } | ||
|
|
||
| Server::InfoEntries Server::GetCommandsStatsInfo() { | ||
| Server::InfoEntries Server::GetCommandsStatsInfo(const std::string &ns) { | ||
| auto cmd_stats_ptr = ns == kDefaultNamespace ? AggregateNamespaceStats() : GetOrCreateNamespaceStats(ns); | ||
| const Stats &cmd_stats = *cmd_stats_ptr; | ||
|
|
||
| InfoEntries entries; | ||
|
|
||
| for (const auto &cmd_stat : stats.commands_stats) { | ||
| for (const auto &cmd_stat : cmd_stats.commands_stats) { | ||
| auto calls = cmd_stat.second.calls.load(); | ||
| if (calls == 0) continue; | ||
|
|
||
|
|
@@ -1433,18 +1497,18 @@ Server::InfoEntries Server::GetCommandsStatsInfo() { | |
| static_cast<double>(latency) / static_cast<double>(calls))); | ||
| } | ||
|
|
||
| for (const auto &cmd_hist : stats.commands_histogram) { | ||
| for (const auto &cmd_hist : cmd_stats.commands_histogram) { | ||
| auto command_name = cmd_hist.first; | ||
| auto calls = stats.commands_histogram[command_name].calls.load(); | ||
| auto calls = cmd_hist.second.calls.load(); | ||
| if (calls == 0) continue; | ||
|
|
||
| auto sum = stats.commands_histogram[command_name].sum.load(); | ||
| auto sum = cmd_hist.second.sum.load(); | ||
| std::string result; | ||
| for (std::size_t i{0}; i < stats.commands_histogram[command_name].buckets.size(); ++i) { | ||
| auto bucket_value = stats.commands_histogram[command_name].buckets[i]->load(); | ||
| for (std::size_t i{0}; i < cmd_hist.second.buckets.size(); ++i) { | ||
| auto bucket_value = cmd_hist.second.buckets[i]->load(); | ||
| auto bucket_bound = std::numeric_limits<double>::infinity(); | ||
| if (i < stats.bucket_boundaries.size()) { | ||
| bucket_bound = stats.bucket_boundaries[i]; | ||
| if (i < cmd_stats.bucket_boundaries.size()) { | ||
| bucket_bound = cmd_stats.bucket_boundaries[i]; | ||
| } | ||
|
|
||
| result.append(fmt::format("{}={},", bucket_bound, bucket_value)); | ||
|
|
@@ -1539,11 +1603,16 @@ Server::InfoEntries Server::GetKeyspaceInfo(const std::string &ns) { | |
| // this section can't be shown when loading(i.e. !is_loading_). | ||
| std::string Server::GetInfo(const std::string &ns, const std::vector<std::string> §ions, InfoFormat format) { | ||
| std::vector<std::pair<std::string, std::function<InfoEntries(Server *)>>> info_funcs = { | ||
| {"Server", &Server::GetServerInfo}, {"Clients", &Server::GetClientsInfo}, | ||
| {"Memory", &Server::GetMemoryInfo}, {"Persistence", &Server::GetPersistenceInfo}, | ||
| {"Stats", &Server::GetStatsInfo}, {"Replication", &Server::GetReplicationInfo}, | ||
| {"CPU", &Server::GetCpuInfo}, {"CommandStats", &Server::GetCommandsStatsInfo}, | ||
| {"Cluster", &Server::GetClusterInfo}, {"Keyspace", [&ns](Server *srv) { return srv->GetKeyspaceInfo(ns); }}, | ||
| {"Server", &Server::GetServerInfo}, | ||
| {"Clients", &Server::GetClientsInfo}, | ||
| {"Memory", &Server::GetMemoryInfo}, | ||
| {"Persistence", &Server::GetPersistenceInfo}, | ||
| {"Stats", [&ns](Server *srv) { return srv->GetStatsInfo(ns); }}, | ||
| {"Replication", &Server::GetReplicationInfo}, | ||
| {"CPU", &Server::GetCpuInfo}, | ||
| {"CommandStats", [&ns](Server *srv) { return srv->GetCommandsStatsInfo(ns); }}, | ||
| {"Cluster", &Server::GetClusterInfo}, | ||
| {"Keyspace", [&ns](Server *srv) { return srv->GetKeyspaceInfo(ns); }}, | ||
| {"RocksDB", &Server::GetRocksDBInfo}, | ||
| }; | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟨 LATENCY command is no longer admin-only, exposing per-namespace latency data to any authenticated user
The
adminflag was removed from the LATENCY command registration (src/commands/cmd_server.cc:1833), so any authenticated namespace user can now runLATENCY HISTOGRAMandLATENCY RESET. The histogram is scoped to the caller's namespace (src/commands/cmd_server.cc:1725-1729), so cross-namespace data is not leaked to non-default users, but this is still a widening of the command's access control surface that was previously restricted to administrators.Was this helpful? React with 👍 or 👎 to provide feedback.