Commit Graph

2 Commits

Author SHA1 Message Date
Joseph Doherty ec3b835e4a fix(controlplane,runtime): #868 fourth review — SQL poison allowlist, memory-counted acks survive restart, adopted revisions unconfirmed
M1 IsPoison classified every SQL Server transient as poison:
   Microsoft.Data.SqlClient never sets DbException.IsTransient, so a deadlock
   (1205), timeout (-2 / Win32 258), dropped connection or AG failover was
   counted from memory and the node's row left Applying. Poison is now an
   ALLOWLIST: a DbUpdateException (not concurrency) carrying a SqlException
   whose number is 8152, 2628, 547, 515, 2627 or 2601. Everything else
   retries, bounded by the deadline deferral cap. IsPoison is public for tests.
   Acks counted only from memory (poison branch and the deferral cap) are kept
   in _memoryOnlyAcks and re-sent by PreRestart alongside _unpersisted, so a
   restart before the terminal write does not lose them.
M2 Revisions adopted without an outcome-reporting apply are unconfirmed, so a
   same-revision dispatch re-applies instead of short-circuiting a clean
   Applied over an empty address space: TryRecoverFromStale (restores only the
   revision marker) and RestoreApplied when the artifact read fails (null
   blob, LoadArtifactBlob swallows the error) or the restore throws.
L1 The re-drive comment documents the CreatedAtUtc recompute after a second
   restart / failover.
L2 _currentRevisionDegraded renamed _currentRevisionUnconfirmed; the stale
   TryBootFromCache remark corrected.
L3 docs/Redundancy.md: a Direct node running from cache that still cannot
   reach the ConfigDb acks a same-revision re-dispatch Failed
   (PartiallyFailed, not Sealed); Stale recovery and failed restores are
   unconfirmed too.

Test harness: SqlExceptionFactory builds real SqlExceptions (reflection on
SqlError / SqlErrorCollection / SqlException.CreateException);
ConstraintEnforcingDbFactory now throws DbUpdateException over SQL 2628
(over-length), 547 (poison) and 1205 (transient), plus FailNextContexts.

Tests (all fail on the previous code): poison allowlist theory (1205, 40613,
10054, 4060 not poison) + timeout/concurrency/providerless not poison; a
deadlocked ack write is retried and recorded; a memory-counted ack survives a
coordinator restart; Stale-recovered and failed-restore revisions re-apply on
re-dispatch.

Claude-Session: https://claude.ai/code/session_01GT67AD99zQQqpLCVm3p7xk
2026-11-11 13:31:49 -05:00
Joseph Doherty 780c87eba3 fix(controlplane,runtime): #868 third review — bounded failure reasons, poison ack writes, cache-booted revisions unconfirmed
1 PersistNodeAck wrote a Failed ack's FailureReason unbounded; the column is
  nvarchar(2048), so on SQL Server the write failed forever, the ack retried
  forever, the deadline deferred forever and every later deploy was refused.
  - One shared bound, Configuration.FailureReasons.Truncate (MaxLength 2048),
    used by the coordinator and DriverHostActor.UpsertNodeDeploymentState
    (exception messages and degraded reasons).
  - A non-transient DbUpdateException (not concurrency, no transient
    DbException / timeout / socket / IO underneath) is POISON: the ack is
    counted from memory with an Error, never retried.
  - Deadline deferral on unpersisted acks is capped at 10 x the retry ceiling;
    past it the outcome is decided from the acks held in memory.
2 A node restarting in the outcome window boots the cached revision (cache is
  written before the outcome) and never acks. Cache-booted revisions
  (BootstrapFromCache / TryBootFromCache) are now marked unconfirmed, so a
  re-dispatch re-applies and acks the real outcome instead of a clean
  short-circuit. docs/Redundancy.md corrected (replay only on the ConfigDb boot
  path).
3 A lost dispatch re-driven near its deadline now gets a FULL deadline (a node
  needs its whole outcome timeout + margin = ApplyDeadline, so any shorter
  remainder timed out nodes that applied). No dispatched-at column: a second
  restart recomputes from CreatedAtUtc and closes it instead (documented).
4 docs/Redundancy.md upgrade note: do not deploy while the central pair is mid
  rolling-upgrade across this change.

Tests (all fail on the previous code): oversized Failed reason truncated (on a
SaveChanges interceptor that enforces the column like SQL Server); rejected ack
write counted from memory; bounded deadline deferral; full deadline on a
near-deadline re-drive; cache-booted revision re-applies on re-dispatch;
oversized degraded reason truncated on the node's own row.

Claude-Session: https://claude.ai/code/session_01GT67AD99zQQqpLCVm3p7xk
2026-11-11 13:13:36 -05:00