Reduce Redis proxy drops and Raft snapshot load#1153
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (15)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
TLA+ spec divergence review (auto-triggered)This PR touches files that the TLA+ safety spec has an anchor on (per Anchored files changed in this PR head (a30bf25):
What to check, by subsystem:
If the change is correct but requires a spec update, edit @claude review please verify TLA+ spec divergence per the checklist above. @codex review please verify TLA+ spec divergence per the checklist above. |
|
To use Codex here, create a Codex account and connect to github. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 657681946a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Prewrite cleanup runs on the snapshot receive path. Avoid a full payload | ||
| // CRC here; startup and actual restore still do full verification before | ||
| // trusting the snapshot bytes. | ||
| return fsmSnapshotFooterMatchesToken(fsmSnapPath(fsmSnapDir, index), tok.CRC32C) == nil |
There was a problem hiding this comment.
Don't mark footer-only snapshots restorable
When prewrite cleanup sees multiple snapshot pairs, keepRestorablePrewriteSnapshots/purgeUnretainedPrewriteSnapshots keep only the newest candidate considered restorable and may delete older pairs. With this footer-only check, a .fsm whose payload is corrupt but whose 4-byte footer still matches the token is treated as restorable; cleanup can then keep that corrupt newest pair and purge the previous valid snapshot, leaving startup/restore to fail full CRC verification with no fallback. If the receive path must avoid a full CRC, avoid using the footer-only result as the retention signal that permits deleting older verified snapshots.
Useful? React with 👍 / 👎.
| const ( | ||
| defaultPoolSize = 128 | ||
| defaultElasticKVPoolSize = 4 | ||
| defaultElasticKVPoolSize = 64 |
There was a problem hiding this comment.
Leave headroom under the server connection cap
The ElasticKV backend pool now defaults to the same value as the server's default per-peer cap (defaultRedisPerPeerConnectionCap is also 64). In deployments where this pool is saturated, any Pub/Sub session against ElasticKV calls RedisBackend.NewPubSub, which opens a dedicated connection outside the pool, so the 65th connection from the same proxy host is rejected with ERR max connections per client exceeded. Keep the default pool below the cap or raise the server cap alongside it so subscribing/shadow Pub/Sub clients do not reintroduce connection drops under load.
Useful? React with 👍 / 👎.
Summary
Tests