From 77e3fb84e0d9f091a7239de95722865b503e404c Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Wed, 26 Mar 2025 18:54:21 +0100 Subject: [PATCH] Load FieldInfos from store if not yet initialised through a refresh on IndexShard (#125650) 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 --- docs/changelog/125650.yaml | 7 ++++ .../elasticsearch/index/shard/IndexShard.java | 40 ++++++++++++++----- .../SearchableSnapshotsSearchIntegTests.java | 5 +++ 3 files changed, 42 insertions(+), 10 deletions(-) create mode 100644 docs/changelog/125650.yaml diff --git a/docs/changelog/125650.yaml b/docs/changelog/125650.yaml new file mode 100644 index 0000000000000..89bacc3cee4a1 --- /dev/null +++ b/docs/changelog/125650.yaml @@ -0,0 +1,7 @@ +pr: 125650 +summary: Load `FieldInfos` from store if not yet initialised through a refresh on + `IndexShard` +area: Search +type: bug +issues: + - 125483 diff --git a/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java b/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java index f199e8f202959..e67debfe443bf 100644 --- a/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java @@ -156,6 +156,8 @@ import java.io.Closeable; import java.io.IOException; import java.io.PrintStream; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.VarHandle; import java.nio.channels.ClosedByInterruptException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; @@ -413,7 +415,6 @@ public IndexShard( this.refreshFieldHasValueListener = new RefreshFieldHasValueListener(); this.relativeTimeInNanosSupplier = relativeTimeInNanosSupplier; this.indexCommitListener = indexCommitListener; - this.fieldInfos = FieldInfos.EMPTY; } public ThreadPool getThreadPool() { @@ -1015,12 +1016,26 @@ private Engine.IndexResult applyIndexOperation( return index(engine, operation); } - public void setFieldInfos(FieldInfos fieldInfos) { - this.fieldInfos = fieldInfos; + private static final VarHandle FIELD_INFOS; + + static { + try { + FIELD_INFOS = MethodHandles.lookup().findVarHandle(IndexShard.class, "fieldInfos", FieldInfos.class); + } catch (Exception e) { + throw new ExceptionInInitializerError(e); + } } public FieldInfos getFieldInfos() { - return fieldInfos; + var res = fieldInfos; + if (res == null) { + // don't replace field infos loaded via the refresh listener to avoid overwriting the field with an older version of the + // field infos when racing with a refresh + var read = loadFieldInfos(); + var existing = (FieldInfos) FIELD_INFOS.compareAndExchange(this, null, read); + return existing == null ? read : existing; + } + return res; } public static Engine.Index prepareIndex( @@ -4114,16 +4129,21 @@ public void beforeRefresh() {} @Override public void afterRefresh(boolean didRefresh) { - if (enableFieldHasValue && (didRefresh || fieldInfos == FieldInfos.EMPTY)) { - try (Engine.Searcher hasValueSearcher = getEngine().acquireSearcher("field_has_value")) { - setFieldInfos(FieldInfos.getMergedFieldInfos(hasValueSearcher.getIndexReader())); - } catch (AlreadyClosedException ignore) { - // engine is closed - no updated FieldInfos is fine - } + if (enableFieldHasValue && (didRefresh || fieldInfos == null)) { + FIELD_INFOS.setRelease(IndexShard.this, loadFieldInfos()); } } } + private FieldInfos loadFieldInfos() { + try (Engine.Searcher hasValueSearcher = getEngine().acquireSearcher("field_has_value")) { + return FieldInfos.getMergedFieldInfos(hasValueSearcher.getIndexReader()); + } catch (AlreadyClosedException ignore) { + // engine is closed - no update to FieldInfos is fine + } + return FieldInfos.EMPTY; + } + /** * Returns the shard-level field stats, which includes the number of segments in the latest NRT reader of this shard * and the total number of fields across those segments. diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsSearchIntegTests.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsSearchIntegTests.java index f77a3f5698c98..1e44ba8766be6 100644 --- a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsSearchIntegTests.java +++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsSearchIntegTests.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.searchablesnapshots; +import org.elasticsearch.action.fieldcaps.FieldCapabilitiesResponse; import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.action.search.SearchType; @@ -125,5 +126,9 @@ public void testKeywordSortedQueryOnFrozen() throws Exception { assertThat(searchResponse.getTotalShards(), equalTo(20)); assertThat(searchResponse.getHits().getTotalHits().value(), equalTo(4L)); }); + + // check that field_caps empty field filtering works as well + FieldCapabilitiesResponse response = client().prepareFieldCaps(mountedIndices).setFields("*").setincludeEmptyFields(false).get(); + assertNotNull(response.getField("keyword")); } }