You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
• Prioritizes announced KPlane leaders in the shared endpoint list.
• Atomically switches value-equivalent endpoints without overwriting concurrent leader changes.
• Releases stale channels only when the effective endpoint changes.
Diagram
graph TD
A["Announce stream"] --> B{"Leader announced?"}
B -->|Yes| C["Select leader"] --> D["Leader-first list"]
B -->|No| D
D -->|Redirected| E["Reconnect stream"]
D -->|Stable| F["Yield cluster"]
D --> G["Leader RPCs"] -->|Failure or redirect| H["Atomic failover"] -->|Retry| G
Loading
High-Level Assessment
The scoped lock-free approach fits the existing client design: it fixes value-equivalent endpoint replacement while preserving concurrent updates and prioritizes the leader without introducing broader state management. Lock-based synchronization or a unified immutable routing-state object could simplify reasoning, but would add coordination overhead and require a substantially wider refactor for little benefit here.
Files changed (2) +37 / -8
Enhancement (1) +21 / -2
GrpcKontrolPlaneClient.csPrioritize the resolved leader in Kontrol Plane topology+21/-2
Prioritize the resolved leader in Kontrol Plane topology
• Processes leader redirects before rebuilding the shared node list, ensuring the resolved leader occupies the first position and is not duplicated. The updated topology is retained before reconnecting, and a file-scoped endpoint inequality helper is introduced.
GrpcKontrolPlaneClient.Utils.csMake endpoint switching value-aware and concurrency-safe+16/-6
Make endpoint switching value-aware and concurrency-safe
• Reworks address replacement into a compare-and-swap loop that can replace value-equivalent endpoint instances while preserving competing updates. Uses explicit endpoint equality when deciding whether to release cached channels and when locating the next node.
1. Failover retries the same dead node✓ Resolved🐞 Bug☼ Reliability
Description
GetKontrollerNodes reads addresses[0] on every loop iteration, producing only the leader or
repeated copies of the first advertised endpoint instead of all members. When that endpoint fails,
NextAddress cycles through the malformed list and the appointment operations repeatedly retry an
unavailable node rather than reaching another controller.
Deterministic indexing bug: every iteration converts addresses[0], omitting all subsequent failover
endpoints.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
The helper iterates over the full address count but always converts element zero, while
NextAddress advances only through the resulting _kontrollerNodes list. The server explicitly
populates responses from every controller node, so omitting all elements after the first discards
real failover targets.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
`GetKontrollerNodes` reads index zero during every loop iteration, so peer Kontrol Plane nodes are omitted and failover can retry the same unavailable endpoint indefinitely.
## Fix Focus Areas
- src/KurrentDB.DataPlane/KontrolPlane/Transport/Grpc/GrpcKontrolPlaneClient.cs[79-89]
## Recommended Fix
Convert `addresses[i]` on each iteration instead of `addresses[0]`, retaining the existing leader-first ordering and leader de-duplication. Add coverage proving that multiple distinct advertised endpoints remain available in their expected failover order.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
2. Failover uses a stale node list✓ Resolved🐞 Bug☼ Reliability
Description
AnnounceNodeAsync now assigns _kontrollerNodes only inside the non-empty leader redirect branch,
replacing the previous unconditional assignment from every response. Normal streaming responses
carry an empty leader and a populated membership list, so topology changes remain absent from every
later failover until some redirect occurs.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
The client updates _kontrollerNodes only when KontrollerLeader is non-empty, but the server sets
that field to empty on ordinary cluster responses and still populates KontrollerNodes. All
subsequent endpoint rotation reads _kontrollerNodes, making the ignored membership unavailable to
failover.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
Normal announcement responses contain current Kontrol Plane membership without a redirect, but the client now ignores those lists and continues failing over through stale seed data.
## Fix Focus Areas
- src/KurrentDB.DataPlane/KontrolPlane/Transport/Grpc/GrpcKontrolPlaneClient.cs[53-66]
## Recommended Fix
Update `_kontrollerNodes` for every announcement response. Select the advertised leader when present and otherwise use the current successful streaming endpoint as the leader passed to `GetKontrollerNodes`, preserving leader-first ordering without discarding normal membership updates.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Review mode: ⚖️ Balanced: The push changes concurrent endpoint replacement and control-plane leader address selection, so it carries behavioral and concurrency risk despite being localized.
Tip of the day
💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR
1. Failover retries the same dead node✓ Resolved🐞 Bug☼ Reliability
Description
GetKontrollerNodes reads addresses[0] on every loop iteration, producing only the leader or
repeated copies of the first advertised endpoint instead of all members. When that endpoint fails,
NextAddress cycles through the malformed list and the appointment operations repeatedly retry an
unavailable node rather than reaching another controller.
Deterministic indexing bug: every iteration converts addresses[0], omitting all subsequent failover
endpoints.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
The helper iterates over the full address count but always converts element zero, while
NextAddress advances only through the resulting _kontrollerNodes list. The server explicitly
populates responses from every controller node, so omitting all elements after the first discards
real failover targets.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
`GetKontrollerNodes` reads index zero during every loop iteration, so peer Kontrol Plane nodes are omitted and failover can retry the same unavailable endpoint indefinitely.
## Fix Focus Areas
- src/KurrentDB.DataPlane/KontrolPlane/Transport/Grpc/GrpcKontrolPlaneClient.cs[79-89]
## Recommended Fix
Convert `addresses[i]` on each iteration instead of `addresses[0]`, retaining the existing leader-first ordering and leader de-duplication. Add coverage proving that multiple distinct advertised endpoints remain available in their expected failover order.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
2. Failover uses a stale node list✓ Resolved🐞 Bug☼ Reliability
Description
AnnounceNodeAsync now assigns _kontrollerNodes only inside the non-empty leader redirect branch,
replacing the previous unconditional assignment from every response. Normal streaming responses
carry an empty leader and a populated membership list, so topology changes remain absent from every
later failover until some redirect occurs.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
The client updates _kontrollerNodes only when KontrollerLeader is non-empty, but the server sets
that field to empty on ordinary cluster responses and still populates KontrollerNodes. All
subsequent endpoint rotation reads _kontrollerNodes, making the ignored membership unavailable to
failover.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
Normal announcement responses contain current Kontrol Plane membership without a redirect, but the client now ignores those lists and continues failing over through stale seed data.
## Fix Focus Areas
- src/KurrentDB.DataPlane/KontrolPlane/Transport/Grpc/GrpcKontrolPlaneClient.cs[53-66]
## Recommended Fix
Update `_kontrollerNodes` for every announcement response. Select the advertised leader when present and otherwise use the current successful streaming endpoint as the leader passed to `GetKontrollerNodes`, preserving leader-first ordering without discarding normal membership updates.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changed: Optimized KPlane leader resolution