valkey
valkey copied to clipboard
Introduce CLUSTER SLOT-STATS command (#20).
The command provides detailed slot usage statistics upon invocation, with initial support for key-count metric. cpu-usec (approved) and memory-bytes (pending-approval) metrics will soon follow after the merger of this PR.
Run:
git rebase HEAD~1 --signoff
git push origin --force 20-slotstats
To fix the DCO complaint.
Thanks for sharing. Two changes added;
- Fixed DCO complaint through
--signoff - Added trailing new line for
cluster_slots.c
@PingXie I think you asked for this in the past, you should also take a look if you have time.
Codecov Report
Attention: Patch coverage is 87.36842% with 12 lines in your changes missing coverage. Please review.
Project coverage is 70.12%. Comparing base (
ce79539) to head (3cf86ab). Report is 10 commits behind head on unstable.
Additional details and impacted files
@@ Coverage Diff @@
## unstable #351 +/- ##
============================================
+ Coverage 70.05% 70.12% +0.06%
============================================
Files 110 111 +1
Lines 60084 60203 +119
============================================
+ Hits 42094 42218 +124
+ Misses 17990 17985 -5
| Files | Coverage Δ | |
|---|---|---|
| src/commands.def | 100.00% <ø> (ø) |
|
| src/server.h | 100.00% <ø> (ø) |
|
| src/cluster_slot_stats.c | 87.36% <87.36%> (ø) |
Review this during a separate meeting.
- High level approval for the approach of adding per-slot statistics.
- Start with CPU, key count, network bytes in, network bytes out.
- Defer the decision about memory usage since it was contentious.
Still need alignment about the command arguments. Will have separate approval for that.
I've cut a 2nd revision, which includes;
- Renamed
cluster_slots.ctocluster_slot_stats.c - Renamed function signature and variables (ex:
*slots-->*assigned_slots). - Removed 0 argument support (just
CLUSTER SLOT-STATS).
@PingXie @madolson I did not update the existing bytes array into bitmaps, based on the performance testing result attached here. Unless there's a strong opinion and/or hard evidence otherwise, I'd prefer moving forward with what we have now.
@kyle-yh-kim Can you do a git merge and resolve the conflicts. Also please open a PR like https://github.com/valkey-io/valkey-doc/pull/143 to add documentation for it.
PR for Valkey-doc has been opened; https://github.com/valkey-io/valkey-doc/pull/150