SQL Server: concurrent appends with `ExpectedStreamVersion.Any` fail because `check_stream` reads the stream without locks

[nmummau]

Describe the bug

check_stream reads Streams.Version with a plain SELECT. With the default READ COMMITTED isolation (READ_COMMITTED_SNAPSHOT OFF), two concurrent appends to the same stream can read the same version. The second append then fails with AppendToStreamException, even with ExpectedStreamVersion.Any, which should never conflict.

There are two failure cases:

1. Existing stream. Both appends insert at the same StreamPosition. The second one fails on UQ_StreamIdAndStreamPosition:

   WrongExpectedVersion: duplicate append for stream 1 with expected_version=-2. SQL: Violation of UNIQUE KEY constraint 'UQ_StreamIdAndStreamPosition'. Cannot insert duplicate key in object '<schema>.Messages'. The duplicate key value is (1, 1).
   

2. New stream. Both appends find no row, and both insert into Streams. The second one fails on UQ_StreamName:

   WrongExpectedVersion -2, stream already exists
   

In both cases, the insert into Messages that rolls back has already used an identity value. This leaves a gap in GlobalPosition, and all-stream subscriptions must wait for the gap timeout before they skip it (see #588).

To Reproduce

A failing test is in commit nmummau/eventuous@e0f0e5e (ConcurrentAppendTests.cs), on the branch test/sqlserver-concurrent-append-any of the fork. The commit is based on dev at ed6cfd1c. The test uses the existing StoreFixture Testcontainer.

1. Check out the commit:

   git fetch https://github.com/nmummau/eventuous.git test/sqlserver-concurrent-append-any
   git checkout e0f0e5ef0bb69971bc043487365877d0ea51b170

2. Run:

   dotnet test --project src/SqlServer/test/Eventuous.Tests.SqlServer -f net10.0 --treenode-filter "/*/*/ConcurrentAppendTests/*"

3. See all three tests fail:

   Test	Result
   ShouldAppendWithAnyWhenTwoAppendsReadAnExistingStreamTogether	❌ Violation of UNIQUE KEY constraint 'UQ_StreamIdAndStreamPosition'
   ShouldNotLeaveGlobalPositionGapWhenTwoAppendsReadAnExistingStreamTogether	❌ identity grew by 2 for 1 stored event
   ShouldAppendWithAnyWhenTwoAppendsCreateTheSameStreamTogether	❌ WrongExpectedVersion -2, stream already exists

The race window is short, so two appends started at the same time fail only sometimes. The test makes the failure happen on every run. A third "gate" connection holds a lock. The lock lets check_stream read the stream, but stops the write that comes after the read:

Case	Gate SQL (held in an open transaction)
Existing stream	SELECT COUNT(*) FROM <schema>.Messages WITH (TABLOCKX);
New stream	SELECT StreamId FROM <schema>.Streams WITH (UPDLOCK, HOLDLOCK) WHERE StreamName = @stream_name;

The test starts two AppendEvents(stream, ExpectedStreamVersion.Any, ...) calls. It waits until sys.dm_exec_requests shows both requests blocked inside append_events or check_stream, then rolls back the gate transaction.

Expected behavior

Two concurrent appends with ExpectedStreamVersion.Any to the same stream both succeed, for an existing stream and for a new stream. A second append to the same stream waits for the first one to commit, and no append uses a GlobalPosition that it does not keep.

Screenshots

N/A. The error messages are in the sections above.

Environment

• OS: Windows 11, with SQL Server in a Linux container (Rancher Desktop)
• SQL Server: 2022 (mcr.microsoft.com/mssql/server:2022-latest), READ_COMMITTED_SNAPSHOT OFF
• Eventuous.SqlServer: 0.16.4, and dev at v0.17.0 (ed6cfd1c)
• .NET: 10

Additional context

The problem occurs most on streams that many writers append to with Any. The atomic multi-stream AppendEvents makes this a common pattern, for example an aggregate stream and a shared index stream in one transaction.

Proposed fix, in src/SqlServer/src/Eventuous.SqlServer/Scripts/3_CheckStream.sql:

     SELECT
         @current_version = [Version],
         @stream_id = StreamId
-    FROM [__schema__].Streams
+    FROM [__schema__].Streams WITH (UPDLOCK, HOLDLOCK)
     WHERE StreamName = @stream_name;

• UPDLOCK makes a second append to the same stream wait until the first one commits. The second append then reads the new version.
• HOLDLOCK also locks the key range of a StreamName that does not exist yet. Two appends that create the same stream therefore also run one after the other.
• Appends to different streams still run in parallel.
• The lock stays in the caller's transaction. A multi-stream append holds the lock of each stream until it commits. Callers that append to more than one stream in a transaction must use a consistent stream order, or deadlocks can occur.
• The script uses CREATE OR ALTER, so existing databases get the change when the schema initializes.

With this change, the three tests above pass on dev, and no GlobalPosition is used by a failed append.

Related: #590 proposes changes to check_stream and append_events for sharding. This fix changes the same SELECT in check_stream. A failed append would also leave a gap in a per-shard position sequence, so the fix applies with sharding too.

I can send a PR with the fix and the tests.

[linear]

EVE-252