-
Notifications
You must be signed in to change notification settings - Fork 15
Custom responses and Standard Error types for cluster commands #260
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: main
Are you sure you want to change the base?
Conversation
5023075 to
2bc45cb
Compare
Signed-off-by: Nilanshu Sharma <[email protected]>
Signed-off-by: Nilanshu Sharma <[email protected]>
2bc45cb to
cbe091e
Compare
|
✅ Pull request no significant performance differences ✅ SummaryNew baseline 'pull_request' is WITHIN the 'main' baseline thresholds. Full Benchmark ComparisonComparing results between 'main' and 'pull_request'ValkeyBenchmarksClient: GET benchmark metricsMalloc (total): results within specified thresholds, fold down for details.
Client: GET benchmark | parallel 20 | 20 concurrent connections metricsMalloc (total): results within specified thresholds, fold down for details.
Connection: GET benchmark metricsMalloc (total): results within specified thresholds, fold down for details.
Connection: GET benchmark – NoOpTracer metricsMalloc (total): results within specified thresholds, fold down for details.
Connection: Pipeline array benchmark metricsMalloc (total): results within specified thresholds, fold down for details.
Connection: Pipeline benchmark metricsMalloc (total): results within specified thresholds, fold down for details.
HashSlot – {user}.whatever metricsMalloc (total): results within specified thresholds, fold down for details.
ValkeyCommandEncoder – Command with 7 words metricsMalloc (total): results within specified thresholds, fold down for details.
ValkeyCommandEncoder – Simple GET metricsMalloc (total): results within specified thresholds, fold down for details.
ValkeyCommandEncoder – Simple MGET 15 keys metricsMalloc (total): results within specified thresholds, fold down for details.
|
Signed-off-by: Nilanshu Sharma <[email protected]>
Signed-off-by: Nilanshu Sharma <[email protected]>
Signed-off-by: Nilanshu Sharma <[email protected]>
Signed-off-by: Nilanshu Sharma <[email protected]>
adam-fowler
left a comment
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.
Hi @nilanshu-sharma I've had a quick look at this on my phone. Hard to do a proper review on the phone, but see my comments related to errors thrown.
adam-fowler
left a comment
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.
A quick review from my iPhone
Signed-off-by: Nilanshu Sharma <[email protected]>
|
Thanks @adam-fowler. I have addressed your comments. |
868f4b3 to
deeae68
Compare
Change Summary:
Adding custom responses for the following commands:
Converting error types for all Cluster Commands to
RESPDecodeErrorRemoving docker network to make cluster-mode valkey directly accessible to integration tests while testing locally on laptop.
Restoring platform availability macros causing builds to fail
Also realized that the Integration tests meant to cluster-mode features are not running in the CI, so CI needs to be enhanced to run cluster mode related docker dependencies. For now the verification of new integration tests is done locally.