Skip to content

Commit 90fbe04

Browse files
paulirwinclaude
andauthored
Guard Commit() in TestTransactions to handle injected I/O failures, apache#1298 (apache#1312)
Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
1 parent c00eabb commit 90fbe04

1 file changed

Lines changed: 69 additions & 4 deletions

File tree

src/Lucene.Net.Tests/Index/TestTransactions.cs

Lines changed: 69 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using J2N.Threading;
2+
using Lucene.Net.Attributes;
23
using Lucene.Net.Documents;
34
using Lucene.Net.Index.Extensions;
45
using Lucene.Net.Store;
@@ -37,6 +38,7 @@ namespace Lucene.Net.Index
3738
using MockAnalyzer = Lucene.Net.Analysis.MockAnalyzer;
3839
using MockDirectoryWrapper = Lucene.Net.Store.MockDirectoryWrapper;
3940
using RAMDirectory = Lucene.Net.Store.RAMDirectory;
41+
using StackTraceHelper = Lucene.Net.Util.StackTraceHelper;
4042
using StringField = StringField;
4143

4244
[TestFixture]
@@ -169,8 +171,29 @@ public override void DoWork()
169171
return;
170172
}
171173

172-
writer1.Commit();
173-
writer2.Commit();
174+
// LUCENENET specific: deviates from upstream Java, which leaves
175+
// these Commit() calls unguarded. The test injects random I/O
176+
// failures (RandomFailure) for the whole prepare+commit region,
177+
// and Commit() writes the segments file footer via
178+
// SegmentInfos.FinishCommit, so an injected IOException can escape
179+
// Commit() into TimedThread.Run's catch and trip failed=true. That
180+
// is not the behavior under test: the test verifies the
181+
// transactional protocol holds *in the presence of* random I/O
182+
// failures, so a Commit() failure should be handled the same way as
183+
// a PrepareCommit() failure (roll back and abort the cycle). The
184+
// same limitation exists in upstream Java; it just surfaces rarely.
185+
// See https://github.com/apache/lucenenet/issues/1298.
186+
try
187+
{
188+
writer1.Commit();
189+
writer2.Commit();
190+
}
191+
catch (Exception t) when (t.IsThrowable())
192+
{
193+
writer1.Rollback();
194+
writer2.Rollback();
195+
return;
196+
}
174197
}
175198
finally
176199
{
@@ -280,15 +303,57 @@ public virtual void InitIndex(Directory dir)
280303

281304
[Test]
282305
public virtual void TestTransactions_Mem()
306+
{
307+
DoTestTransactions(new RandomFailure(), new RandomFailure());
308+
}
309+
310+
// LUCENENET specific: deterministic regression test for the race surfaced
311+
// intermittently in https://github.com/apache/lucenenet/issues/1298
312+
// (TestTransactions_Mem reporting "Expected: True Actual: False"). The
313+
// standard test relies on RandomFailure, which throws during PrepareCommit
314+
// ~100% of the time, so it almost never reaches Commit, making the
315+
// Commit-phase failure nearly impossible to reproduce. This variant injects
316+
// an exception that fires *only* inside SegmentInfos.FinishCommit, which is
317+
// on the call stack of writer.Commit() (it writes the segments file footer)
318+
// but never of writer.PrepareCommit(). That makes Commit() throw
319+
// deterministically while PrepareCommit() stays clean, exercising the
320+
// previously-unguarded Commit() path in IndexerThread.DoWork. Without the
321+
// Commit() try/catch added for #1298 this fails with "Expected: True
322+
// Actual: False"; with it, the Commit failure is treated as a recoverable
323+
// cycle abort and the test passes.
324+
[Test, LuceneNetSpecific]
325+
public virtual void TestTransactions_CommitFailure_IsHandled()
326+
{
327+
DoTestTransactions(new FailOnlyInFinishCommit(), new FailOnlyInFinishCommit());
328+
}
329+
330+
// Fires "now failing on purpose" only when the call stack is inside
331+
// SegmentInfos.FinishCommit (which is marked NoInlining so the stack frame
332+
// remains observable). That method is reachable from writer.Commit() but
333+
// not writer.PrepareCommit(), so PrepareCommit completes cleanly and the
334+
// test reaches the Commit() calls in IndexerThread.DoWork.
335+
private class FailOnlyInFinishCommit : Failure
336+
{
337+
public override void Eval(MockDirectoryWrapper dir)
338+
{
339+
if (TestTransactions.doFail &&
340+
StackTraceHelper.DoesStackTraceContainMethod(nameof(SegmentInfos), nameof(SegmentInfos.FinishCommit)))
341+
{
342+
throw new IOException("now failing on purpose");
343+
}
344+
}
345+
}
346+
347+
private void DoTestTransactions(Failure failure1, Failure failure2)
283348
{
284349
Console.WriteLine("start test");
285350
// we can't use non-ramdir on windows, because this test needs to double-write.
286351
MockDirectoryWrapper dir1 = new MockDirectoryWrapper(Random, new RAMDirectory());
287352
MockDirectoryWrapper dir2 = new MockDirectoryWrapper(Random, new RAMDirectory());
288353
dir1.PreventDoubleWrite = false;
289354
dir2.PreventDoubleWrite = false;
290-
dir1.FailOn(new RandomFailure());
291-
dir2.FailOn(new RandomFailure());
355+
dir1.FailOn(failure1);
356+
dir2.FailOn(failure2);
292357
dir1.FailOnOpenInput = false;
293358
dir2.FailOnOpenInput = false;
294359

0 commit comments

Comments
 (0)