Skip to content
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

feat: support histograms for command latency statistics #2721

Open
wants to merge 17 commits into
base: unstable
Choose a base branch
from

Conversation

rabunkosar-dd
Copy link

@rabunkosar-dd rabunkosar-dd commented Jan 14, 2025

This PR adds an option to build kvrocks with histogram support to breakdown the command latencies. Current statistics are based on the total latency and the command count, giving averages. For our use case, where tail latencies are very impactful, this was not sufficient to track the general performance of the instances.

It also adds a config parameter to define the bucket boundaries. I will also follow up with a PR to add similar support in kvrocks_exporter (and support additive histograms for prometheus).

The buckets are not additive and there is an implicit bucket that tracks values beyond the defined values (inf bucket).

I added a build paramater (ENABLE_HISTOGRAMS), when set to ON will compile the support into the binary.
The feature is enabled if the config parameter defines a non-empty list of buckets that are coma separated

@rabunkosar-dd rabunkosar-dd changed the title (feat) support histograms for command latency statistics feat (stats) support histograms for command latency statistics Jan 14, 2025
@rabunkosar-dd rabunkosar-dd changed the title feat (stats) support histograms for command latency statistics feat(stats) support histograms for command latency statistics Jan 14, 2025
@rabunkosar-dd rabunkosar-dd changed the title feat(stats) support histograms for command latency statistics feat: support histograms for command latency statistics Jan 14, 2025
@git-hulk
Copy link
Member

git-hulk commented Jan 15, 2025

@rabunkosar-dd Thanks for your contribution. You use ./x.py format to format your source codes after installing the clang-format-14

CMakeLists.txt Outdated Show resolved Hide resolved
Copy link
Member

@git-hulk git-hulk left a comment

Choose a reason for hiding this comment

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

Looks good to me, one comment inline.

src/stats/stats.cc Outdated Show resolved Hide resolved
src/config/config.cc Outdated Show resolved Hide resolved
src/stats/stats.cc Outdated Show resolved Hide resolved
src/stats/stats.h Outdated Show resolved Hide resolved
@git-hulk git-hulk requested a review from PragmaTwice January 17, 2025 09:57
@git-hulk
Copy link
Member

@rabunkosar-dd Could you please fix the lint issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants