-
Notifications
You must be signed in to change notification settings - Fork 491
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
[NEW] Introduce slot level metrics to Redis cluster #20
Comments
I fully agree! |
I've also re-added my favorite creature comfort. @placeholderkv/core-team thoughts? |
Sure, metrics seem fine. I don't have strong opinions about it, only that I think fixing the cluster consistency problems is more important than metrics. |
I think our big initial play should be cluster overhaul. I think a lot of us want it, and it makes the most compelling sense as the big "next major features". |
Good to hear back on this thread, hope you all have been doing well. Where we left-offIn total, there were 3 proposed metrics under
Next steps
I will open two PRs for As for
|
It is great, and I prefer to add this feature in CLUSTER INFO. |
Why cluster info? It's a free form field I guess, it could be a new sub info field I suppose |
Thanks for chiming in. Personally, I'm opposed to Imagine dumping ~16384 slot level metrics under A new command dedicated for slot level metrics querying, in this case,
I'll wait for the core team to finalize this decision, before opening the PRs. |
@kyle-yh-kim Yeah CLUSTER SLOT-STATS. We're a bit overloaded with the forking stuff, new core team, new project, etc. but I think we want this for our next release. There was already a lot of review done and I think it was almost ready to merge. Do you want to bring over your PR? |
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.
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.
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.
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.
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.
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.
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.
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.
Ignore my spam references above, I was reviewing the diff manually over Github UI. PR has now been opened; #351 This PR is part one of the three upcoming PRs;
|
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. Signed-off-by: Kyle Kim <kimkyle@amazon.com>
Moving ahead, I would like to resume our conversation on per-slot memory metrics. I'd argue this is the most important per-slot metric of all, as it enables for smoother horizontal scaling given the accurate memory tracking at per-slot granularity. Last time, we converged on its high-level strategy in "online analysis" (amortizing memory tracking cost per mutative-command, over offline RDB snapshot analysis / forking a process), as well as its performance and memory impact. The following conclusion was drawn, before the issue was then put on halt by the previously open-sourced Redis-core team.
As for module consideration, I mention in details here to keep this feature as an opt-in service to maintain backwards compatibility. For opt-in modules, they will be required to accurately track its value size, and call a newly introduced hook If we are still aligned to this strategy, I will start on its implementation, and open incremental PRs following the merger of the above |
I’m revisiting the feature proposal we discussed in redis/redis#10472, which aims at providing metrics at the slot level. Despite the substantial effort and detailed discussions back then, we didn’t land this feature. I believe it’s worth reconsidering, given the potential benefits and previous interest.
@kyle-yh-kim @zuiderkwast @madolson
The text was updated successfully, but these errors were encountered: