pendingCommit in IndexWriter is not thoroughly tested [LUCENE-3116]
- Dominant language
- Java
- Stars
- 3.6k
- Forks
- 1.4k
- Avg merge
- 2d 11h
- Merged PRs (30d)
- 88
Description
When working on #4157, I had a copy-paste error in my patch (see revision 1124307 and corrected in 1124316), I replaced pendingCommit by segmentInfos in IndexWriter, corrected by the following patch:
```
--- lucene/dev/trunk/lucene/src/java/org/apache/lucene/index/IndexWriter.java (original)
+++ lucene/dev/trunk/lucene/src/java/org/apache/lucene/index/IndexWriter.java Wed May 18 16:16:29 2011
`@@` -2552,7 +2552,7 `@@` public class IndexWriter implements Clos
lastCommitChangeCount = pendingCommitChangeCount;
segmentInfos.updateGeneration(pendingCommit);
segmentInfos.setUserData(pendingCommit.getUserData());
- rollbackSegments = segmentInfos.createBackupSegmentInfos(true);
+ rollbackSegments = pendingCommit.createBackupSegmentInfos(true);
deleter.checkpoint(pendingCommit, true);
} finally {
// Matches the incRef done in startCommit:
```
This did not cause any test failure.
On IRC, Mike said:
> [19:21] mikemccand: ThetaPh1: hmm
> [19:21] mikemccand: well
> [19:22] mikemccand: pendingCommit and sis only differ while commit() is running
> [19:22] mikemccand: ie if a thread starts commit
> [19:22] mikemccand: but fsync is taking a long time
> [19:22] mikemccand: and another thread makes a change to sis
> [19:22] ThetaPh1: ok so hard to find that bug
> [19:22] mikemccand: we need our mock dir wrapper to sometimes take a long time syncing\.\.\.\.
Maybe we need such a test, I feel bad when such stupid changes don't make any test fail.
---
Migrated from [LUCENE-3116](https://issues.apache.org/jira/browse/LUCENE-3116) by Uwe Schindler (@uschindler), updated May 09 2016
Contributor guide
Research direction
Start by reading the commit and startCommit paths in lucene/src/java/org/apache/lucene/index/IndexWriter.java, then inspect the existing mock directory wrapper used for filesystem tests. Reproduce a slow fsync while another thread changes segmentInfos, and add coverage for the pendingCommit rollback state. Done means the regression test fails for the incorrect implementation and passes with the corrected behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- search, testing
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100