Make state writes conditional, so a setting change is not lost to a check

[ivanvyd]

Reading a state, changing it and writing it back is three steps, and until now nothing guarded the gap. A check finishing between another writer's read and its write had its result overwritten — or overwrote a setting somebody had just changed from the dashboard.

CosmosDbStateProvider's README has documented this as a known limitation from the beginning, the CHANGELOG listed it under Known issues, and the README roadmap said resolving it "needs the interface to carry a version, which is a breaking change and a major release."

It is not breaking. That was the substantive finding here, and it is the third time in this series that a "needs a major version" claim turned out to be wrong on inspection.

The API

Three new members on IStateProvider, all defaulted interface methods:

member	meaning
SupportsOptimisticConcurrency	feature detection; false by default
GetStateEntryAsync	the state plus the version it was read at
TrySetStateAsync	writes only if that version is still current

A provider written against the original two-method interface still compiles and still works. Its default GetStateEntryAsync reports no version, and its default TrySetStateAsync refuses a versioned write rather than performing an unconditional one — ignoring the version would lose exactly the update the version was passed to protect, and would do it silently.

A refused write returns false rather than throwing, unlike Orleans (InconsistentStateException) and EF Core (DbUpdateConcurrencyException). Under contention a conflict is the expected outcome, not an exceptional one, and the caller's answer is always the same: read again, reapply, write again. StateProviderExtensions.UpdateStateAsync is that loop — the read-modify-write pattern the Azure SDK's conditional-request guidance describes.

Both writers go through it: the nine setting changes on PulseChecker, and a check storing its own result. The second one matters more than it looks, and I got it wrong first time round — see below.

Per provider

provider	conditional write	create-if-absent	conflict looks like
CosmosDB	ETag as If-Match	CreateItemAsync	412 / 409
PostgreSQL	version in UPDATE ... WHERE	ON CONFLICT (name) DO NOTHING	0 rows affected
SQL Server	same	WHERE NOT EXISTS (... WITH (UPDLOCK, HOLDLOCK))	0 rows affected
SQLite	same	ON CONFLICT(name) DO NOTHING	0 rows affected
In-memory	TryUpdate CAS	TryAdd	returns false

The relational providers add the column to an existing table on startup, probing the schema portably (SELECT * WHERE 1 = 0 + CommandBehavior.SchemaOnly) rather than relying on an IF NOT EXISTS that SQLite lacks for ADD COLUMN.

Three cases, and collapsing any two is a bug

Nothing stored yet has no version to compare, so the write would go through unconditionally — and two writers both finding nothing, both creating, would lose one of the two changes. IStateProvider.AbsentVersion is the "only if it is still missing" precondition, spelled If-None-Match: * in HTTP.

Stored and versioned is the ordinary case: the version goes in the write.

Stored but unversioned — a row written before the column existed — is not the same as absent. Treating it as absent asks for a create that can never succeed, so the first setting change after upgrading would retry five times and throw, on every existing checker. It writes unconditionally instead, exactly as it did before the upgrade, and carries a version from then on.

What review caught (second commit)

Three lenses ran over the first commit. All three findings pointed the same way — the guarantee was claimed more widely than the code delivered it:

The check's own write was never converted. TriggerAsync still read with GetStateAsync and wrote with SetStateAsync, unconditionally. Because a result is stored by replacing the whole state, that write put every other field back to what it was when the check started — so the exact sentence the CHANGELOG used to describe the fix was the case still broken. Within one process the semaphore hides it; across two replicas sharing one store, which is the only reason to version a write at all, nothing does. Two reviewers reproduced it independently, one against an isolated snapshot build.

The test meant to cover that could not fail. It raced TriggerAsync against SetGroupAsync on one checker, where the semaphore serialises them, and nothing suspends against the in-memory provider — so the two ran one after the other, twenty times over. It passed against the unversioned code too. My own mutation run had already said so and I misread it: the "checker writes unconditionally" mutation killed exactly one test, and it was the other one.

INSERT ... SELECT ... WHERE NOT EXISTS is not atomic. Under READ COMMITTED two writers both find no row, both insert, and the loser takes a primary key violation — an exception, where the contract says a lost write returns false. The SQL Server dialect's own remarks in the same file already said precisely this about the upsert, and the comment I added above the new statement asserted the opposite.

The migration had the same shape — two replicas both find the column missing, both ALTER, and the loser fails out of IHostedService.StartAsync, so the host does not start. A failure is no longer taken at face value: the columns are read again, and one that left the column present was the other instance winning. Anything else is rethrown untouched, so this neither matches on error text nor hides a real failure.

Verification

394 unit tests green on both net8.0 and net10.0; 11 consecutive clean runs of the full suite. The relational tests run against a real SQLite file, so the conditional UPDATE, the create path and the column migration are executed rather than inspected.

Mutation-tested, because a passing test proves nothing until it has been seen to fail:

mutation	tests failed
in-memory ignores the expected version	3
unversioned provider silently ignores the version	1
checker writes unconditionally	1
relational claims every conditional write landed	2
create-if-absent overwrites a concurrent create	1
the null-version collapse (the upgrade regression)	1
the check writes its result unconditionally	1
a dialect falls back to the racy portable insert	1
losing the migration race crashes startup	1
a real migration failure is swallowed too	1

Docs

CHANGELOG.md, the root README.md roadmap, Healthie.StateProviding.CosmosDb/README.md and Healthie.LeaderElection/README.md all asserted last-write-wins or "planned for the next major version". All four corrected.

Not included

• No release. Version stays at 3.1.4; tagging is the maintainer's call.
• CosmosDB's new paths have no automated coverage — consistent with the repo's existing pattern (no CosmosDB unit tests at all), but the ETag and 409 paths are verified by reading, not by running. Worth a HEALTHIE_TEST_COSMOS-gated suite later.
• dotnet format --verify-no-changes fails on files outside this diff. Pre-existing, not a CI gate, deliberately left alone so the diff stays reviewable.