Fix #5502 -- check the row count of every progression write, not just the first

[jeremydmiller]

Fixes #5502. Reported with a complete reproduction — and the cumulative-RecordsAffected diagnosis — by @arnelirobles.

The bug is bigger than "index 0"

UpdateProjectionProgress is both AssertsOnCallback and NoDataReturnedCall, so from index 1 on the loop hits continue before anything looks at the marker. A stale-floor progression update is a silent no-op.

That is not only reachable through QueueOperation. ExecutionStage.ExecuteDownstreamAsync records every composite member's progress into the parent's shared batch:

// Need to record the individual progress even though it's locked together
await cloned.ActiveBatch!.RecordProgress(cloned);

…on top of the parent's own, queued by StartProjectionBatchAsync. So a composite with N members puts N+1 progression operations on one page — the shipped three-member fixture produces four rows. The parent's is deterministically index 0 (queued before any stage runs), so the parent was checked and every member was not.

Why the obvious fix doesn't work

DbDataReader.RecordsAffected is cumulative across the batch, so a second progression update inherits the first one's count and never sees its own zero. I confirmed the naive fix (mirroring the index-0 branch) still doesn't throw.

It also means the index-0 check was never reliable either: when a later statement contributes a row, index 0's own zero is masked. That's the second test here, and it was red before this change.

The per-statement count lives on NpgsqlBatchCommand.RecordsAffected — but Npgsql populates it lazily as the reader advances. I measured this: parked on statement 0, even after draining 5000 rows, statements 1 and 2 both read -1; they only become correct after NextResultAsync() steps past them. So there is no point inside the callback loop at which an operation could read its own count, in any design.

Hence a separate pass after the reader is drained. Compile() keeps the NpgsqlBatch for it. BatchBuilder.StartNewCommand() appends exactly one command per operation and no Marten operation starts one of its own, so _operations[i] is _compiled.BatchCommands[i].

It logs, it does not throw

Deliberate, and the main thing to push back on if you disagree. Making the check effective converts progression writes that have been silently no-opping into hard shard failures, and ShardFailure categorizes ProgressionProgressOutOfOrderException as a split-brain double-daemon — an actively misleading diagnosis for what is usually a loader bug (#5501 was exactly one). Promoting to a throw is a breaking change and belongs in a major; the exception the operation hands back is the one that will be thrown then. The long-standing index-0 throw is untouched.

The logged exception uses the three-argument overload, so it carries the floor and ceiling: "expected floor 5 but it had already moved, attempted ceiling 50" is what tells an operator which of the two they're looking at.

Scope

One additive interface, IAssertsRecordsAffected, implemented only by UpdateProjectionProgress. InsertProjectionProgress doesn't need it (a failed insert raises a PK violation). DeleteProjectionProgress isn't AssertsOnCallback, so projection teardown never enters the pass — and a page with no opted-in operation returns before it even drains.

Entirely inside Marten. PostprocessAsync lives in Weasel.Core and is untouched — changing its signature to carry a count would have been a cross-repo binary-compat break across Marten and Polecat, which is why this is a separate interface instead.

Per-tenant partitioning turned out not to be a second instance of this. Each {name}:All:{tenant} shard gets its own agent, execution and ProjectionUpdateBatch with exactly one progression op; SessionForTenant only ever adds document operations. The test added there pins that invariant so a future change that starts sharing batches across tenants doesn't quietly reintroduce the hazard.

Tests

Three of the six unit cases are red without the pass. The rest are guards against exactly the risk this change carries:

• a_lone_stale_update_still_throws_exactly_as_it_always_has — no behavior change at index 0;
• a_healthy_batch_of_several_progression_updates_reports_nothing — no false positives;
• an_ordinary_document_batch_is_untouched;
• a_document_write_mixed_in_with_a_stale_progression_update_still_commits — the extra pass doesn't disturb a real operation's result-set handling.

Plus two end-to-end runs covering the shapes that actually batch: a composite daemon run (four progression rows, all advancing, zero warnings) and a per-tenant daemon run across two tenants.

suite	Newtonsoft	System.Text.Json
DocumentDbTests	1113 / 1 skipped / 0 failed	1109 / 5 skipped / 0 failed
DaemonTests	332 / 332	332 / 332
EventSourcingTests	2300 / 20 skipped / 0 failed	2300 / 20 skipped / 0 failed
TenantPartitionedEventsTests	253 / 2 skipped / 0 failed	—
CoreTests	686 / 7 skipped / 0 failed	—

net9.0 against local PostgreSQL 17. DocumentDbTests is identical to the master baseline.

Related: #5506 (#5501), which is one concrete source of the stale floors this now reports.

🤖 Generated with Claude Code

https://claude.ai/code/session_01481eiEZg1Bv6pu4DrgPRg3