From 1066c8c8f6c47f626766327ff72547415c3cf8f1 Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Fri, 31 Oct 2025 08:52:28 -0700 Subject: [PATCH 1/5] Initial spike of percolator field fallback compatibility --- .../percolator/src/main/java/module-info.java | 1 + .../percolator/PercolateQueryBuilder.java | 97 ++++++++++++++----- .../percolator/PercolatorFieldMapper.java | 22 +++-- 3 files changed, 86 insertions(+), 34 deletions(-) diff --git a/modules/percolator/src/main/java/module-info.java b/modules/percolator/src/main/java/module-info.java index 51848d92a5e6c..392d5d959be4e 100644 --- a/modules/percolator/src/main/java/module-info.java +++ b/modules/percolator/src/main/java/module-info.java @@ -16,4 +16,5 @@ requires org.apache.lucene.memory; requires org.apache.lucene.queries; requires org.apache.lucene.sandbox; + requires org.elasticsearch.logging; } diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java index 468836232771e..10dc046c18f9a 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java @@ -63,6 +63,11 @@ import org.elasticsearch.index.query.SearchExecutionContext; import org.elasticsearch.indices.breaker.CircuitBreakerService; import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; +import org.elasticsearch.logging.LogManager; +import org.elasticsearch.logging.Logger; +import org.elasticsearch.search.lookup.Source; +import org.elasticsearch.search.lookup.SourceFilter; +import org.elasticsearch.search.lookup.SourceProvider; import org.elasticsearch.xcontent.ConstructingObjectParser; import org.elasticsearch.xcontent.NamedXContentRegistry; import org.elasticsearch.xcontent.ParseField; @@ -86,6 +91,8 @@ import static org.elasticsearch.xcontent.ConstructingObjectParser.optionalConstructorArg; public class PercolateQueryBuilder extends AbstractQueryBuilder { + private static final Logger LOGGER = LogManager.getLogger(PercolateQueryBuilder.class); + public static final String NAME = "percolate"; static final ParseField DOCUMENT_FIELD = new ParseField("document"); @@ -557,34 +564,37 @@ static PercolateQuery.QueryStore createStore(MappedFieldType queryBuilderFieldTy return docId -> { if (binaryDocValues.advanceExact(docId)) { BytesRef qbSource = binaryDocValues.binaryValue(); - try ( - InputStream in = new ByteArrayInputStream(qbSource.bytes, qbSource.offset, qbSource.length); - StreamInput input = new NamedWriteableAwareStreamInput(new InputStreamStreamInput(in, qbSource.length), registry) - ) { - // Query builder's content is stored via BinaryFieldMapper, which has a custom encoding - // to encode multiple binary values into a single binary doc values field. - // This is the reason we need to first read the number of values and - // then the length of the field value in bytes. - int numValues = input.readVInt(); - assert numValues == 1; - int valueLength = input.readVInt(); - assert valueLength > 0; - - TransportVersion transportVersion; - if (indexVersion.before(IndexVersions.V_8_8_0)) { - transportVersion = TransportVersion.fromId(indexVersion.id()); - } else { - transportVersion = TransportVersion.readVersion(input); + QueryBuilder queryBuilder; + + try { + queryBuilder = readQueryBuilder(qbSource, registry, indexVersion); + } catch (IllegalStateException e) { + // query builder is written in an incompatible format, fall-back to reading it from source + if (context.isSourceEnabled() == false) { + throw new ElasticsearchException( + "Unable to read percolator query. Original transport version is incompatible and source is " + + "unavailable on index [{}].", + context.index().getName() + ); } - // set the transportversion here - only read vints so far, so can change the version freely at this point - input.setTransportVersion(transportVersion); - - QueryBuilder queryBuilder = input.readNamedWriteable(QueryBuilder.class); - assert in.read() == -1; - queryBuilder = Rewriteable.rewrite(queryBuilder, context); - return queryBuilder.toQuery(context); + LOGGER.warn( + "Reading percolator query from source. For best performance, reindexing of index [{}] is required.", + context.index().getName() + ); + SourceProvider sourceProvider = context.createSourceProvider(new SourceFilter(null, null)); + Source source = sourceProvider.getSource(ctx, docId); + SourceToParse sourceToParse = new SourceToParse( + String.valueOf(docId), + source.internalSourceRef(), + source.sourceContentType() + ); + ParsedDocument document = context.parseDocument(sourceToParse); + BytesRef binaryValue = document.rootDoc().getBinaryValue(queryBuilderFieldType.name()); + queryBuilder = readQueryBuilder(binaryValue, registry, indexVersion); } + queryBuilder = Rewriteable.rewrite(queryBuilder, context); + return queryBuilder.toQuery(context); } else { return null; } @@ -592,6 +602,43 @@ static PercolateQuery.QueryStore createStore(MappedFieldType queryBuilderFieldTy }; } + private static QueryBuilder readQueryBuilder(BytesRef bytesRef, NamedWriteableRegistry registry, IndexVersion indexVersion) + throws IOException { + try ( + InputStream in = new ByteArrayInputStream(bytesRef.bytes, bytesRef.offset, bytesRef.length); + StreamInput input = new NamedWriteableAwareStreamInput(new InputStreamStreamInput(in, bytesRef.length), registry) + ) { + // Query builder's content is stored via BinaryFieldMapper, which has a custom encoding + // to encode multiple binary values into a single binary doc values field. + // This is the reason we need to first read the number of values and + // then the length of the field value in bytes. + int numValues = input.readVInt(); + assert numValues == 1; + int valueLength = input.readVInt(); + assert valueLength > 0; + + TransportVersion transportVersion; + if (indexVersion.before(IndexVersions.V_8_8_0)) { + transportVersion = TransportVersion.fromId(indexVersion.id()); + } else { + transportVersion = TransportVersion.readVersion(input); + } + + QueryBuilder queryBuilder; + + if (TransportVersion.isCompatible(transportVersion)) { + // set the transportversion here - only read vints so far, so can change the version freely at this point + input.setTransportVersion(transportVersion); + queryBuilder = input.readNamedWriteable(QueryBuilder.class); + assert in.read() == -1; + } else { + throw new IllegalStateException("Unable to read query builder. Unsupported transportVersion: " + transportVersion); + } + + return queryBuilder; + } + } + static SearchExecutionContext wrap(SearchExecutionContext delegate) { return new SearchExecutionContext(delegate) { diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java index 22d0ddfb53a11..aa98dd2da6772 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java @@ -433,15 +433,7 @@ static QueryBuilder parseQueryBuilder(DocumentParserContext context) { // make sure that we don't expand dots in field names while parsing, otherwise queries will // fail parsing due to unsupported inner objects context.path().setWithinLeafObject(true); - return parseTopLevelQuery(parser, queryName -> { - if (queryName.equals("has_child")) { - throw new IllegalArgumentException("the [has_child] query is unsupported inside a percolator query"); - } else if (queryName.equals("has_parent")) { - throw new IllegalArgumentException("the [has_parent] query is unsupported inside a percolator query"); - } else if (queryName.equals(KnnVectorQueryBuilder.NAME)) { - throw new IllegalArgumentException("the [knn] query is unsupported inside a percolator query"); - } - }); + return getQueryBuilder(parser); } catch (IOException e) { throw new ParsingException(parser.getTokenLocation(), "Failed to parse", e); } finally { @@ -449,6 +441,18 @@ static QueryBuilder parseQueryBuilder(DocumentParserContext context) { } } + static QueryBuilder getQueryBuilder(XContentParser parser) throws IOException { + return parseTopLevelQuery(parser, queryName -> { + if (queryName.equals("has_child")) { + throw new IllegalArgumentException("the [has_child] query is unsupported inside a percolator query"); + } else if (queryName.equals("has_parent")) { + throw new IllegalArgumentException("the [has_parent] query is unsupported inside a percolator query"); + } else if (queryName.equals(KnnVectorQueryBuilder.NAME)) { + throw new IllegalArgumentException("the [knn] query is unsupported inside a percolator query"); + } + }); + } + static void createQueryBuilderField( IndexVersion indexVersion, TransportVersion clusterTransportVersion, From 9907d6d607d3502b7adcdb086d1cd3092b65118f Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Fri, 31 Oct 2025 08:57:55 -0700 Subject: [PATCH 2/5] Remove unnecessary refactoring --- .../percolator/PercolatorFieldMapper.java | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java index aa98dd2da6772..22d0ddfb53a11 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java @@ -433,7 +433,15 @@ static QueryBuilder parseQueryBuilder(DocumentParserContext context) { // make sure that we don't expand dots in field names while parsing, otherwise queries will // fail parsing due to unsupported inner objects context.path().setWithinLeafObject(true); - return getQueryBuilder(parser); + return parseTopLevelQuery(parser, queryName -> { + if (queryName.equals("has_child")) { + throw new IllegalArgumentException("the [has_child] query is unsupported inside a percolator query"); + } else if (queryName.equals("has_parent")) { + throw new IllegalArgumentException("the [has_parent] query is unsupported inside a percolator query"); + } else if (queryName.equals(KnnVectorQueryBuilder.NAME)) { + throw new IllegalArgumentException("the [knn] query is unsupported inside a percolator query"); + } + }); } catch (IOException e) { throw new ParsingException(parser.getTokenLocation(), "Failed to parse", e); } finally { @@ -441,18 +449,6 @@ static QueryBuilder parseQueryBuilder(DocumentParserContext context) { } } - static QueryBuilder getQueryBuilder(XContentParser parser) throws IOException { - return parseTopLevelQuery(parser, queryName -> { - if (queryName.equals("has_child")) { - throw new IllegalArgumentException("the [has_child] query is unsupported inside a percolator query"); - } else if (queryName.equals("has_parent")) { - throw new IllegalArgumentException("the [has_parent] query is unsupported inside a percolator query"); - } else if (queryName.equals(KnnVectorQueryBuilder.NAME)) { - throw new IllegalArgumentException("the [knn] query is unsupported inside a percolator query"); - } - }); - } - static void createQueryBuilderField( IndexVersion indexVersion, TransportVersion clusterTransportVersion, From 263ba036d6ea571f042e74f945f2da330e51ef64 Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Fri, 31 Oct 2025 11:29:55 -0700 Subject: [PATCH 3/5] Remove exception-based flow control --- .../percolator/PercolateQueryBuilder.java | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java index 10dc046c18f9a..e49d83889a0f6 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java @@ -37,6 +37,7 @@ import org.elasticsearch.TransportVersion; import org.elasticsearch.TransportVersions; import org.elasticsearch.action.get.GetRequest; +import org.elasticsearch.common.CheckedSupplier; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.InputStreamStreamInput; import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput; @@ -564,11 +565,7 @@ static PercolateQuery.QueryStore createStore(MappedFieldType queryBuilderFieldTy return docId -> { if (binaryDocValues.advanceExact(docId)) { BytesRef qbSource = binaryDocValues.binaryValue(); - QueryBuilder queryBuilder; - - try { - queryBuilder = readQueryBuilder(qbSource, registry, indexVersion); - } catch (IllegalStateException e) { + QueryBuilder queryBuilder = readQueryBuilder(qbSource, registry, indexVersion, () -> { // query builder is written in an incompatible format, fall-back to reading it from source if (context.isSourceEnabled() == false) { throw new ElasticsearchException( @@ -588,10 +585,9 @@ static PercolateQuery.QueryStore createStore(MappedFieldType queryBuilderFieldTy source.internalSourceRef(), source.sourceContentType() ); - ParsedDocument document = context.parseDocument(sourceToParse); - BytesRef binaryValue = document.rootDoc().getBinaryValue(queryBuilderFieldType.name()); - queryBuilder = readQueryBuilder(binaryValue, registry, indexVersion); - } + + return context.parseDocument(sourceToParse).rootDoc().getBinaryValue(queryBuilderFieldType.name()); + }); queryBuilder = Rewriteable.rewrite(queryBuilder, context); return queryBuilder.toQuery(context); @@ -602,8 +598,12 @@ static PercolateQuery.QueryStore createStore(MappedFieldType queryBuilderFieldTy }; } - private static QueryBuilder readQueryBuilder(BytesRef bytesRef, NamedWriteableRegistry registry, IndexVersion indexVersion) - throws IOException { + private static QueryBuilder readQueryBuilder( + BytesRef bytesRef, + NamedWriteableRegistry registry, + IndexVersion indexVersion, + CheckedSupplier fallbackSource + ) throws IOException { try ( InputStream in = new ByteArrayInputStream(bytesRef.bytes, bytesRef.offset, bytesRef.length); StreamInput input = new NamedWriteableAwareStreamInput(new InputStreamStreamInput(in, bytesRef.length), registry) @@ -631,6 +631,9 @@ private static QueryBuilder readQueryBuilder(BytesRef bytesRef, NamedWriteableRe input.setTransportVersion(transportVersion); queryBuilder = input.readNamedWriteable(QueryBuilder.class); assert in.read() == -1; + } else if (fallbackSource != null) { + // incompatible transport version, try the fallback + queryBuilder = readQueryBuilder(fallbackSource.get(), registry, indexVersion, null); } else { throw new IllegalStateException("Unable to read query builder. Unsupported transportVersion: " + transportVersion); } From 077e77eb451b325fd06b64e79f8a297a32a4e1da Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Mon, 17 Nov 2025 10:19:19 -0800 Subject: [PATCH 4/5] Update QueryBuilderBWCIT to verify backcompat --- .../percolator/PercolateQueryBuilder.java | 6 +- .../upgrades/QueryBuilderBWCIT.java | 73 ++++--------------- 2 files changed, 18 insertions(+), 61 deletions(-) diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java index e49d83889a0f6..c78f51256e539 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java @@ -626,16 +626,14 @@ private static QueryBuilder readQueryBuilder( QueryBuilder queryBuilder; - if (TransportVersion.isCompatible(transportVersion)) { + if (TransportVersion.isCompatible(transportVersion) || fallbackSource == null) { // set the transportversion here - only read vints so far, so can change the version freely at this point input.setTransportVersion(transportVersion); queryBuilder = input.readNamedWriteable(QueryBuilder.class); assert in.read() == -1; - } else if (fallbackSource != null) { + } else { // incompatible transport version, try the fallback queryBuilder = readQueryBuilder(fallbackSource.get(), registry, indexVersion, null); - } else { - throw new IllegalStateException("Unable to read query builder. Unsupported transportVersion: " + transportVersion); } return queryBuilder; diff --git a/qa/full-cluster-restart/src/javaRestTest/java/org/elasticsearch/upgrades/QueryBuilderBWCIT.java b/qa/full-cluster-restart/src/javaRestTest/java/org/elasticsearch/upgrades/QueryBuilderBWCIT.java index 701bf83e5e6eb..27585cdaf275c 100644 --- a/qa/full-cluster-restart/src/javaRestTest/java/org/elasticsearch/upgrades/QueryBuilderBWCIT.java +++ b/qa/full-cluster-restart/src/javaRestTest/java/org/elasticsearch/upgrades/QueryBuilderBWCIT.java @@ -11,18 +11,10 @@ import com.carrotsearch.randomizedtesting.annotations.Name; -import org.elasticsearch.TransportVersion; -import org.elasticsearch.Version; import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; import org.elasticsearch.common.Strings; -import org.elasticsearch.common.io.stream.InputStreamStreamInput; -import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput; -import org.elasticsearch.common.io.stream.NamedWriteableRegistry; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.Fuzziness; -import org.elasticsearch.core.UpdateForV10; import org.elasticsearch.index.query.BoolQueryBuilder; import org.elasticsearch.index.query.ConstantScoreQueryBuilder; import org.elasticsearch.index.query.DisMaxQueryBuilder; @@ -37,33 +29,23 @@ import org.elasticsearch.index.query.SpanTermQueryBuilder; import org.elasticsearch.index.query.functionscore.FunctionScoreQueryBuilder; import org.elasticsearch.index.query.functionscore.RandomScoreFunctionBuilder; -import org.elasticsearch.search.SearchModule; import org.elasticsearch.test.cluster.ElasticsearchCluster; import org.elasticsearch.test.cluster.local.LocalClusterConfigProvider; import org.elasticsearch.test.cluster.local.distribution.DistributionType; import org.elasticsearch.xcontent.XContentBuilder; import org.junit.ClassRule; -import java.io.ByteArrayInputStream; -import java.io.InputStream; import java.util.ArrayList; -import java.util.Base64; -import java.util.Collections; import java.util.List; -import java.util.Map; -import static org.elasticsearch.cluster.ClusterState.VERSION_INTRODUCING_TRANSPORT_VERSIONS; import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder; /** * An integration test that tests whether percolator queries stored in older supported ES version can still be read by the * current ES version. Percolator queries are stored in the binary format in a dedicated doc values field (see - * PercolatorFieldMapper#createQueryBuilderField(...) method). Using the query builders writable contract. This test - * does best effort verifying that we don't break bwc for query builders between the first previous major version and - * the latest current major release. - * - * The queries to test are specified in json format, which turns out to work because we tend break here rarely. If the - * json format of a query being tested here then feel free to change this. + * PercolatorFieldMapper#createQueryBuilderField(...) method). We don't attempt to assert anything on results here, simply executing + * a percolator query will force deserialization of the old query builder. This also ensures verifies that our fallback compatibility + * functionality is working correction, otherwise the search request will throw an exception. */ public class QueryBuilderBWCIT extends ParameterizedFullClusterRestartTestCase { private static final List CANDIDATES = new ArrayList<>(); @@ -227,43 +209,20 @@ public void testQueryBuilderBWC() throws Exception { assertEquals(201, rsp.getStatusLine().getStatusCode()); } } else { - NamedWriteableRegistry registry = new NamedWriteableRegistry( - new SearchModule(Settings.EMPTY, Collections.emptyList()).getNamedWriteables() - ); - - for (int i = 0; i < CANDIDATES.size(); i++) { - QueryBuilder expectedQueryBuilder = (QueryBuilder) CANDIDATES.get(i)[1]; - Request request = new Request("GET", "/" + index + "/_search"); - request.setJsonEntity(Strings.format(""" - {"query": {"ids": {"values": ["%s"]}}, "docvalue_fields": [{"field":"query.query_builder_field"}]} - """, i)); - Response rsp = client().performRequest(request); - assertEquals(200, rsp.getStatusLine().getStatusCode()); - var hitRsp = (Map) ((List) ((Map) responseAsMap(rsp).get("hits")).get("hits")).get(0); - String queryBuilderStr = (String) ((List) ((Map) hitRsp.get("fields")).get("query.query_builder_field")).get(0); - byte[] qbSource = Base64.getDecoder().decode(queryBuilderStr); - try ( - InputStream in = new ByteArrayInputStream(qbSource, 0, qbSource.length); - StreamInput input = new NamedWriteableAwareStreamInput(new InputStreamStreamInput(in), registry) - ) { - @UpdateForV10(owner = UpdateForV10.Owner.SEARCH_FOUNDATIONS) // won't need to read <8.8 data anymore - boolean originalClusterHasTransportVersion = parseLegacyVersion(getOldClusterVersion()).map( - v -> v.onOrAfter(VERSION_INTRODUCING_TRANSPORT_VERSIONS) - ).orElse(true); - TransportVersion transportVersion; - if (originalClusterHasTransportVersion == false) { - transportVersion = TransportVersion.fromId( - parseLegacyVersion(getOldClusterVersion()).map(Version::id).orElse(TransportVersion.minimumCompatible().id()) - ); - } else { - transportVersion = TransportVersion.readVersion(input); + Request request = new Request("GET", "/" + index + "/_search"); + request.setJsonEntity(""" + { + "query": { + "percolate": { + "field": "query", + "document": { + "foo": "bar" + } } - input.setTransportVersion(transportVersion); - QueryBuilder queryBuilder = input.readNamedWriteable(QueryBuilder.class); - assert in.read() == -1; - assertEquals(expectedQueryBuilder, queryBuilder); - } - } + } + }"""); + Response rsp = client().performRequest(request); + assertEquals(200, rsp.getStatusLine().getStatusCode()); } } } From 1e5f29fca79dd77cb701a5e3684fcf0ab18ce4f1 Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Mon, 17 Nov 2025 10:24:06 -0800 Subject: [PATCH 5/5] Fix javadoc typos --- .../java/org/elasticsearch/upgrades/QueryBuilderBWCIT.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qa/full-cluster-restart/src/javaRestTest/java/org/elasticsearch/upgrades/QueryBuilderBWCIT.java b/qa/full-cluster-restart/src/javaRestTest/java/org/elasticsearch/upgrades/QueryBuilderBWCIT.java index 27585cdaf275c..670b90dec6afb 100644 --- a/qa/full-cluster-restart/src/javaRestTest/java/org/elasticsearch/upgrades/QueryBuilderBWCIT.java +++ b/qa/full-cluster-restart/src/javaRestTest/java/org/elasticsearch/upgrades/QueryBuilderBWCIT.java @@ -44,8 +44,8 @@ * An integration test that tests whether percolator queries stored in older supported ES version can still be read by the * current ES version. Percolator queries are stored in the binary format in a dedicated doc values field (see * PercolatorFieldMapper#createQueryBuilderField(...) method). We don't attempt to assert anything on results here, simply executing - * a percolator query will force deserialization of the old query builder. This also ensures verifies that our fallback compatibility - * functionality is working correction, otherwise the search request will throw an exception. + * a percolator query will force deserialization of the old query builder. This also verifies that our fallback compatibility + * functionality is working correctly, otherwise the search request will throw an exception. */ public class QueryBuilderBWCIT extends ParameterizedFullClusterRestartTestCase { private static final List CANDIDATES = new ArrayList<>();