fix(zookeeper): preserve snapshot reads under contention - #11387
Conversation
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused implementation matches the stated consistency model and is covered by deterministic tests.
Review effort: Balanced
Findings: None
What changed in this PR
Removes the arbitrary ZooKeeper snapshot retry limit so reads tolerate sustained finite membership updates while remaining cancellation-bound.
Changes:
- Retry inconsistent snapshot fences until stability or cancellation.
- Add coverage for more than five updates and perpetual churn cancellation.
- Keep cleanup contention retries separately bounded.
| File | Description |
|---|---|
src/Orleans.Clustering.ZooKeeper/ZooKeeperBasedMembershipTable.cs |
Makes snapshot retries cancellation-bound. |
test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperBasedMembershipTableUnitTests.cs |
Verifies eventual stability and cancellation behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Code coverage
Report-only conclusion: improved. The current-main baseline is commit Coverage combines every CI test matrix job, including providers, CodeGen, .NET 8/10, Linux, Windows, and macOS, using canonical physical source and branch identities. The comparison remains report-only while normal line and branch variance is calibrated. Coverage details |
Problem
ZooKeeper membership reads use the deployment node's data and child versions to fence a consistent snapshot. The sequential-read resilience change in #11338 bounded fence retries to five passes, so finite concurrent membership updates could legitimately advance the version more than five times and cause a read to fail despite forward progress.
Solution
Retry a snapshot pass whenever its opening and closing fences differ, continuing until the fence is stable or the caller cancels. Preserve the existing operation-owned session, sequential row reads, connection-loss retry policy, and separate bounded cleanup conflict policy.
Update the native-operation coverage to prove that a read succeeds after more than five canonical updates and that perpetual canonical updates stop at caller cancellation.
Rationale
A fence mismatch proves that a canonical membership transaction committed during the pass; it is not a failed native read. An attempt count is unrelated to snapshot validity and can reject valid finite progress. Caller cancellation provides the liveness boundary while the version fence preserves consistency.
Fixes #11378
Microsoft Reviewers: Open in CodeFlow