Skip to content

Add ability to client to run other workloads than the connection manager #174

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

adam-fowler
Copy link
Collaborator

  • Add protocol ActionRunner which runs actions inside discarding task group
  • Conform ValkeyClient to ActionRunner
  • Add single action runClient which runs a ValkeyNodeClient

Copy link

github-actions bot commented Aug 4, 2025

✅ Pull request no significant performance differences ✅

Summary

New baseline 'pull_request' is WITHIN the 'main' baseline thresholds.

Full Benchmark Comparison

Comparing results between 'main' and 'pull_request'

Host '3f15f5b6a4bf' with 4 'x86_64' processors with 15 GB memory, running:
#18~24.04.1-Ubuntu SMP Sat Jun 28 04:46:03 UTC 2025

ValkeyBenchmarks

Client: GET benchmark metrics

Malloc (total): results within specified thresholds, fold down for details.

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 78 79 80 81 82 82 82 6
pull_request 78 78 80 81 82 82 82 6
Δ 0 -1 0 0 0 0 0 0
Improvement % 0 1 0 0 0 0 0 0

Client: GET benchmark | parallel 20 | 20 concurrent connections metrics

Malloc (total): results within specified thresholds, fold down for details.

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 93 97 98 101 102 104 104 25
pull_request 91 96 97 100 104 106 106 25
Δ -2 -1 -1 -1 2 2 2 0
Improvement % 2 1 1 1 -2 -2 -2 0

Connection: GET benchmark metrics

Malloc (total): results within specified thresholds, fold down for details.

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 4 4 4 4 4 4 4 8
pull_request 4 4 4 4 4 4 4 9
Δ 0 0 0 0 0 0 0 1
Improvement % 0 0 0 0 0 0 0 1

Connection: Pipeline benchmark metrics

Malloc (total): results within specified thresholds, fold down for details.

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 36 37 37 37 37 37 37 5
pull_request 37 37 37 37 38 38 38 5
Δ 1 0 0 0 1 1 1 0
Improvement % -3 0 0 0 -3 -3 -3 0

HashSlot – {user}.whatever metrics

Malloc (total): results within specified thresholds, fold down for details.

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 0 0 0 0 0 0 0 19
pull_request 0 0 0 0 0 0 0 18
Δ 0 0 0 0 0 0 0 -1
Improvement % 0 0 0 0 0 0 0 -1

ValkeyCommandEncoder – Command with 7 words metrics

Malloc (total): results within specified thresholds, fold down for details.

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 0 0 0 0 0 0 0 753
pull_request 0 0 0 0 0 0 0 739
Δ 0 0 0 0 0 0 0 -14
Improvement % 0 0 0 0 0 0 0 -14

ValkeyCommandEncoder – Simple GET metrics

Malloc (total): results within specified thresholds, fold down for details.

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 0 0 0 0 0 0 0 1908
pull_request 0 0 0 0 0 0 0 1888
Δ 0 0 0 0 0 0 0 -20
Improvement % 0 0 0 0 0 0 0 -20

ValkeyCommandEncoder – Simple MGET 15 keys metrics

Malloc (total): results within specified thresholds, fold down for details.

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 0 0 0 0 0 0 0 358
pull_request 0 0 0 0 0 0 0 360
Δ 0 0 0 0 0 0 0 2
Improvement % 0 0 0 0 0 0 0 2

@adam-fowler adam-fowler requested a review from fabianfett August 4, 2025 15:19
@adam-fowler
Copy link
Collaborator Author

Don't know if the protocol is overkill, but it could also be used for the cluster client

Copy link
Collaborator

@fabianfett fabianfett left a comment

Choose a reason for hiding this comment

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

Would you mind outlining your motivation for this change? I'm sure this will help me understand this change.

@adam-fowler
Copy link
Collaborator Author

adam-fowler commented Aug 4, 2025

Would you mind outlining your motivation for this change? I'm sure this will help me understand this change.

To support replica discovery, running replicas connection pools and failover/redirect on a standalone client.
Replica support for standalone mode, will probably need a few of the components from your cluster implementation, so I might be breaking a few other pieces of code in the next few weeks.

@adam-fowler adam-fowler requested a review from fabianfett August 7, 2025 08:16
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.

2 participants