RavenDB-22973 Fixing the check - we must not throw if WaitForJournalS…

…tateToBeUpdated() has completed due to env dispose or cancellation token. Fixing possible NRE in the test.
arekpalinski · Oct 28, 2024 · 6d2059a · 6d2059a
1 parent 35250be
commit 6d2059a
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 7 deletions.
diff --git a/src/Voron/Impl/Journal/WriteAheadJournal.cs b/src/Voron/Impl/Journal/WriteAheadJournal.cs
@@ -739,17 +739,17 @@ private void ApplyJournalStateAfterFlush(CancellationToken token,
                     ExceptionDispatchInfo edi = null;
                     var sp = Stopwatch.StartNew();
 
-                    var appliedSuccessfully = false;
+                    var executedSuccessfully = false;
 
-                    WaitForJournalStateToBeUpdated(token, transactionPersistentContext, txw =>
+                    var applied = WaitForJournalStateToBeUpdated(token, transactionPersistentContext, txw =>
                     {
                         try
                         {
                             txw.AppliedJournalStateAfterFlush = true;
                             txw.UpdateDataPagerState(dataPagerState);
                             UpdateJournalStateUnderWriteTransactionLock(txw, bufferOfPageFromScratchBuffersToFree, record);
 
-                            appliedSuccessfully = true;
+                            executedSuccessfully = true;
 
                             if (_waj._logger.IsDebugEnabled)
                                 _waj._logger.Debug($"Updated journal state under write tx lock (txId: {txw.Id}) after waiting for {sp.Elapsed}");
@@ -766,7 +766,7 @@ private void ApplyJournalStateAfterFlush(CancellationToken token,
 
                     if (edi != null)
                         edi.Throw();
-                    else if (appliedSuccessfully == false)
+                    else if (applied && executedSuccessfully == false)
                         throw new InvalidOperationException($"Journal state was not applied successfully after the flush (waited - {sp.Elapsed}, last flushed tx: id - {record.TransactionId}, written to journal - {record.WrittenToJournalNumber})");
                 }
                 finally
@@ -775,7 +775,7 @@ private void ApplyJournalStateAfterFlush(CancellationToken token,
                 }
             }
 
-            private void WaitForJournalStateToBeUpdated(CancellationToken token, TransactionPersistentContext transactionPersistentContext,
+            private bool WaitForJournalStateToBeUpdated(CancellationToken token, TransactionPersistentContext transactionPersistentContext,
                 Action<LowLevelTransaction> currentAction, ByteStringContext byteStringContext)
             {
                 _forTestingPurposes?.OnWaitForJournalStateToBeUpdated_BeforeAssigning_updateJournalStateAfterFlush?.Invoke();
@@ -796,7 +796,7 @@ private void WaitForJournalStateToBeUpdated(CancellationToken token, Transaction
                         }
                         catch (OperationCanceledException)
                         {
-                            return; // we disposed the server
+                            return false; // we disposed the server
                         }
                         catch (TimeoutException)
                         {
@@ -824,7 +824,7 @@ private void WaitForJournalStateToBeUpdated(CancellationToken token, Transaction
 
                                 case 1:
                                     // cancellation token
-                                    return;
+                                    return false;
 
                                 case WaitHandle.WaitTimeout:
                                     // timeout
@@ -844,6 +844,8 @@ private void WaitForJournalStateToBeUpdated(CancellationToken token, Transaction
                     }
                     // if it was changed, this means that we are done - note that it could be applied by the commit of another write transaction
                 } while (currentAction == _updateJournalStateAfterFlush);
+
+                return true;
             }
 
             private void UpdateJournalStateUnderWriteTransactionLock(LowLevelTransaction txw,

diff --git a/test/SlowTests/Voron/Issues/RavenDB_22973.cs b/test/SlowTests/Voron/Issues/RavenDB_22973.cs
@@ -359,6 +359,8 @@ public void Must_not_omit_UpdateJournalStateUnderWriteTransactionLock_during_flu
             mre.Set();
         };
 
+        var flushStarted = new ManualResetEvent(false);
+
         Thread t = null;
 
         Env.ForTestingPurposesOnly().OnWriteTransactionCompleted += tx =>
@@ -371,6 +373,8 @@ public void Must_not_omit_UpdateJournalStateUnderWriteTransactionLock_during_flu
                 });
                 t.Start();
 
+                flushStarted.Set();
+
                 mre.WaitOne();
             }
         };
@@ -380,6 +384,8 @@ public void Must_not_omit_UpdateJournalStateUnderWriteTransactionLock_during_flu
         Env.Journal.Applicator.ForTestingPurposesOnly().OnWaitForJournalStateToBeUpdated_AfterAssigning_updateJournalStateAfterFlush = null;
         Env.ForTestingPurposesOnly().OnWriteTransactionCompleted = null;
 
+        flushStarted.WaitOne();
+
         t.Join();
 
         using (var txw2 = Env.WriteTransaction())