apache / apache/lucene

Openning IndexWriter with an existing commit incorrectly marks a index as changed

Open
#13,151 0 comments 0 reactions 0 assignees View on GitHub
type:bug
Dominant language
Java
Stars
3.6k
Forks
1.4k
Avg merge
2d 11h
Merged PRs (30d)
88

Description

### Description

Hey,

I found what looks to be a bug here:
https://github.com/apache/lucene/blob/releases/lucene/9.9.0/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java#L1102-L1105

```
if (commit != null) {
// Swap out all segments, but, keep metadata in
// SegmentInfos, like version & generation, to
// preserve write-once. This is important if
// readers are open against the future commit
// points.
if (commit.getDirectory() != directoryOrig) {
throw new IllegalArgumentException(
"IndexCommit's directory doesn't match my directory, expected="
+ directoryOrig
+ ", got="
+ commit.getDirectory());
}

SegmentInfos oldInfos =
SegmentInfos.readCommit(directoryOrig, commit.getSegmentsFileName());
segmentInfos.replace(oldInfos);
changed();

if (infoStream.isEnabled("IW")) {
infoStream.message(
"IW", "init: loaded commit \"" + commit.getSegmentsFileName() + "\"");
}
}
```

When we called `segmentsInfos.replace` point, there is no guarantee that the information replaced actually changed. In the cases where information actually isn't changed, we still call changed(). This means the next commit we attempt will actually write a new commit rather than being a no-op.

I have some ideas on how to fix, like add a check to .replace() to see if information actually changed, but want to hear other thoughts.

### Version and environment details

_No response_

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.