fix(transactions): honor transaction timeout for participant locks

[ReubenBond]

Problem

TOC can acquire its priority-committer lock before Azure-backed participants finish joining a transaction. Transactions request a 10-second execution timeout, but participant locks were independently expired after the configured 8-second lock timeout. Under storage latency, that allowed the committer lock to break before prepare and surfaced OrleansBrokenTransactionLockException.

Solution

Propagate the requested transaction timeout through TransactionInfo and its forks, then retain acquired participant locks for the greater of that timeout and the configured lock timeout. Queued lock groups receive the same effective lease when promoted, with the duration applied against each participant's local clock. Regression coverage verifies propagation, fork behavior, fallback behavior, and the held-lock deadline, and the affected TOC theories are re-enabled.

Fixes #9556

Microsoft Reviewers: Open in CodeFlow

[Copilot]

Pull request overview

This PR addresses transaction lock flakiness in Orleans Transactions by propagating the requested transaction timeout through TransactionInfo (including forks) and using it to extend participant lock retention beyond the configured lock timeout when needed, preventing premature lock expiry under storage latency.

Changes:

• Propagate the transaction timeout via TransactionInfo and ensure it is preserved across forks.
• Update participant lock acquisition to compute/retain deadlines using an effective lock timeout (max(transaction timeout, configured lock timeout)).
• Add/adjust regression tests and re-enable previously skipped TOC test theories.

Show a summary per file

File	Description
test/Transactions/Orleans.Transactions.Tests/TransactionRecoveryLatencyTests.cs	Adds regression tests for timeout propagation across forks and lock-deadline behavior.
src/Orleans.Transactions/TOC/TransactionCommitter.cs	Passes transaction timeout into lock acquisition to align committer behavior with transaction timeout.
src/Orleans.Transactions/State/TransactionalState.cs	Propagates transaction timeout into participant lock acquisition for reads/updates.
src/Orleans.Transactions/State/ReaderWriterLock.cs	Extends lock group deadline logic to honor effective lock timeout and exposes internal helpers for tests.
src/Orleans.Transactions/DistributedTM/TransactionRecord.cs	Stores per-transaction effective lock timeout for deadline calculation when a queued group is promoted.
src/Orleans.Transactions/DistributedTM/TransactionInfo.cs	Adds serialized timeout field and copies it in the fork/copy constructor.
src/Orleans.Transactions/DistributedTM/TransactionAgent.cs	Sets TransactionInfo.Timeout when starting a transaction.
src/Orleans.Transactions.TestKit.xUnit/TOCGoldenPathTestRunner.cs	Re-enables TOC golden-path theory (removes skip).
src/Orleans.Transactions.TestKit.xUnit/TocFaultTransactionTestRunner.cs	Re-enables TOC fault theories (removes skip).

Review details

Tip

Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

• Files reviewed: 9/9 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

Review details

• Files reviewed: 9/9 changed files
• Comments generated: 0 new
• Review effort level: Lite

[ReubenBond]

Rebased onto current main \9dcf8d3a3b53723c81ccc14a5d5ca56362ee029e; new head: \1c8940c8c7a0a6a4efa0f4c65e57c6438db4d1e8.\n\nFocused validation on this exact head:\n- \TransactionRecoveryLatencyTests: 14/14 net8.0, 14/14 net10.0\n- Azure TOC suite against CI-pinned Azurite 3.35.0: 8/8 net8.0, 8/8 net10.0, no skips\n\nThe previous aggregate Azure net10 failure was the unrelated #10452 stream readiness flake and occurred before the transaction test assembly ran.

[ReubenBond]

Current-head run 31560515935 red-check classification (all unrelated to this transaction-lock/TOC diff):\n\n- PostgreSQL net8, job 94001664118: \AdoNet_16_MultipleStreams_ManyDifferent_ManyProducerGrainsManyConsumerGrains\ delivery timeout; tracked in #10458.\n- Azure Storage net8, job 94001664189: \SMS_StreamRel_AllSilosRestart_PubSubCounts\ retained two publishers; tracked in #10503, with fix #10532 open. The transaction assembly was not reached in this job.\n- Azure Storage net10, job 94001664223: the transaction assembly's only failure was \TransactionWillRecoverAfterRandomSiloGracefulShutdown\ exceeding its graceful-recovery drain deadline; tracked in #10529, with fix #10537 open. No TOC test failed.\n- Ubuntu BVT net8, job 94001664204: \ShardExecutorTests.RunShardAsync_WhenRetryPersistenceFails_ReleasesConcurrencyAndContinuesProcessing\ leaked its injected persistence exception; newly tracked separately after an exact local rerun passed 1/1.\n\nFocused exact-head TOC validation remains 8/8 on both net8.0 and net10.0 against Azurite 3.35.0.

[Copilot]

Review details

• Files reviewed: 9/9 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

Review details

• Files reviewed: 9/9 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

Copilot review overview

🔵 Needs a closer look

Review tier: Lite
Findings: None

Issues resolved since last review (1)

Severity	Finding
	test/​Transactions/​Orleans.Transactions.Tests/​TransactionRecoveryLatencyTests.cs — StartTransaction(readOnly: false, timeout) mixes a named argument with a subsequent positional… View resolved comment

Suppressed comments (1)

src/Orleans.Transactions/State/ReaderWriterLock.cs:400

• The expired-waiter pruning loop just above this block removes entries from currentGroup while iterating it, which can throw InvalidOperationException (modifying a Dictionary during enumeration). Iterate over a snapshot before removing.

                            currentGroup.Deadline = currentGroup.Count == 0
                                ? null
                                : AddTimeout(now, currentGroup.Values.Max(record => record.LockTimeout));

[ReubenBond]

The failed CI run exposed a merge-ref compile error in TransactionRecoveryLatencyTests.cs: EnterLock gained the transaction-timeout parameter, but the newer cancellation regression test still passed the old positional argument list, producing CS8323 across the matrix.

Fixed in 170becd0d5 by rebasing onto current main and passing the lock timeout explicitly while retaining the original default access counter. dotnet build Orleans.slnx -bl and TransactionRecoveryLatencyTests on net8.0/net10.0 pass. The rebased branch was force-pushed with an exact lease.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Review tier: Lite
Findings: None

Suppressed comments (1)

src/Orleans.Transactions/State/ReaderWriterLock.cs:398

• The foreach loop immediately above this block removes entries from currentGroup (currentGroup.Remove(kvp.Key)) while enumerating it. Since LockGroup inherits Dictionary, that can throw InvalidOperationException. Enumerate a snapshot (e.g., currentGroup.ToArray()) before removing expired waiters.

                            currentGroup.Deadline = currentGroup.Count == 0

[Copilot]

Copilot review overview

🔵 Needs a closer look

Review tier: Lite
Findings: None

Suppressed comments (1)

src/Orleans.Transactions/State/ReaderWriterLock.cs:400

• The queued-waiter pruning loop removes entries from currentGroup while enumerating it (currentGroup.Remove(kvp.Key) inside foreach (var kvp in currentGroup)), which can throw InvalidOperationException because LockGroup inherits Dictionary<,>. Iterate over a snapshot (e.g., currentGroup.ToList()) before removing entries so pruning is safe and deterministic.

                            currentGroup.Deadline = currentGroup.Count == 0
                                ? null
                                : AddTimeout(now, currentGroup.Values.Max(record => record.LockTimeout));