Skip to content

Commit 24d60c7

Browse files
authored
Fix from_range in search_after in changes snapshot (#33335)
We can have multiple documents in Lucene with the same seq_no for parent-child documents (or without rollback). In this case, the usage "lastSeenSeqNo + 1" is an off-by-one error as it may miss some documents. This error merely affects the `skippedOperations` contract. See: #33222 (comment) Closes #33318
1 parent 42424af commit 24d60c7

File tree

3 files changed

+7
-5
lines changed

3 files changed

+7
-5
lines changed

modules/parent-join/src/test/java/org/elasticsearch/join/query/ParentChildTestCase.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import org.elasticsearch.common.xcontent.XContentHelper;
2626
import org.elasticsearch.common.xcontent.json.JsonXContent;
2727
import org.elasticsearch.index.IndexModule;
28-
import org.elasticsearch.index.IndexSettings;
2928
import org.elasticsearch.join.ParentJoinPlugin;
3029
import org.elasticsearch.plugins.Plugin;
3130
import org.elasticsearch.test.ESIntegTestCase;
@@ -59,8 +58,6 @@ protected Collection<Class<? extends Plugin>> transportClientPlugins() {
5958
@Override
6059
public Settings indexSettings() {
6160
Settings.Builder builder = Settings.builder().put(super.indexSettings())
62-
// AwaitsFix: https://github.com/elastic/elasticsearch/issues/33318
63-
.put(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), false)
6461
// aggressive filter caching so that we can assert on the filter cache size
6562
.put(IndexModule.INDEX_QUERY_CACHE_ENABLED_SETTING.getKey(), true)
6663
.put(IndexModule.INDEX_QUERY_CACHE_EVERYTHING_SETTING.getKey(), true);

server/src/main/java/org/elasticsearch/index/engine/LuceneChangesSnapshot.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ private void fillParallelArray(ScoreDoc[] scoreDocs, ParallelArray parallelArray
212212
}
213213

214214
private TopDocs searchOperations(ScoreDoc after) throws IOException {
215-
final Query rangeQuery = LongPoint.newRangeQuery(SeqNoFieldMapper.NAME, lastSeenSeqNo + 1, toSeqNo);
215+
final Query rangeQuery = LongPoint.newRangeQuery(SeqNoFieldMapper.NAME, Math.max(fromSeqNo, lastSeenSeqNo), toSeqNo);
216216
final Sort sortedBySeqNoThenByTerm = new Sort(
217217
new SortField(SeqNoFieldMapper.NAME, SortField.Type.LONG),
218218
new SortField(SeqNoFieldMapper.PRIMARY_TERM_NAME, SortField.Type.LONG, true)

server/src/test/java/org/elasticsearch/index/engine/LuceneChangesSnapshotTests.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,12 +182,17 @@ public void testDedupByPrimaryTerm() throws Exception {
182182
}
183183
}
184184
long maxSeqNo = engine.getLocalCheckpointTracker().getMaxSeqNo();
185-
try (Translog.Snapshot snapshot = engine.newChangesSnapshot("test", mapperService, 0, maxSeqNo, false)) {
185+
engine.refresh("test");
186+
Engine.Searcher searcher = engine.acquireSearcher("test", Engine.SearcherScope.INTERNAL);
187+
try (Translog.Snapshot snapshot = new LuceneChangesSnapshot(searcher, mapperService, between(1, 100), 0, maxSeqNo, false)) {
188+
searcher = null;
186189
Translog.Operation op;
187190
while ((op = snapshot.next()) != null) {
188191
assertThat(op.toString(), op.primaryTerm(), equalTo(latestOperations.get(op.seqNo())));
189192
}
190193
assertThat(snapshot.skippedOperations(), equalTo(totalOps - latestOperations.size()));
194+
} finally {
195+
IOUtils.close(searcher);
191196
}
192197
}
193198

0 commit comments

Comments
 (0)