Skip to content

feat(client): make reconnect behavior policy-driven - #602

Merged
SunSi12138 merged 1 commit into
devfrom
issue-592-reconnect-policy
Sep 8, 2026
Merged

feat(client): make reconnect behavior policy-driven#602
SunSi12138 merged 1 commit into
devfrom
issue-592-reconnect-policy

Conversation

@SunSi12138

@SunSi12138 SunSi12138 commented Sep 8, 2026

Copy link
Copy Markdown
Owner

Closes #592

Summary

  • Add one immutable SharpLinkReconnectPolicy covering initial delay, max backoff, multiplier, jitter bounds, and stable-reset window.
  • Add initial configuration through SharpClientBuilder.UseReconnectPolicy(...) plus runtime GetReconnectPolicy() / UpdateReconnectPolicy(...).
  • Apply the same policy model to the existing fixed, multi-endpoint static, and dynamic-resolver reconnect owners without introducing a second reconnect coordinator.
  • Runtime publication wakes an armed reconnect wait but does not reset topology-owned failure streak/backoff position, stable-ready history, task ownership, or an in-progress dial.
  • Link policy generations so a dial already in flight across one or more updates completes under the existing lifecycle owner, while its subsequent state transition uses the newest effective complete policy.
  • Reconcile active backoff deterministically: preserve values inside replacement bounds, clamp to a raised InitialDelay or lowered MaxBackoff, retain position when bounds expand, and use the latest multiplier/jitter for subsequent scheduling.
  • Preserve Ready history across policy publication; StableResetWindow changes evaluate existing monotonic history rather than manufacturing a new stable period.
  • Keep logical RPC retry and ordinary RPC invocation independent from reconnect-policy publication.
  • Document the fixed/static/dynamic compatibility matrix and lifecycle semantics in doc/client-reconnect-policy.md.

Review follow-up

Engineering review identified three reconnect-state correctness issues; all are fixed in the final head.

  1. Monotonic timestamp zero is valid. Ready-history presence is represented separately from timestamp value. Fixed topology uses _hasReconnectReadyTimestamp; static/dynamic endpoint states use explicit HasReadyTimestamp state with mark/clear operations. A valid TimeProvider.GetTimestamp() == 0 is preserved, and later Ready publications do not shift an already-active stability anchor.
  2. Reconnect completion must linearize to one policy generation. Fixed/static/dynamic completion paths capture completionPolicy = generation.Policy exactly once after the attempt and use that immutable object for the entire state transition through ResolveReconnectCompletionDelay. A runtime publication cannot combine StableResetWindow from one successor generation with InitialDelay/backoff semantics from another within a single completion transition.
  3. Failed Ready publication must not manufacture stability. Static and dynamic endpoint state now calls MarkReadyTimestamp(...) only after ReconcileResponseCompressionPreferenceAfterReadyPublication(...) succeeds. If reconciliation rolls the candidate connection/snapshot back, no Ready-history anchor is committed. A later successful reconnect therefore starts its stability window from the real successful Ready publication.

The obsolete ReconnectPolicyChanged() topology reset seam was also removed so ordinary policy publication has no path that zeroes live backoff state.

All three inline review threads were replied to and resolved after exact-head validation.

Validation coverage

Deterministic fake-time/unit coverage includes:

  • InitialDelay increase/decrease and MaxBackoff increase/decrease mid-streak
  • multiplier and jitter changes while a failure streak is active
  • stable-reset threshold changes against preserved Ready history, including a valid zero monotonic timestamp
  • explicit Ready-history presence/absence and zero-valued static/dynamic endpoint anchors
  • one-complete-policy reconnect completion semantics
  • two failed Ready-reconciliation publications followed by a successful dynamic Ready publication, proving a 5s stable-reset window starts at the successful publication rather than either rolled-back candidate
  • armed-wait wake/reschedule without duplicate reconnect ownership
  • in-flight dial crossing rapid repeated policy generations
  • reconnect success followed by the next failure/reset sequence
  • fixed pool shrink/grow racing policy publication
  • dynamic endpoint-generation replacement retiring the old reconnect owner
  • GoAway/disconnect lifecycle behavior
  • rapid publication before connectivity plus Stop sealing further updates
  • invalid candidates/default compatibility and RPC-retry independence

Final branch / CI evidence

Final branch is based directly on dev@09d6804c026dc2456d68e96a7ccf31487589334e and is one commit ahead / zero behind. Temporary issue-592 validation workflow/script files are absent from the final diff.

Exact head: 0c49b62817e7ea6f1c62dcfa3e48cbf08b4d6e9b

  • PR Fast 34231236789success: formatting, maintainability, Release build, generated-assembly boundary, full Unit/Generator/Load suites, and deterministic allocation gate.
  • CodeQL 34231236723success.
  • Pending and codec validation 34231236698success.
  • Extension fault containment 34231236705success.

Maintainability

Reconnect scheduling remains inside StaticClusterRuntime's existing single-gate lifecycle owner. The repository's narrow maintainability-baseline mechanism carries maxLoc: 850 for this file, explicitly tied to #592 and the broader topology-decomposition debt tracked by #350. The maintainability gate and its own tests pass on the final exact head.

This PR intentionally remains Draft for maintainer review.

@SunSi12138
SunSi12138 force-pushed the issue-592-reconnect-policy branch 4 times, most recently from c7e7952 to 21487e5 Compare September 8, 2026 11:30

@SunSi12138 SunSi12138 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two engineering-significant correctness findings in the reconnect state model: a valid zero monotonic timestamp is currently treated as “no Ready history”, and reconnect-success reconciliation can mix fields/semantics from two policy generations if an update lands between repeated generation.Policy reads. Inline details below; otherwise I did not find a reason to broaden scope.

Comment thread src/SharpLink.Client/SharpLinkClient.RuntimeReconnectPolicy.cs Outdated
Comment thread src/SharpLink.Client/SharpLinkClient.RpcChannel.cs Outdated
@SunSi12138
SunSi12138 force-pushed the issue-592-reconnect-policy branch from 9bdf941 to 52ee9e5 Compare September 8, 2026 12:08

@SunSi12138 SunSi12138 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two earlier findings are fixed on the current head. One additional lifecycle-state correctness issue surfaced while checking the rollback path: a failed Ready publication can leave fabricated stable-ready history. Inline below; the dynamic path has the same ordering.

Comment thread src/SharpLink.Client/SharpLinkClient.StaticCluster.cs Outdated
Complete runtime reconnect policy semantics across fixed, static, and dynamic topologies. Preserve topology-owned backoff and Ready history across policy publication, linearize completion to one effective policy snapshot, represent timestamp-zero history explicitly, and anchor stable-reset history only after Ready reconciliation succeeds. Add deterministic fake-time coverage for policy updates, lifecycle races, topology/pool changes, and Ready-publication rollback.
@SunSi12138
SunSi12138 force-pushed the issue-592-reconnect-policy branch from 306fe1e to 0c49b62 Compare September 8, 2026 13:19
@SunSi12138
SunSi12138 marked this pull request as ready for review September 8, 2026 13:34
@SunSi12138
SunSi12138 merged commit 1365b08 into dev Sep 8, 2026
29 checks passed
@SunSi12138
SunSi12138 deleted the issue-592-reconnect-policy branch September 8, 2026 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant