-
Notifications
You must be signed in to change notification settings - Fork 2
Tune redis proxy connection capacity #1147
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
Changes from 3 commits
30004df
e4ce5d0
41be10a
fa52ca7
1f3b80d
b6f5ebf
2607b32
6c805f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,9 +26,11 @@ Implementation status: | |
| `adapter/redis_peer_limiter.go`, wired through `RedisServer.Run` accept and | ||
| close hooks. Default cap is 8 per peer IP and is configurable via | ||
| `ELASTICKV_REDIS_PER_PEER_CONNECTIONS` / | ||
| `WithRedisPerPeerConnectionLimit`. Redis leader-proxy clients use a small | ||
| explicit go-redis pool below that default cap, and Pub/Sub detached sockets | ||
| stay counted until their Pub/Sub cleanup path closes. | ||
| `WithRedisPerPeerConnectionLimit`. The redis-proxy deployment can raise the | ||
| cap explicitly on ElasticKV nodes before increasing the proxy's ElasticKV | ||
| pool size. Redis leader-proxy clients use a small explicit go-redis pool | ||
| below that default cap, and Pub/Sub detached sockets stay counted until their | ||
| Pub/Sub cleanup path closes. | ||
| - Shipped: Layer 4 stream entry-per-key layout in `store/stream_helpers.go`, | ||
| `adapter/redis_stream_cmds.go`, and `adapter/redis_compat_helpers.go`. | ||
| XREAD now range-scans `!stream|entry|...` after the requested ID instead of | ||
|
|
@@ -374,7 +376,7 @@ one check per accept, not per command. | |
|
|
||
| ### Recommended v1 shape | ||
|
|
||
| **Per-peer-IP connection cap, default `N=8`, env-configurable, | ||
| **Per-peer-IP connection cap, default `N=64`, env-configurable, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The implementation still sets Useful? React with 👍 / 👎. |
||
| enforced at accept.** On reject, accept the TCP connection, write a | ||
| `-ERR max connections per client exceeded` RESP error, then close — | ||
| so the client sees a protocol-level message instead of a bare | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -414,7 +414,7 @@ groups: | |
| | Parameter | Value | Description | | ||
| |-----------|-------|-------------| | ||
| | Redis connection pool size | 128 | Default go-redis pool size for Redis | | ||
| | ElasticKV connection pool size | 4 | Default per-leader pool; keep within the server per-peer connection limit | | ||
| | ElasticKV connection pool size | 4 | Default per-leader pool; keep within the server per-peer connection limit and leave room for dedicated sockets | | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 実装のデフォルト値とドキュメントをPRの目的に揃えてください。 この表はElasticKVのデフォルトpool sizeを4と記載していますが、PR目的は4から16へのデフォルト増加です。現在の 🤖 Prompt for AI Agents |
||
| | Dial timeout | 5s | Backend connection timeout | | ||
| | Read timeout | 3s | Backend read timeout | | ||
| | Write timeout | 3s | Backend write timeout | | ||
|
|
@@ -439,7 +439,8 @@ Recommended shutdown order: `redis-proxy -> application -> Redis / ElasticKV`. | |
| ### Secondary writes are falling behind | ||
| - Check `proxy_async_queue_depth`, `proxy_async_queue_delay_seconds`, and `proxy_async_drops_by_queue_total` before increasing concurrency. | ||
| - Check `proxy_backend_pool_pending_requests` and the `waits`/`timeouts` pool events. Pool waits mean concurrency is too high for the configured pool. | ||
| - Increase the ElasticKV pool only together with `ELASTICKV_REDIS_PER_PEER_CONNECTIONS`; keep `-secondary-write-concurrency` at or below the pool size. | ||
| - Increase the ElasticKV pool only together with `ELASTICKV_REDIS_PER_PEER_CONNECTIONS` when the proxy pool can exceed the server's per-peer cap; keep `-secondary-write-concurrency` at or below the pool size. | ||
| - For the production proxy shape that needs more backend connections, first deploy ElasticKV nodes with `ELASTICKV_REDIS_PER_PEER_CONNECTIONS=64`, then raise the proxy with `REDIS_PROXY_ELASTICKV_POOL_SIZE=16` or an explicit `-elastickv-pool-size=16`, keeping `-secondary-write-concurrency` below the pool size, for example `8`. Do not roll a larger proxy pool before the server-side cap is active on every node. | ||
| - A sustained `expired` rate means secondary throughput is below ingress. Increasing queue size only delays the loss; profile ElasticKV before raising concurrency. | ||
|
|
||
| ### High divergence count | ||
|
|
||
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.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
「legacy cap」の期待値を上限8に合わせてください。
limit: 8のケースでデフォルトのプールサイズを期待しているため、プール合計が8を超える構成では Line 113 の上限検証に必ず失敗します。旧上限の回帰ケースは通常4・ブロッキング4(または上限から算出した値)を期待し、現在のデフォルト値の検証は別ケースに分離してください。修正例
📝 Committable suggestion
🤖 Prompt for AI Agents