Repository navigation
feat(sm): RFC 3539 watchdog for connections a StateMachine accepts - #77
Conversation
Watchdog supervision existed only on sm.Client. A server built from sm.New and diam.Server answered the DWRs it received, but never sent one and never noticed a peer that went silent while its transport stayed up, although RFC 6733 §5.5.3 requires every implementation to support the RFC 3539 algorithm, which runs on all open connections (§3.4). - Settings gains EnableWatchdog, WatchdogInterval (Twinit, default 30s, minimum 6s, ±2s jitter per Tw, RFC 3539 §3.4.1), WatchdogStream and OnWatchdogConnEvent, with the names sm.Client uses. Supervision is opt-in; conforming deployments should enable it. - Supervision starts once the CEA is written and OnHandshake has returned, never for a rejected or aborted CER. It follows Appendix A on the established connection: a DWR after Tw of silence, never resent while one is pending; SUSPECT at the next expiry, reported as WatchdogSuspect (a responder has no queue to fail over); DOWN and close at the expiry after that. Any valid received message resets Tw and recovers a SUSPECT connection; a valid DWA clears the pending DWR. - A peer's DPR and a local Disconnect stop supervision before the Closing exchange (RFC 6733 §5.6), including a Disconnect issued while the CEA is being written or OnHandshake runs. Once the stop returns, no DWR is written, no DWA is credited, no activity counts, no watchdog event is emitted and the watchdog does not close the transport. - Counting received traffic takes no lock, so a blocked DWR write or a slow event callback cannot stall inbound dispatch; Server.WriteTimeout bounds the write itself. - Client and server share one supervision loop (watchdog.go); Client's behaviour and tests are unchanged. - peer.Manager rejects the new Settings fields, because it supervises its peers itself. Tests cover the issue's reproducer at the default Twinit in virtual time and the per-expiry jitter; TCP and SCTP (DWR on the configured stream, DWA credited from any stream) in every dispatch mode; recovery, DWA handling, per-connection answers, no supervision before or after a rejected handshake, DPR and Disconnect racing the handshake and the expiry, cleanup on every close cause, a blocked DWR write, and callback panics. Fixes #72
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
The lint failure is not from this change: staticcheck 2026.2.1 cannot read the export data of Go 1.27.2, the stable release CI now installs, so it stopped before analysing any package. Under Go 1.27.2, a staticcheck built from go-tools master (f1838cc3, which carries the fix) and golangci-lint both report no issues on this branch. The workflow pin is updated in a separate change. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b69c321fe
ℹ️ 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".
| observe: func(event WatchdogEvent) { w.observe(c, event) }, | ||
| timeout: func() { w.terminate(nil, slog.LevelWarn, "sm: watchdog timeout; closing connection", nil) }, | ||
| } | ||
| dwa := handleDWA(sm, w.signals.dwac, func(_ diam.Conn, event WatchdogEvent) { w.emit(event) }) |
There was a problem hiding this comment.
The accepted supervisor passes every syntactically successful DWA to handleDWA, but it never records the DWR's Hop-by-Hop/End-to-End IDs, and handleDWA therefore clears Pending for unsolicited, duplicated, or stale answers. For example, a delayed DWA from an earlier probe arriving while a newer DWR is outstanding credits the newer request and postpones or prevents the SUSPECT/DOWN transition. The managed-peer watchdog already enforces this correlation in diam/peer/watchdog.go; this path should likewise retain the outstanding IDs and reject answers that do not match.
Useful? React with 👍 / 👎.
Watchdog supervision existed only on
sm.Client. A server built fromsm.Newanddiam.Serveranswered the DWRs it received, but it never sent one, and it never noticed a peer that went silent while its transport stayed up. RFC 6733 §5.5.3 requires every implementation to support the RFC 3539 algorithm, and RFC 3539 §3.4 runs that algorithm on all open connections.Settings
SettingsgainsEnableWatchdog,WatchdogInterval,WatchdogStreamandOnWatchdogConnEvent, with the same namessm.Clientuses.peer.Newrejects these fields, because the Manager supervises its peers itself.Behaviour (RFC 3539 Appendix A on an established connection)
OnHandshakehas returned. It never starts for a rejected or aborted CER, or on a connection already closed or Closing.WatchdogSuspect. A responder has no request queue to fail over (§3.4.1 [3]).Disconnectstops supervision, including aDisconnectissued while the CEA is being written orOnHandshakeruns. Once the stop returns:Server.WriteTimeoutbounds the DWR write.watchdog.go). Client behaviour and its existing tests are unchanged.Tests
Disconnect, including aDisconnectracing the handshake or an expiry, and a stale DWA after the stop.Validation
Under Go 1.27.2:
go build ./...,go test ./... -count=1,go test -race ./diam/... -count=1,go test -race -count=10 -run Watchdog ./diam/sm/andgo vet ./...pass;golangci-lint run ./...report no issues, andgofmt -l .is empty;examples/middlewarebuild, vet and test pass.Fixes #72
Summary by cubic
Adds RFC 3539 watchdog supervision to connections accepted by a
StateMachineserver, previously onlysm.Clienthad it. New opt-insm.Settingsfields (EnableWatchdog,WatchdogInterval,WatchdogStream,OnWatchdogConnEvent) supervise an accepted connection after a successful CEA andOnHandshake. A silent peer gets a DWR after Tw, SUSPECT at the next expiry, and DOWN with transport closure at the third; any received message resets Tw. Supervision stops on DPR orDisconnect(RFC 6733 §5.6).peer.Managerrejects these fields because it supervises peers itself. Fixes #72.Behavior
WatchdogSuspectevent; DOWN closes the transport.Written for commit 6b69c32. Summary will update on new commits.