Skip to content

Commit 2a70a0d

Browse files
committed
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 b85cbd1 commit 2a70a0d

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
@@ -26,7 +26,6 @@
2626
import org.elasticsearch.common.xcontent.XContentHelper;
2727
import org.elasticsearch.common.xcontent.json.JsonXContent;
2828
import org.elasticsearch.index.IndexModule;
29-
import org.elasticsearch.index.IndexSettings;
3029
import org.elasticsearch.join.ParentJoinPlugin;
3130
import org.elasticsearch.plugins.Plugin;
3231
import org.elasticsearch.test.ESIntegTestCase;
@@ -60,8 +59,6 @@ protected Collection<Class<? extends Plugin>> transportClientPlugins() {
6059
@Override
6160
public Settings indexSettings() {
6261
Settings.Builder builder = Settings.builder().put(super.indexSettings())
63-
// AwaitsFix: https://github.com/elastic/elasticsearch/issues/33318
64-
.put(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), false)
6562
// aggressive filter caching so that we can assert on the filter cache size
6663
.put(IndexModule.INDEX_QUERY_CACHE_ENABLED_SETTING.getKey(), true)
6764
.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
@@ -213,7 +213,7 @@ private void fillParallelArray(ScoreDoc[] scoreDocs, ParallelArray parallelArray
213213
}
214214

215215
private TopDocs searchOperations(ScoreDoc after) throws IOException {
216-
final Query rangeQuery = LongPoint.newRangeQuery(SeqNoFieldMapper.NAME, lastSeenSeqNo + 1, toSeqNo);
216+
final Query rangeQuery = LongPoint.newRangeQuery(SeqNoFieldMapper.NAME, Math.max(fromSeqNo, lastSeenSeqNo), toSeqNo);
217217
final Sort sortedBySeqNoThenByTerm = new Sort(
218218
new SortField(SeqNoFieldMapper.NAME, SortField.Type.LONG),
219219
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
@@ -183,12 +183,17 @@ public void testDedupByPrimaryTerm() throws Exception {
183183
}
184184
}
185185
long maxSeqNo = engine.getLocalCheckpointTracker().getMaxSeqNo();
186-
try (Translog.Snapshot snapshot = engine.newChangesSnapshot("test", mapperService, 0, maxSeqNo, false)) {
186+
engine.refresh("test");
187+
Engine.Searcher searcher = engine.acquireSearcher("test", Engine.SearcherScope.INTERNAL);
188+
try (Translog.Snapshot snapshot = new LuceneChangesSnapshot(searcher, mapperService, between(1, 100), 0, maxSeqNo, false)) {
189+
searcher = null;
187190
Translog.Operation op;
188191
while ((op = snapshot.next()) != null) {
189192
assertThat(op.toString(), op.primaryTerm(), equalTo(latestOperations.get(op.seqNo())));
190193
}
191194
assertThat(snapshot.skippedOperations(), equalTo(totalOps - latestOperations.size()));
195+
} finally {
196+
IOUtils.close(searcher);
192197
}
193198
}
194199

0 commit comments

Comments
 (0)