Skip to content

Rename active clients stats fields#14972

Open
udi-speedb wants to merge 2 commits intoredis:unstablefrom
udi-speedb:udi/RED-188967-OSS-backport-active-clients-stats-renamed-counters
Open

Rename active clients stats fields#14972
udi-speedb wants to merge 2 commits intoredis:unstablefrom
udi-speedb:udi/RED-188967-OSS-backport-active-clients-stats-renamed-counters

Conversation

@udi-speedb
Copy link
Copy Markdown
Contributor

@udi-speedb udi-speedb commented Apr 2, 2026

This PR follows #14841

Optimize the active client statistics fields with unclear names.

Fields changes

INFO stats:

  • avg_pipeline_length_sum -> commands_per_parse_batch_sum
  • avg_pipeline_length_cnt -> commands_per_parse_batch_cnt
  • avg_pipeline_length -> commands_per_parse_batch_avg

CLIENT LIST / CLIENT INFO:

  • avg-pipeline-len-sum -> parse-batch-cmd-sum
  • avg-pipeline-len-cnt -> parse-batch-cnt

Note

Low Risk
Mostly a metric/field rename affecting observability outputs and tests; risk is limited to compatibility for users parsing the old stat names.

Overview
Renames the client/server “avg pipeline length” statistics to better reflect what’s actually being measured: commands parsed per input parsing batch.

This updates internal counters (client + global), the INFO stats fields (including the derived average), and the CLIENT LIST/CLIENT INFO output keys to the new commands_per_parse_batch* / parse-batch-* names, along with corresponding test expectations.

Written by Cursor Bugbot for commit f064608. This will update automatically on new commits. Configure here.

@augmentcode
Copy link
Copy Markdown

augmentcode Bot commented Apr 2, 2026

🤖 Augment PR Summary

Summary: This PR backports new client-activity and parsing statistics to improve observability.

Changes:

  • Adds an active_clients metric (sliding ~512ms window) and updates main-thread + IO-thread handoff paths to account for activity.
  • Introduces server-wide counters for input-buffer processing frequency and parse-batch (pipeline) sizing, including an average.
  • Adds per-client counters (read-events, parse-batch-*) and exposes them via CLIENT LIST/CLIENT INFO.
  • Tracks eventloop cycles that observed client input-buffer processing using an atomic snapshot in beforeSleep().
  • Resets the new server-level counters on CONFIG RESETSTAT.
  • Adds/updates unit tests to validate INFO fields, reset behavior, and IO-thread one-by-one command accounting.

🤖 Was this summary useful? React with 👍 or 👎

Copy link
Copy Markdown

@augmentcode augmentcode Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. 1 suggestion posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Comment thread src/server.c
@sundb sundb force-pushed the udi/RED-188967-OSS-backport-active-clients-stats-renamed-counters branch from 6321c9c to d174f15 Compare April 2, 2026 08:46
@sundb sundb added release-notes indication that this issue needs to be mentioned in the release notes state:needs-doc-pr requires a PR to redis-doc repository labels Apr 2, 2026
@sundb sundb changed the title Udi/red 188967 oss backport active clients stats renamed counters rename active clients stats counters Apr 2, 2026
@sundb sundb changed the title rename active clients stats counters Rename active clients stats counters Apr 2, 2026
@sundb sundb requested a review from oranagra April 2, 2026 08:50
@sundb sundb changed the title Rename active clients stats counters Rename active clients stats fields Apr 10, 2026
@oranagra
Copy link
Copy Markdown
Member

@udi-speedb why rename these now? please note that in ROF it's already in use and part of grafana dashboards. renaming will cause compatibility issues between versions

@udi-speedb
Copy link
Copy Markdown
Contributor Author

@udi-speedb why rename these now? please note that in ROF it's already in use and part of grafana dashboards. renaming will cause compatibility issues between versions

The rename was meant to better reflect what's measured — commands parsed per processInputBuffer pass, not really a "pipeline length" (a client can send N commands in one read without pipelining, and a pipelined batch can be split across reads).

But you're right that breaking existing dashboards for a cosmetic win isn't worth it. I'll drop the commit and keep the original names.

@oranagra
Copy link
Copy Markdown
Member

i can argue that it is the pipeline from redis's command processing perspective 🤷
it can still help us understand the client better since a client that sends only 1 command, will indicate 1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes indication that this issue needs to be mentioned in the release notes state:needs-doc-pr requires a PR to redis-doc repository

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants