Throttle background maintenance under pressure#1072
Conversation
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 (a20162e):
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. |
📝 WalkthroughWalkthroughマルチピアRaftリーダーのFSMコンパクションを抑制し、ロック解決器にグループ別スキャンカーソル、予算超過時のバックオフ、work context に基づく判定を追加しています。 ChangesFSMコンパクション制御
ロック解決制御
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant LockResolver
participant resolveAllGroups
participant resolveGroupLocks
LockResolver->>resolveAllGroups: グループ解決を開始
resolveAllGroups->>resolveGroupLocks: 再開位置からロックをスキャン
resolveGroupLocks-->>resolveAllGroups: 次回スキャン位置を更新
resolveAllGroups-->>LockResolver: 解決結果を返却
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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 |
|
To use Codex here, create a Codex account and connect to github. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request optimizes the background lock resolver and compactor. It updates the FSM compactor to skip compaction on multi-peer leaders and enhances the LockResolver by tracking the scan cursor progress and implementing a backoff mechanism when a group's resolution budget is exhausted. This prevents the resolver from repeatedly scanning from the beginning of the key range or hammering saturated paths. Unit tests have been added to verify these new behaviors. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
kv/lock_resolver.go (1)
144-159: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winraft未準備時はカーソルを進めない
advanceLockScanStartがlockResolverRaftReadyより先にあるため、ループ途中で raft が未準備になったときに未処理のkvp.Keyまでスキャン位置が進みます。advanceLockScanStartはresolveScannedLockの直前へ移したいです。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@kv/lock_resolver.go` around lines 144 - 159, Move the advanceLockScanStart call in the lockKVs loop to immediately before resolveScannedLock, after the lockResolverRaftReady check. Ensure the scan cursor is not advanced for the current kvp.Key when Raft is unavailable, while preserving the existing resolution and outcome-counting flow.
🧹 Nitpick comments (1)
kv/compactor_test.go (1)
163-184: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win単一ノードリーダーでコンパクションが維持されることを検証するテストケースを追加してください。
PR目的は「単一ノードリーダーではコンパクションを維持する」ことですが、そのケース（
StateLeader+NumPeersが単一ノードを示す値）でcompactCalledがtrueになることを検証するテストがありません。マルチピアのスキップだけでなく、単一ノードリーダーの保持パスもテストでカバーすべきです。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@kv/compactor_test.go` around lines 163 - 184, Add a test covering the single-node leader path in FSM compaction, using StateLeader with NumPeers set to the single-node value and the existing deadlineCapturingStore setup. Invoke FSMCompactor.SyncOnce and assert that compactCalled is true, while preserving the existing multi-peer skip test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@kv/lock_resolver.go`:
- Around line 144-159: Move the advanceLockScanStart call in the lockKVs loop to
immediately before resolveScannedLock, after the lockResolverRaftReady check.
Ensure the scan cursor is not advanced for the current kvp.Key when Raft is
unavailable, while preserving the existing resolution and outcome-counting flow.
---
Nitpick comments:
In `@kv/compactor_test.go`:
- Around line 163-184: Add a test covering the single-node leader path in FSM
compaction, using StateLeader with NumPeers set to the single-node value and the
existing deadlineCapturingStore setup. Invoke FSMCompactor.SyncOnce and assert
that compactCalled is true, while preserving the existing multi-peer skip test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5d36362f-a3b4-42fd-96fc-3776a4e884f9
📒 Files selected for processing (4)
kv/compactor.gokv/compactor_test.gokv/lock_resolver.gokv/lock_resolver_test.go
1 participant
Summary
Risk
Tests
Summary by CodeRabbit
改善
テスト