-
Notifications
You must be signed in to change notification settings - Fork 490
upgrade radix from v3 to v4 for improved pipeline handling #1041
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
Conversation
src/redis/driver_impl.go
Outdated
| // ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) | ||
| // defer cancel() | ||
| // client.Do(ctx, cmd) | ||
| // - Consider increasing REDIS_POOL_SIZE if using CREATE or ERROR previously |
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.
Why not just return error or panic instead if we detect CREATE or ERROR for REDIS_POOL_ON_EMPTY_BEHAVIOR? Othewise, users who had set REDIS_POOL_ON_EMPTY_BEHAVIOR=ERROR/CREATE will just experience indefinite blocking.
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.
@collin-lee Thank you for reviewing this.
I've updated the logic to panic/return an error when REDIS_POOL_ON_EMPTY_BEHAVIOR is set to CREATE or ERROR.
| // RedisPerSecondSentinelAuth is the password for authenticating to per-second Redis Sentinel nodes. | ||
| // This is separate from RedisPerSecondAuth which is used for authenticating to the Redis master/replica nodes. | ||
| // If empty, no authentication will be attempted when connecting to per-second Sentinel nodes. | ||
| RedisPerSecondSentinelAuth string `envconfig:"REDIS_PERSECOND_SENTINEL_AUTH" default:""` |
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.
REDIS_PERSECOND_SENTINEL_AUTH should be mentioned in README.md too I think
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.
@collin-lee
This PR focuses on radix v4 migration and doesn't change sentinel auth functionality.
And REDIS_PERSECOND_SENTINEL_AUTH is already documented in README.md.
Is there something specific you'd like to see added or clarified?
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.
Thanks for pointing that out. I was just trying to review yesterday and wanted to make sure any newly introduced env variables are mentioned in README.md
Radix v4 Benchmark Results (Redis Single Instance)Comprehensive performance testing comparing Radix v3 (main) vs Radix v4 under a controlled load of 500 RPS. Executive SummaryRadix v4 delivers 53-55% latency reduction compared to the current main branch (Radix v3).
Key ResultsFixed Key ScenarioMixed2 Scenario (1 fixed + 1 variable key)Detailed Comparison TableClick to expand: Complete benchmark resultsFixed Key Scenario (500 RPS)
Improvements (radixv4 Explicit+Write-buffer vs. radix v3 Window):
Mixed2 Scenario (500 RPS)
Improvements (radixv4 Explicit+Write-buffer vs. radix v3 Window):
Test ConfigurationClick to expand: Test environment and settingsTest Environment
gRPC Client ConfigurationWorker count calculation (Little's Law): For 500 RPS at ~10ms expected latency, concurrent requests needed = 500 × 0.01s = 5. Workers = 5 × 4 (headroom) = 20, gRPC connections = 20 / 4 = 5. Redis Configurationmain branch (Radix v3): REDIS_TYPE=single
REDIS_URL=localhost:6379
REDIS_PIPELINE_WINDOW=150us
REDIS_PIPELINE_LIMIT=0 # No explicit limit
REDIS_POOL_SIZE=20
REDIS_POOL_ON_EMPTY_BEHAVIOR=WAIT # To fix pool sizeradixv4 - Write-buffer : REDIS_TYPE=single
REDIS_URL=localhost:6379
REDIS_USE_EXPLICIT_PIPELINE=false
REDIS_PIPELINE_WINDOW=150us
REDIS_POOL_SIZE=20radixv4 - Explicit Pipeline + Write-buffer : REDIS_TYPE=single
REDIS_URL=localhost:6379
REDIS_USE_EXPLICIT_PIPELINE=true # Key difference
REDIS_PIPELINE_WINDOW=150us
REDIS_POOL_SIZE=20Test Scenarios ExplainedClick to expand: What are "fixed" and "mixed2" scenarios?Scenario 1:
|
|
I've improved performance in Redis Cluster as well by automatically applying pipelining for operations on the same key. Please refer to this commit: 2f597fd Radix v4 Benchmark Results (Redis Cluster 🕸️)Comprehensive performance testing comparing Radix v3 (main) vs Radix v4 in Redis Cluster mode under controlled 500 RPS load. Executive SummaryRadix v4 delivers 35-64% latency reduction across all scenarios compared to main branch in Redis Cluster deployments.
Key Results
Detailed Comparison TableClick to expand: Complete benchmark resultsFixed Key Scenario (500 RPS)
Improvements:
Mixed2 Scenario (500 RPS)
Improvements:
Mixed10 Scenario (Note: RPS limited by workload complexity)
Improvements:
Note: mixed10 scenario could not sustain 500 RPS due to workload complexity (10 keys per request). Radix v4 shows significant throughput improvement (+50%) in addition to latency reduction. Test ConfigurationClick to expand: Test environment and settingsTest Environment
gRPC Client ConfigurationRedis Configurationmain branch (Radix v3): REDIS_TYPE=cluster
REDIS_URL=localhost:7001,localhost:7002,localhost:7003
REDIS_PIPELINE_WINDOW=150us
REDIS_PIPELINE_LIMIT=8
REDIS_POOL_SIZE=10radixv4: REDIS_TYPE=cluster
REDIS_URL=localhost:7001,localhost:7002,localhost:7003
REDIS_PIPELINE_WINDOW=150us
REDIS_POOL_SIZE=10
# Note: REDIS_PIPELINE_LIMIT removed (deprecated in v4)Test Scenarios ExplainedClick to expand: Scenario descriptionsScenario 1:
|
Upgrade radix Redis client from v3.8.1 to v4.1.4. Main changes: - Import paths: radix/v3 -> radix/v4 - Pool/Cluster/Sentinel use Config.New() instead of New() - All client operations require context.Context parameter - Dialer setup changed from functional options to struct config - Pipelining uses radix.NewPipeline() and Append() - Write buffering via Dialer.WriteFlushInterval Breaking from v3: - Pool on-empty behavior (WAIT/CREATE/ERROR) not available - REDIS_PIPELINE_LIMIT setting deprecated (no effect in v4) Tested with existing test suite - all tests passing. Signed-off-by: seonghyun <[email protected]>
Update documentation to reflect radix v4's pipeline behavior: - REDIS_PIPELINE_WINDOW now sets WriteFlushInterval (auto-flush timing) - REDIS_PIPELINE_LIMIT deprecated - no effect in v4 - Add REDIS_USE_EXPLICIT_PIPELINE for manual pipeline control - Required for Redis Cluster: PIPELINE_WINDOW must be non-zero Update terminology from "implicit pipelining" to "write buffering" to better match radix v4's actual behavior. Signed-off-by: seonghyun <[email protected]>
- Add useExplicitPipeline parameter to test client creation - Update error assertions for v4's error message format (v4 prefixes with "response returned from Conn:") - Handle different connection errors (EOF, connection reset, broken pipe) - Update radix.FlatCmd usage for v4 API Signed-off-by: seonghyun <[email protected]>
Signed-off-by: seonghyun <[email protected]>
Replace deprecated RedisPipelineLimit with RedisPipelineWindow in configRedisCluster function. Radix v4 requires WriteFlushInterval (RedisPipelineWindow) for cluster mode buffering instead of the deprecated pipeline limit setting. Signed-off-by: seonghyun <[email protected]>
Signed-off-by: seonghyun <[email protected]>
Radix v4 does not support CREATE or ERROR behaviors for REDIS_POOL_ON_EMPTY_BEHAVIOR. Previously, these settings were logged as errors but the application would continue with blocking behavior, which could cause unexpected issues in production. Changes: - Panic at startup when CREATE or ERROR is detected - Prevent silent behavior changes that could cause blocking - Update tests to verify panic behavior - Improve migration documentation in comments This ensures users are immediately notified of incompatible configuration rather than experiencing unexpected blocking in production. Signed-off-by: seonghyun <[email protected]>
The default value 'CREATE' is not supported in radix v4 and causes integration tests to panic at startup. Changed default to 'WAIT' which matches radix v4's actual pool behavior (always blocks when empty). This fixes integration test failures where tests without explicit REDIS_POOL_ON_EMPTY_BEHAVIOR settings would panic during initialization with: "REDIS_POOL_ON_EMPTY_BEHAVIOR=CREATE is not supported in radix v4" Also updated documentation to clarify that CREATE/ERROR are not supported and marked RedisPoolOnEmptyWaitDuration as deprecated. Signed-off-by: seonghyun <[email protected]>
- Fix WaitForTcpPort to use timeoutCtx instead of ctx This ensures the timeout parameter is actually respected when dialing TCP connections. - Increase gRPC server startup timeout from 1s to 10s Radix v4 cluster connection initialization takes longer, especially when establishing connections to multiple cluster nodes. This prevents "connection refused" errors in integration tests. Signed-off-by: seonghyun <[email protected]>
Consolidates Redis and Sentinel dialer setup into a reusable createDialer helper function, eliminating ~30 lines of duplicated code. Improves logging by including connection target details (e.g., "sentinel(master,host1,host2)") instead of generic "sentinel" string. Signed-off-by: seonghyun <[email protected]>
Remove the deprecated poolOnEmptyWaitDuration parameter and related configuration settings as they have no effect in radix v4. The pool always blocks until a connection is available when using WAIT behavior. Signed-off-by: seonghyun <[email protected]>
Remove REDIS_USE_EXPLICIT_PIPELINE configuration option and automatically determine pipeline mode based on Redis deployment type: - Cluster mode: uses grouped pipeline (groups same-key commands) - INCRBY + EXPIRE for same key are pipelined together (same slot) - Reduces round-trips from 2 to 1 per key in cluster mode - Single/Sentinel mode: uses explicit pipeline (batches all commands) - All commands in one pipeline for minimal latency - Optimal for non-cluster deployments This simplifies configuration by removing user-facing options while automatically choosing the optimal pipeline strategy for each Redis type. Breaking changes: - Remove REDIS_USE_EXPLICIT_PIPELINE env var - Remove REDIS_PERSECOND_USE_EXPLICIT_PIPELINE env var - Remove UseExplicitPipeline() interface method Signed-off-by: seonghyun <[email protected]>
The REDIS_USE_EXPLICIT_PIPELINE and REDIS_PERSECOND_USE_EXPLICIT_PIPELINE settings were documented in README but do not exist in settings.go. Removed the documentation to match the actual implementation. Signed-off-by: seonghyun <[email protected]>
Signed-off-by: seonghyun <[email protected]>
|
@collin-lee |
collin-lee
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.
LGTM
FYI @arkodg
|
@collin-lee |

Summary
This PR upgrades the Redis client library from radix v3 to radix v4.
radix v3 has entered maintenance mode (only accepting bug fixes), and v4 introduces several key improvements:
context.Contextsupport on all blocking operationsChanges
Core Changes
github.com/mediocregopher/radix/v3togithub.com/mediocregopher/radix/v4radix.DialOpt,radix.PoolOpt) to struct-based configuration (radix.Dialer,radix.PoolConfig)context.Contextparameter to all Redis operations (required in v4)FlatCmdcall signature to align with the v4 APIPipeline Behavior Changes
Pipeline mode is now automatically selected based on Redis deployment type:
Cluster mode:
Single/Sentinel mode:
Deprecated:
REDIS_PIPELINE_LIMIT- no longer has any effect (warning logged if set)Pool Behavior Changes
radix v4 uses a fixed-size pool that blocks when exhausted. Note the changes in
REDIS_POOL_ON_EMPTY_BEHAVIOR:WAITCREATEERRORRemoved settings:
REDIS_POOL_ON_EMPTY_WAIT_DURATION- No longer used since v4 always blocksREDIS_PERSECOND_POOL_ON_EMPTY_WAIT_DURATION- Same as aboveBreaking Changes
REDIS_PIPELINE_LIMITis deprecated - No longer has any effect (warning logged if set)implicit pipelineis removed - Pipeline mode is now automatic based on Redis typeREDIS_POOL_ON_EMPTY_WAIT_DURATIONremoved - No longer needed in v4REDIS_POOL_ON_EMPTY_BEHAVIOR=CREATE, increaseREDIS_POOL_SIZEinsteadMigration Guide
Most users need no changes. If you have custom settings:
Remove deprecated settings:
# Remove these if present REDIS_PIPELINE_LIMIT=8 REDIS_POOL_ON_EMPTY_WAIT_DURATION=10sCluster mode continues to work:
REDIS_TYPE=cluster REDIS_PIPELINE_WINDOW=150us # Still supported