[Proposal] WIP: Add an option to exclusively lock Transactional State#9970
[Proposal] WIP: Add an option to exclusively lock Transactional State#9970scalalang2 wants to merge 1 commit intodotnet:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR proposes an opt-in mechanism to acquire exclusive locks for transactional state even during read operations, aiming to reduce lock upgrade conflicts under high contention in Orleans Transactions.
Changes:
- Introduces
[UseExclusiveLock]and propagates aUseExclusiveLockflag through transactional request/TransactionInfo. - Adds
useExclusiveLockoverloads toITransactionClient.RunTransaction(...)and plumbs the flag throughTransactionClient. - Updates
ReaderWriterLock.EnterLock(...)call sites and adjusts conflict detection to treat reads as exclusive when requested.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/Orleans.Transactions/TransactionAttribute.cs |
Adds [UseExclusiveLock] and request-side propagation into TransactionInfo. |
src/Orleans.Transactions/TOC/TransactionCommitter.cs |
Passes UseExclusiveLock into lock acquisition during commit/write path. |
src/Orleans.Transactions/State/TransactionalState.cs |
Passes UseExclusiveLock into lock acquisition for reads/updates. |
src/Orleans.Transactions/State/ReaderWriterLock.cs |
Adds exclusiveLock parameter and modifies how conflicts are computed. |
src/Orleans.Transactions/ITransactionClient.cs |
Adds RunTransaction(..., useExclusiveLock) overloads. |
src/Orleans.Transactions/DistributedTM/TransactionInfo.cs |
Adds UseExclusiveLock flag to serialized transaction context and copies it on fork. |
src/Orleans.Transactions/DistributedTM/TransactionClient.cs |
Implements new overloads and sets UseExclusiveLock when requested. |
| // check if the operation conflicts with other transactions in the group | ||
| if (HasConflict(isRead, priority, transactionId, group, out var resolvable)) | ||
| if (HasConflict(isRead && !exclusiveLock, priority, transactionId, group, out var resolvable)) | ||
| { | ||
| if (!resolvable) | ||
| { |
There was a problem hiding this comment.
exclusiveLock is only applied to the Find(...)/HasConflict(...) calls, but the rest of the lock bookkeeping still uses the original isRead value. As a result, an exclusive read can still be treated as non-conflicting by later readers (because TransactionRecord.NumberWrites is never incremented), which defeats the purpose of exclusive locking. Consider introducing a single “effective lock mode” (shared vs exclusive) and using it consistently for group selection, conflict checks, and per-record tracking (or store an explicit per-record flag which HasConflict consults).
| // Apply exclusive lock flag if requested | ||
| if (useExclusiveLock) | ||
| { | ||
| transactionInfo.UseExclusiveLock = true; | ||
| } |
There was a problem hiding this comment.
When joining an ambient transaction, this sets UseExclusiveLock only on the forked transactionInfo, then calls ambientTransactionInfo.Join(transactionInfo). Since TransactionInfo.Union(...) does not currently merge UseExclusiveLock, the flag can be lost when the ambient context reconciles pending calls, meaning useExclusiveLock: true may not apply to the overall joined transaction beyond this delegate. Either make UseExclusiveLock propagate when joining (e.g., logical-OR during union) or document/enforce that the option is only valid when creating a new transaction.
| /// Run transaction delegate with exclusive lock option | ||
| /// </summary> | ||
| /// <param name="transactionOption"></param> | ||
| /// <param name="transactionDelegate"></param> | ||
| /// <param name="useExclusiveLock">When <see langword="true"/>, all transactional states accessed during this transaction | ||
| /// will acquire exclusive locks even for read operations, preventing lock upgrade conflicts under high contention.</param> | ||
| /// <returns><see cref="Task"/></returns> | ||
| Task RunTransaction(TransactionOption transactionOption, Func<Task> transactionDelegate, bool useExclusiveLock); | ||
|
|
||
| /// <summary> | ||
| /// Run transaction delegate with exclusive lock option | ||
| /// </summary> | ||
| /// <param name="transactionOption"></param> | ||
| /// <param name="transactionDelegate"></param> | ||
| /// <param name="useExclusiveLock">When <see langword="true"/>, all transactional states accessed during this transaction | ||
| /// will acquire exclusive locks even for read operations, preventing lock upgrade conflicts under high contention.</param> | ||
| /// <returns>True if the transaction should commit</returns> |
There was a problem hiding this comment.
This adds a new public API surface area and a new transactional behavior (acquiring exclusive locks for reads), but there are no corresponding tests in the existing Transactions test projects to validate the semantics (eg, that concurrent reads conflict when useExclusiveLock/[UseExclusiveLock] is set, and that lock-upgrade failures are reduced). Adding focused concurrency tests around ReaderWriterLock/TransactionalState would help prevent regressions.
| /// Run transaction delegate with exclusive lock option | |
| /// </summary> | |
| /// <param name="transactionOption"></param> | |
| /// <param name="transactionDelegate"></param> | |
| /// <param name="useExclusiveLock">When <see langword="true"/>, all transactional states accessed during this transaction | |
| /// will acquire exclusive locks even for read operations, preventing lock upgrade conflicts under high contention.</param> | |
| /// <returns><see cref="Task"/></returns> | |
| Task RunTransaction(TransactionOption transactionOption, Func<Task> transactionDelegate, bool useExclusiveLock); | |
| /// <summary> | |
| /// Run transaction delegate with exclusive lock option | |
| /// </summary> | |
| /// <param name="transactionOption"></param> | |
| /// <param name="transactionDelegate"></param> | |
| /// <param name="useExclusiveLock">When <see langword="true"/>, all transactional states accessed during this transaction | |
| /// will acquire exclusive locks even for read operations, preventing lock upgrade conflicts under high contention.</param> | |
| /// <returns>True if the transaction should commit</returns> | |
| /// Runs a transactional delegate, optionally acquiring exclusive locks for all accessed transactional state. | |
| /// </summary> | |
| /// <param name="transactionOption">Controls the transactional behavior, such as whether to join an ambient transaction | |
| /// or always start a new one.</param> | |
| /// <param name="transactionDelegate">The asynchronous delegate to execute within the transactional context.</param> | |
| /// <param name="useExclusiveLock"> | |
| /// When <see langword="true"/>, all transactional states accessed during this transaction will acquire exclusive locks, | |
| /// even for read operations. This avoids reader-to-writer lock upgrade conflicts under high contention, at the cost of | |
| /// reduced concurrent access. When <see langword="false"/>, the default shared/read locking behavior is used. | |
| /// </param> | |
| /// <returns><see cref="Task"/> that completes when the transaction has finished executing.</returns> | |
| /// <remarks> | |
| /// Use <paramref name="useExclusiveLock"/> for highly contended transactional state where concurrent readers frequently | |
| /// upgrade to writers, causing lock upgrade failures and retries. In such cases, acquiring exclusive locks for reads can | |
| /// improve overall throughput and reduce latency by eliminating upgrade conflicts. | |
| /// | |
| /// When contention is low or reads rarely upgrade to writes, leaving <paramref name="useExclusiveLock"/> set to | |
| /// <see langword="false"/> will typically yield better concurrency. | |
| /// </remarks> | |
| /// <example> | |
| /// <code> | |
| /// await transactionClient.RunTransaction( | |
| /// TransactionOption.Create, | |
| /// async () => | |
| /// { | |
| /// // Within this delegate, all accessed transactional state will use exclusive locks, | |
| /// // including read operations, to avoid lock upgrade conflicts under contention. | |
| /// var balance = await accountGrain.GetBalance(); | |
| /// await accountGrain.Withdraw(10); | |
| /// }, | |
| /// useExclusiveLock: true); | |
| /// </code> | |
| /// </example> | |
| Task RunTransaction(TransactionOption transactionOption, Func<Task> transactionDelegate, bool useExclusiveLock); | |
| /// <summary> | |
| /// Runs a transactional delegate, optionally acquiring exclusive locks for all accessed transactional state, | |
| /// where the delegate determines whether the transaction should commit. | |
| /// </summary> | |
| /// <param name="transactionOption">Controls the transactional behavior, such as whether to join an ambient transaction | |
| /// or always start a new one.</param> | |
| /// <param name="transactionDelegate"> | |
| /// The asynchronous delegate to execute within the transactional context. The returned <see cref="bool"/> indicates | |
| /// whether the transaction should commit (<see langword="true"/>) or be rolled back (<see langword="false"/>). | |
| /// </param> | |
| /// <param name="useExclusiveLock"> | |
| /// When <see langword="true"/>, all transactional states accessed during this transaction will acquire exclusive locks, | |
| /// even for read operations. This avoids reader-to-writer lock upgrade conflicts under high contention, at the cost of | |
| /// reduced concurrent access. When <see langword="false"/>, the default shared/read locking behavior is used. | |
| /// </param> | |
| /// <returns> | |
| /// A <see cref="Task"/> that completes when the transaction has finished executing. The transaction will commit | |
| /// if the delegate returns <see langword="true"/> and no exception is thrown, otherwise it will be rolled back. | |
| /// </returns> | |
| /// <remarks> | |
| /// This overload is useful when the decision to commit or roll back depends on logic executed inside the transaction. | |
| /// Like the other overload, enabling <paramref name="useExclusiveLock"/> can improve behavior in high-contention | |
| /// scenarios by removing lock upgrade conflicts, but it can also decrease concurrency. | |
| /// </remarks> | |
| /// <example> | |
| /// <code> | |
| /// await transactionClient.RunTransaction( | |
| /// TransactionOption.CreateOrJoin, | |
| /// async () => | |
| /// { | |
| /// var current = await counterGrain.GetValue(); | |
| /// if (current >= 100) | |
| /// { | |
| /// // Abort the transaction without committing. | |
| /// return false; | |
| /// } | |
| /// | |
| /// await counterGrain.Increment(); | |
| /// return true; | |
| /// }, | |
| /// useExclusiveLock: true); | |
| /// </code> | |
| /// </example> |
|
The idea sounds good to me. I am not entirely sure of the benefit to using PerformRead+ExclusiveLock vs using PerformUpdate (other than avoiding the state copy + storage write). EDIT: I see you answered that in the issue thread. |
Motivation
The detailed motivation is described in #9898.
Changes.
The core idea in this PR is to change how
isReadparam is propagated when an exclusive lock is requested, in theReaderWriterLock.csFind(..., isRead, ...)->Find(..., isRead && !exclusiveLock, ...)HasConflict(isRead, ...)->HasConflict(isRead && !exclusiveLock, ...)All other changes are made to support this behavior.
Example
Users attach
UseExclusiveLockAttributeto acquire an exclusive lock.Since default value of
UseExclusiveLockis false, this this change is backward-compatible, and existing code paths remain unaffected unless the attribute is explicitly attached.DIsclamer
This change has not been fully tested and should not be merged into the main branch.
Microsoft Reviewers: Open in CodeFlow