From 119931083fd9010cb3877226be88e24db356d9dc Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Wed, 26 Mar 2025 10:55:12 +0100 Subject: [PATCH 1/5] Load field infos from directory if not yet cached on IndexShard If we don't yet have a cached value ready to go for the field infos, lets load it from the directory. Fixes #125383 --- .../elasticsearch/index/shard/IndexShard.java | 26 +++++++++++++------ .../SearchableSnapshotsSearchIntegTests.java | 5 ++++ 2 files changed, 23 insertions(+), 8 deletions(-) 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 bb091fa0c409b..d33a6d4877598 100644 --- a/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java @@ -418,7 +418,6 @@ public IndexShard( this.refreshFieldHasValueListener = new RefreshFieldHasValueListener(); this.relativeTimeInNanosSupplier = relativeTimeInNanosSupplier; this.indexCommitListener = indexCommitListener; - this.fieldInfos = FieldInfos.EMPTY; } public ThreadPool getThreadPool() { @@ -1025,7 +1024,11 @@ public void setFieldInfos(FieldInfos fieldInfos) { } public FieldInfos getFieldInfos() { - return fieldInfos; + var res = fieldInfos; + if (res == null) { + return loadFieldInfos(); + } + return res; } public static Engine.Index prepareIndex( @@ -4131,16 +4134,23 @@ 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)) { + loadFieldInfos(); } } } + private FieldInfos loadFieldInfos() { + try (Engine.Searcher hasValueSearcher = getEngine().acquireSearcher("field_has_value")) { + var res = FieldInfos.getMergedFieldInfos(hasValueSearcher.getIndexReader()); + setFieldInfos(res); + return res; + } catch (AlreadyClosedException ignore) { + // engine is closed - no update to3 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")); } } From b98920efb7e59978534373420a5fe8385c62b51a Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Wed, 26 Mar 2025 11:05:34 +0100 Subject: [PATCH 2/5] no races --- .../elasticsearch/index/shard/IndexShard.java | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) 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 d33a6d4877598..0a329f8abd689 100644 --- a/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java @@ -155,6 +155,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; @@ -1019,14 +1021,24 @@ 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() { var res = fieldInfos; if (res == null) { - return loadFieldInfos(); + // 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; } @@ -4135,7 +4147,7 @@ public void beforeRefresh() {} @Override public void afterRefresh(boolean didRefresh) { if (enableFieldHasValue && (didRefresh || fieldInfos == null)) { - loadFieldInfos(); + FIELD_INFOS.setRelease(IndexShard.this, loadFieldInfos()); } } } @@ -4143,7 +4155,6 @@ public void afterRefresh(boolean didRefresh) { private FieldInfos loadFieldInfos() { try (Engine.Searcher hasValueSearcher = getEngine().acquireSearcher("field_has_value")) { var res = FieldInfos.getMergedFieldInfos(hasValueSearcher.getIndexReader()); - setFieldInfos(res); return res; } catch (AlreadyClosedException ignore) { // engine is closed - no update to3 FieldInfos is fine From fdd7f33b751feb43ede015c6ef588d8d046a799e Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Wed, 26 Mar 2025 11:10:08 +0100 Subject: [PATCH 3/5] cleanup --- .../main/java/org/elasticsearch/index/shard/IndexShard.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 0a329f8abd689..a8c6ff40bf8dd 100644 --- a/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java @@ -4154,8 +4154,7 @@ public void afterRefresh(boolean didRefresh) { private FieldInfos loadFieldInfos() { try (Engine.Searcher hasValueSearcher = getEngine().acquireSearcher("field_has_value")) { - var res = FieldInfos.getMergedFieldInfos(hasValueSearcher.getIndexReader()); - return res; + return FieldInfos.getMergedFieldInfos(hasValueSearcher.getIndexReader()); } catch (AlreadyClosedException ignore) { // engine is closed - no update to3 FieldInfos is fine } From 4addc4166295a8b4fecc1f09bace3d622461a498 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Wed, 26 Mar 2025 11:10:48 +0100 Subject: [PATCH 4/5] Update docs/changelog/125650.yaml --- docs/changelog/125650.yaml | 7 +++++++ 1 file changed, 7 insertions(+) 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 From d392f4fb95e25798885dbf8f07360866388c34a1 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Wed, 26 Mar 2025 17:45:57 +0100 Subject: [PATCH 5/5] Fix typo in IndexShard.java comment --- .../src/main/java/org/elasticsearch/index/shard/IndexShard.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 3162e5a4524ab..28fd80cf5eafa 100644 --- a/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java @@ -4291,7 +4291,7 @@ 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 to3 FieldInfos is fine + // engine is closed - no update to FieldInfos is fine } return FieldInfos.EMPTY; }