-
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
base: main
Are you sure you want to change the base?
Changes from all commits
30004df
e4ce5d0
41be10a
fa52ca7
1f3b80d
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 |
|---|---|---|
|
|
@@ -72,9 +72,10 @@ func DefaultBackendOptions() BackendOptions { | |
| } | ||
|
|
||
| // DefaultElasticKVBackendOptions returns defaults for proxy backends that | ||
| // connect to ElasticKV's Redis adapter. ElasticKV limits concurrent Redis | ||
| // connections per peer by default, so keep the pool below that cap unless the | ||
| // operator also raises ELASTICKV_REDIS_PER_PEER_CONNECTIONS on the cluster. | ||
| // connect to ElasticKV's Redis adapter. Keep the default within ElasticKV's | ||
| // server-side per-peer cap while leaving room for dedicated Pub/Sub and | ||
| // blocking-command sockets. Operators can still raise this together with | ||
| // ELASTICKV_REDIS_PER_PEER_CONNECTIONS after the cluster is configured for it. | ||
|
Comment on lines
+75
to
+78
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. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 接続予算の説明が実際の共有上限を誤って示しています。
🤖 Prompt for AI Agents |
||
| func DefaultElasticKVBackendOptions() BackendOptions { | ||
| opts := DefaultBackendOptions() | ||
| opts.PoolSize = defaultElasticKVPoolSize | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,8 +75,8 @@ type LeaderAwareRedisBackend struct { | |
| } | ||
|
|
||
| // NewLeaderAwareRedisBackend creates a LeaderAwareRedisBackend with the given | ||
| // seed addresses. The first seed is used as the initial target until the | ||
| // first refresh completes. At least one seed is required. | ||
| // seed addresses. The first command waits for leader discovery instead of | ||
| // sending traffic to a seed that may be down. At least one seed is required. | ||
| func NewLeaderAwareRedisBackend(seeds []string, name string, opts BackendOptions, logger *slog.Logger) *LeaderAwareRedisBackend { | ||
| return NewLeaderAwareRedisBackendWithInterval(seeds, name, opts, defaultLeaderRefreshInterval, defaultLeaderRefreshTimeout, logger) | ||
| } | ||
|
|
@@ -106,7 +106,7 @@ func NewLeaderAwareRedisBackendWithInterval(seeds []string, name string, opts Ba | |
| clients: make(map[string]*redis.Client, len(normalized)), | ||
| clientOrder: make([]string, 0, len(normalized)), | ||
| seedProtect: seedProtect, | ||
| leader: normalized[0], | ||
| leader: "", | ||
| stopCh: make(chan struct{}), | ||
| done: make(chan struct{}), | ||
| refreshCh: make(chan struct{}, 1), | ||
|
|
@@ -389,6 +389,15 @@ func (b *LeaderAwareRedisBackend) currentClient() *redis.Client { | |
| return b.clients[b.leader] | ||
| } | ||
|
|
||
| func (b *LeaderAwareRedisBackend) currentClientOrRefresh(ctx context.Context) *redis.Client { | ||
| cli := b.currentClient() | ||
| if cli != nil { | ||
| return cli | ||
| } | ||
| b.RefreshLeaderNow(ctx) | ||
| return b.currentClient() | ||
| } | ||
|
|
||
| // Do forwards a single command to the current leader. NOTLEADER refreshes the | ||
| // cached leader for the next command, but the current command is not replayed: | ||
| // leadership-loss errors can be returned after an operation has already applied. | ||
|
|
@@ -404,7 +413,7 @@ func (b *LeaderAwareRedisBackend) Do(ctx context.Context, args ...any) *redis.Cm | |
| } | ||
|
|
||
| func (b *LeaderAwareRedisBackend) doOnce(ctx context.Context, args ...any) *redis.Cmd { | ||
| cli := b.currentClient() | ||
| cli := b.currentClientOrRefresh(ctx) | ||
| if cli == nil { | ||
| cmd := redis.NewCmd(ctx, args...) | ||
| cmd.SetErr(ErrNoLeaderBackend) | ||
|
|
@@ -426,7 +435,7 @@ func (b *LeaderAwareRedisBackend) DoWithTimeout(ctx context.Context, timeout tim | |
| } | ||
|
|
||
| func (b *LeaderAwareRedisBackend) doWithTimeoutOnce(ctx context.Context, timeout time.Duration, args ...any) *redis.Cmd { | ||
| cli := b.currentClient() | ||
| cli := b.currentClientOrRefresh(ctx) | ||
| if cli == nil { | ||
| cmd := redis.NewCmd(ctx, args...) | ||
| cmd.SetErr(ErrNoLeaderBackend) | ||
|
|
@@ -437,7 +446,7 @@ func (b *LeaderAwareRedisBackend) doWithTimeoutOnce(ctx context.Context, timeout | |
|
|
||
| // Pipeline forwards a batch to the current leader. | ||
| func (b *LeaderAwareRedisBackend) Pipeline(ctx context.Context, cmds [][]any) ([]*redis.Cmd, error) { | ||
| cli := b.currentClient() | ||
| cli := b.currentClientOrRefresh(ctx) | ||
| if cli == nil { | ||
| return nil, ErrNoLeaderBackend | ||
| } | ||
|
|
@@ -517,7 +526,7 @@ func isLeaderRefreshTransportError(err error) bool { | |
|
|
||
| // NewPubSub opens a subscribe connection on the current leader. | ||
| func (b *LeaderAwareRedisBackend) NewPubSub(ctx context.Context) *redis.PubSub { | ||
| cli := b.currentClient() | ||
| cli := b.currentClientOrRefresh(ctx) | ||
|
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.
When no leader can be discovered at startup or during an election, Useful? React with 👍 / 👎. |
||
| if cli == nil { | ||
| return nil | ||
| } | ||
|
|
||
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 | 🟠 Major | ⚡ Quick win
実装のデフォルト値とドキュメントをPRの目的に揃えてください。
この表はElasticKVのデフォルトpool sizeを4と記載していますが、PR目的は4から16へのデフォルト増加です。現在の
proxy/backend.goのdefaultElasticKVPoolSizeも4のため、明示的な設定なしでは実行時のデフォルトは変わらず、派生write concurrencyも2のままです。定数とドキュメントを16へ更新するか、4が意図的ならPRの目的とテスト説明を修正してください。🤖 Prompt for AI Agents