apache / apache/lucene

pendingCommit in IndexWriter is not thoroughly tested [LUCENE-3116]

Open
#4,189 6 comments 0 reactions 0 assignees View on GitHub
affects-version:3.2 affects-version:4.0-ALPHA legacy-jira-fix-version:4.9 legacy-jira-fix-version:6.0 legacy-jira-priority:Major module:core/index type:test
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.