Skip to content

Commit 63be3d2

Browse files
Load FieldInfos from store if not yet initialised through a refresh on IndexShard (#125650) (#125703)
Load field caps from store if they haven't been initialised through a refresh yet. Keep the plain reads to not mess with performance characteristics too much on the good path but protect against confusing races when loading field infos now (that probably should have been ordered stores in the first place but this was safe due to other locks/volatiles on the refresh path). Closes #125483
1 parent 0824f9a commit 63be3d2

File tree

3 files changed

+42
-10
lines changed

3 files changed

+42
-10
lines changed

docs/changelog/125650.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
pr: 125650
2+
summary: Load `FieldInfos` from store if not yet initialised through a refresh on
3+
`IndexShard`
4+
area: Search
5+
type: bug
6+
issues:
7+
- 125483

server/src/main/java/org/elasticsearch/index/shard/IndexShard.java

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,8 @@
156156
import java.io.Closeable;
157157
import java.io.IOException;
158158
import java.io.PrintStream;
159+
import java.lang.invoke.MethodHandles;
160+
import java.lang.invoke.VarHandle;
159161
import java.nio.channels.ClosedByInterruptException;
160162
import java.nio.charset.StandardCharsets;
161163
import java.util.ArrayList;
@@ -413,7 +415,6 @@ public IndexShard(
413415
this.refreshFieldHasValueListener = new RefreshFieldHasValueListener();
414416
this.relativeTimeInNanosSupplier = relativeTimeInNanosSupplier;
415417
this.indexCommitListener = indexCommitListener;
416-
this.fieldInfos = FieldInfos.EMPTY;
417418
}
418419

419420
public ThreadPool getThreadPool() {
@@ -1015,12 +1016,26 @@ private Engine.IndexResult applyIndexOperation(
10151016
return index(engine, operation);
10161017
}
10171018

1018-
public void setFieldInfos(FieldInfos fieldInfos) {
1019-
this.fieldInfos = fieldInfos;
1019+
private static final VarHandle FIELD_INFOS;
1020+
1021+
static {
1022+
try {
1023+
FIELD_INFOS = MethodHandles.lookup().findVarHandle(IndexShard.class, "fieldInfos", FieldInfos.class);
1024+
} catch (Exception e) {
1025+
throw new ExceptionInInitializerError(e);
1026+
}
10201027
}
10211028

10221029
public FieldInfos getFieldInfos() {
1023-
return fieldInfos;
1030+
var res = fieldInfos;
1031+
if (res == null) {
1032+
// don't replace field infos loaded via the refresh listener to avoid overwriting the field with an older version of the
1033+
// field infos when racing with a refresh
1034+
var read = loadFieldInfos();
1035+
var existing = (FieldInfos) FIELD_INFOS.compareAndExchange(this, null, read);
1036+
return existing == null ? read : existing;
1037+
}
1038+
return res;
10241039
}
10251040

10261041
public static Engine.Index prepareIndex(
@@ -4114,16 +4129,21 @@ public void beforeRefresh() {}
41144129

41154130
@Override
41164131
public void afterRefresh(boolean didRefresh) {
4117-
if (enableFieldHasValue && (didRefresh || fieldInfos == FieldInfos.EMPTY)) {
4118-
try (Engine.Searcher hasValueSearcher = getEngine().acquireSearcher("field_has_value")) {
4119-
setFieldInfos(FieldInfos.getMergedFieldInfos(hasValueSearcher.getIndexReader()));
4120-
} catch (AlreadyClosedException ignore) {
4121-
// engine is closed - no updated FieldInfos is fine
4122-
}
4132+
if (enableFieldHasValue && (didRefresh || fieldInfos == null)) {
4133+
FIELD_INFOS.setRelease(IndexShard.this, loadFieldInfos());
41234134
}
41244135
}
41254136
}
41264137

4138+
private FieldInfos loadFieldInfos() {
4139+
try (Engine.Searcher hasValueSearcher = getEngine().acquireSearcher("field_has_value")) {
4140+
return FieldInfos.getMergedFieldInfos(hasValueSearcher.getIndexReader());
4141+
} catch (AlreadyClosedException ignore) {
4142+
// engine is closed - no update to FieldInfos is fine
4143+
}
4144+
return FieldInfos.EMPTY;
4145+
}
4146+
41274147
/**
41284148
* Returns the shard-level field stats, which includes the number of segments in the latest NRT reader of this shard
41294149
* and the total number of fields across those segments.

x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsSearchIntegTests.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
package org.elasticsearch.xpack.searchablesnapshots;
99

10+
import org.elasticsearch.action.fieldcaps.FieldCapabilitiesResponse;
1011
import org.elasticsearch.action.index.IndexRequestBuilder;
1112
import org.elasticsearch.action.search.SearchRequest;
1213
import org.elasticsearch.action.search.SearchType;
@@ -125,5 +126,9 @@ public void testKeywordSortedQueryOnFrozen() throws Exception {
125126
assertThat(searchResponse.getTotalShards(), equalTo(20));
126127
assertThat(searchResponse.getHits().getTotalHits().value(), equalTo(4L));
127128
});
129+
130+
// check that field_caps empty field filtering works as well
131+
FieldCapabilitiesResponse response = client().prepareFieldCaps(mountedIndices).setFields("*").setincludeEmptyFields(false).get();
132+
assertNotNull(response.getField("keyword"));
128133
}
129134
}

0 commit comments

Comments
 (0)