Skip to content

Commit

Permalink
RavenDB-22257 Making the transaction owner check on tx dispose to be …
Browse files Browse the repository at this point in the history
…Debug only. Removing the usage of await in the scope of a write transaction in the test and making it sync. Added ConfigureAwait(true) to the transaction replay code to in order to try to get to the original thread (that opened the write tx) so the tests for the tx recorder won't fail in debug.
  • Loading branch information
arekpalinski committed May 9, 2024
1 parent 3894f29 commit 8ce3fa9
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 15 deletions.
2 changes: 1 addition & 1 deletion src/Raven.Server/Documents/ReplayTxCommandHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ internal static async IAsyncEnumerable<ReplayProgress> ReplayAsync(DocumentDatab
await using (var readersItr = readers.GetAsyncEnumerator())
{
await ReadStartRecordingDetailsAsync(readersItr, context, peepingTomStream);
while (await readersItr.MoveNextAsync())
while (await readersItr.MoveNextAsync().ConfigureAwait(true))
{
using (readersItr.Current)
{
Expand Down
16 changes: 10 additions & 6 deletions src/Voron/Impl/LowLevelTransaction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -935,8 +935,7 @@ public void Dispose()
if (_txState.HasFlag(TxState.Disposed))
return;

if (Flags == TransactionFlags.ReadWrite && NativeMemory.CurrentThreadStats != CurrentTransactionHolder)
ThrowDisposeOfTxMustBeCalledOnTheSameThreadThatCreatedIt();
EnsureDisposeOfWriteTxIsOnTheSameThreadThatCreatedIt();

try
{
Expand Down Expand Up @@ -1326,11 +1325,16 @@ private static void ThrowAlreadyCommitted()
throw new InvalidOperationException("Cannot commit already committed transaction.");
}

private void ThrowDisposeOfTxMustBeCalledOnTheSameThreadThatCreatedIt()
[Conditional("DEBUG")]
private void EnsureDisposeOfWriteTxIsOnTheSameThreadThatCreatedIt()
{
throw new InvalidOperationException($"Dispose of the transaction must be called from the same thread that created it. " +
$"Transaction {Id} (Flags: {Flags}) was created by {CurrentTransactionHolder.Name}, thread Id: {CurrentTransactionHolder.ManagedThreadId}. " +
$"The dispose was called from {NativeMemory.CurrentThreadStats.Name}, thread Id: {NativeMemory.CurrentThreadStats.ManagedThreadId}");
if (Flags == TransactionFlags.ReadWrite && NativeMemory.CurrentThreadStats != CurrentTransactionHolder)
{
throw new InvalidOperationException($"Dispose of the write transaction must be called from the same thread that created it. " +
$"Transaction {Id} (Flags: {Flags}) was created by {CurrentTransactionHolder.Name}, thread Id: {CurrentTransactionHolder.ManagedThreadId}. " +
$"The dispose was called from {NativeMemory.CurrentThreadStats.Name}, thread Id: {NativeMemory.CurrentThreadStats.ManagedThreadId}. " +
$"Do you have any await call in the scope of the write transaction?");
}
}

public void Rollback()
Expand Down
11 changes: 6 additions & 5 deletions test/RachisTests/BasicTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Threading;
using System.Threading.Tasks;
using Raven.Client.ServerWide;
using Raven.Client.Util;
using Raven.Server.Rachis;
using Raven.Server.ServerWide.Context;
using Sparrow.Json;
Expand Down Expand Up @@ -59,10 +60,10 @@ await ActionWithLeader(async l =>
}

[Fact]
public async Task RavenDB_13659()
public void RavenDB_13659()
{
var leader = await CreateNetworkAndGetLeader(1);
var mre = new AsyncManualResetEvent();
var leader = AsyncHelpers.RunSync(() => CreateNetworkAndGetLeader(1));
var mre = new ManualResetEventSlim();
var tcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
var currentThread = NativeMemory.CurrentThreadStats.ManagedThreadId;

Expand Down Expand Up @@ -90,12 +91,12 @@ public async Task RavenDB_13659()
using (leader.ContextPool.AllocateOperationContext(out ClusterOperationContext context))
using (context.OpenWriteTransaction())
{
await mre.WaitAsync();
mre.Wait();
leader.SetNewStateInTx(context, RachisState.Follower, null, leader.CurrentTerm, "deadlock");
context.Transaction.Commit();
}

await tcs.Task;
AsyncHelpers.RunSync(() => tcs.Task);
}
}
}
9 changes: 6 additions & 3 deletions test/SlowTests/Voron/Issues/RavenDB_22257.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,11 @@ protected override void Configure(StorageEnvironmentOptions options)

options.ManualFlushing = true;
}

#if DEBUG
[RavenFact(RavenTestCategory.Voron)]
#else
[RavenFact(RavenTestCategory.Voron, Skip = "This test relies on Debug only check - LowLevelTransaction.EnsureDisposeOfWriteTxIsOnTheSameThreadThatCreatedIt()")]
#endif
public void MustNotAllowToHaveTwoWriteTransactionsConcurrently()
{
RequireFileBasedPager();
Expand All @@ -48,7 +51,7 @@ public void MustNotAllowToHaveTwoWriteTransactionsConcurrently()
{
ex = Assert.Throws<InvalidOperationException>(() =>
{
txw1.Dispose(); // this is supposed to throw because we're attempting to dispose write tx from a different thread
txw1.Dispose(); // this is supposed to throw in Debug because we're attempting to dispose write tx from a different thread

txw2 = Env.NewLowLevelTransaction(new TransactionPersistentContext(), TransactionFlags.ReadWrite);

Expand All @@ -60,7 +63,7 @@ public void MustNotAllowToHaveTwoWriteTransactionsConcurrently()

newTransactionThread.Join();

Assert.StartsWith("Dispose of the transaction must be called from the same thread that created it. Transaction 2 (Flags: ReadWrite) was created by", ex.Message);
Assert.StartsWith("Dispose of the write transaction must be called from the same thread that created it. Transaction 2 (Flags: ReadWrite) was created by", ex.Message);
});

/*
Expand Down

0 comments on commit 8ce3fa9

Please sign in to comment.