Repository navigation
Simplify field matching in exclude source vectors - #156466
elasticsearchmachine merged 4 commits into
Conversation
The fetch phase decides which vector fields to strip from _source by compiling the field patterns of the request into an automaton. It did so unconditionally, before establishing whether the mapping held any vector embeddings at all, so an index with none paid for a matcher that had nothing to match. Worse, a request asking for enough field patterns exceeded Lucene's determinization work limit, and the resulting TooComplexToDeterminizeException failed the search even on indices where not a single field would have been excluded. Resolve the vector fields first and return early when the mapping has none, which skips the whole step for such indices. Then match the remaining patterns one at a time with Regex#simpleMatch rather than compiling them. Compiling amortizes its cost over the strings tested against the result, and the only strings tested here are the vector fields, of which a mapping holds a handful, while a request can carry arbitrarily many patterns. The build therefore dominated and was never repaid. Matching individually suits this shape better and, because nothing is compiled, no work limit applies. The inference field patterns come from the mapping rather than from the request, so their number is bounded. They keep a compiled matcher, now built through Regex#simpleMatcher so that the automaton is skipped when it is not needed.
|
Pinging @elastic/es-search-relevance (Team:Search Relevance) |
|
Hi @mayya-sharipova, I've created a changelog YAML for you. |
🔍 Preview links for changed docs⏳ Building and deploying preview... View progress This comment will be updated with preview links when the build is complete. |
ℹ️ Important: Docs version tagging👋 Thanks for updating the docs! Just a friendly reminder that our docs are now cumulative. This means all 9.x versions are documented on the same page and published off of the main branch, instead of creating separate pages for each minor version. We use applies_to tags to mark version-specific features and changes. Expand for a quick overviewWhen to use applies_to tags:✅ At the page level to indicate which products/deployments the content applies to (mandatory) What NOT to do:❌ Don't remove or replace information that applies to an older version 🤔 Need help?
|
jimczi
left a comment
There was a problem hiding this comment.
Nice find. The approach looks right to me: not compiling the request patterns is what the rest of the fetch path already does. FieldTypeLookup#getMatchingFieldNames returns the keySet for a match-all, a hash lookup for a concrete name, and only falls back to per pattern Regex#simpleMatch for wildcards.
Comments inline. One thing outside the diff: labels are v9.5.1 and v9.6.0, but this shipped in 9.2.0 and the open patch versions are v9.4.6, v9.5.2 and v9.6.0. Looks like v9.5.1 is already cut and v9.4.6 is missing.
Precompute the vector embedding fields on MappingLookup rather than streaming every field type on each fetch. The mapping is immutable, so the candidate set is established once, in the constructor loop that already builds inferenceFields and syntheticVectorFields, and the early return becomes an isEmpty check with no per request allocation. That is worth it because the requests reaching this path are the wide ones. The existing syntheticVectorFields cannot be reused for this. It is keyed on syntheticVectorsLoader, which is null unless the mapper carries excludeSourceVectors, so the set is empty when index.mapping.exclude_source_vectors is false. This method is still reached in that case through a request level exclude_vectors, so the candidates have to be resolved independently of the setting. In the test fixture, ask for a plain match-all alongside the concrete names instead of a crafted trailing wildcard. That is what a client sends, it reaches the determinization limit with fewer patterns, and it matches the vector field, so the late exclude branch is covered rather than only the up front one. Assert the precondition as well, since the fixture covers nothing if that limit ever moves. Concrete names alone never exceed the limit at any count: a string union is already minimal and determinize short circuits on it. It is the match-all that forces the subset construction, so document that as the mechanism instead. Also use Strings#format, which forbiddenApisTest requires over String#format, and reword the changelog to name the symptom.
|
@jimczi Thanks Jim for the review, all addressed. Please continue the review. |
jimczi
left a comment
There was a problem hiding this comment.
Thanks Mayya, all addressed. Precomputing the set on MappingLookup came out cleaner than what I suggested, and the javadoc on why it is not syntheticVectorFields is worth having.
Ran the unit tests and the search.vectors / get/100_synthetic_source yaml suites locally, all green. The 9.4.5 bwc failure is the MixedClusterEsqlSpecLookupJoinIT wave that is failing on main, unrelated.
LGTM.
…e-source-vectors-matching
💔 Backport failed
You can use sqren/backport to manually backport by running |
The fetch phase decides which vector fields to strip from _source by compiling the field patterns of the request into an automaton. It did so unconditionally, before establishing whether the mapping held any vector embeddings at all, so an index with none paid for a matcher that had nothing to match. Worse, a request asking for enough field patterns exceeded Lucene's determinization work limit, and the resulting TooComplexToDeterminizeException failed the search even on indices where not a single field would have been excluded. Resolve the vector fields first and return early when the mapping has none, which skips the whole step for such indices. Then match the remaining patterns one at a time with Regex#simpleMatch rather than compiling them. Compiling amortizes its cost over the strings tested against the result, and the only strings tested here are the vector fields, of which a mapping holds a handful, while a request can carry arbitrarily many patterns. The build therefore dominated and was never repaid. Matching individually suits this shape better and, because nothing is compiled, no work limit applies. The inference field patterns come from the mapping rather than from the request, so their number is bounded. They keep a compiled matcher, now built through Regex#simpleMatcher so that the automaton is skipped when it is not needed.
…56613) * Simplify field matching in exclude source vectors (#156466) The fetch phase decides which vector fields to strip from _source by compiling the field patterns of the request into an automaton. It did so unconditionally, before establishing whether the mapping held any vector embeddings at all, so an index with none paid for a matcher that had nothing to match. Worse, a request asking for enough field patterns exceeded Lucene's determinization work limit, and the resulting TooComplexToDeterminizeException failed the search even on indices where not a single field would have been excluded. Resolve the vector fields first and return early when the mapping has none, which skips the whole step for such indices. Then match the remaining patterns one at a time with Regex#simpleMatch rather than compiling them. Compiling amortizes its cost over the strings tested against the result, and the only strings tested here are the vector fields, of which a mapping holds a handful, while a request can carry arbitrarily many patterns. The build therefore dominated and was never repaid. Matching individually suits this shape better and, because nothing is compiled, no work limit applies. The inference field patterns come from the mapping rather than from the request, so their number is bounded. They keep a compiled matcher, now built through Regex#simpleMatcher so that the automaton is skipped when it is not needed. (cherry picked from commit 8528446) * Retrigger CI after gradle.org 503
The fetch phase decides which vector fields to strip from _source by compiling the field patterns of the request into an automaton. It did so unconditionally, before establishing whether the mapping held any vector embeddings at all, so an index with none paid for a matcher that had nothing to match. Worse, a request asking for enough field patterns exceeded Lucene's determinization work limit, and the resulting TooComplexToDeterminizeException failed the search even on indices where not a single field would have been excluded. Resolve the vector fields first and return early when the mapping has none, which skips the whole step for such indices. Then match the remaining patterns one at a time with Regex#simpleMatch rather than compiling them. Compiling amortizes its cost over the strings tested against the result, and the only strings tested here are the vector fields, of which a mapping holds a handful, while a request can carry arbitrarily many patterns. The build therefore dominated and was never repaid. Matching individually suits this shape better and, because nothing is compiled, no work limit applies. The inference field patterns come from the mapping rather than from the request, so their number is bounded. They keep a compiled matcher, now built through Regex#simpleMatcher so that the automaton is skipped when it is not needed.
The fetch phase decides which vector fields to strip from _source by compiling the field patterns of the request into an automaton. It did so unconditionally, before establishing whether the mapping held any vector embeddings at all, so an index with none paid for a matcher that had nothing to match. Worse, a request asking for enough field patterns exceeded Lucene's determinization work limit, and the resulting TooComplexToDeterminizeException failed the search even on indices where not a single field would have been excluded. Resolve the vector fields first and return early when the mapping has none, which skips the whole step for such indices. Then match the remaining patterns one at a time with Regex#simpleMatch rather than compiling them. Compiling amortizes its cost over the strings tested against the result, and the only strings tested here are the vector fields, of which a mapping holds a handful, while a request can carry arbitrarily many patterns. The build therefore dominated and was never repaid. Matching individually suits this shape better and, because nothing is compiled, no work limit applies. The inference field patterns come from the mapping rather than from the request, so their number is bounded. They keep a compiled matcher, now built through Regex#simpleMatcher so that the automaton is skipped when it is not needed.
The fetch phase decides which vector fields to strip from _source by
compiling the field patterns of the request into an automaton. It did so
unconditionally, before establishing whether the mapping held any vector
embeddings at all, so an index with none paid for a matcher that had
nothing to match. Worse, a request asking for enough field patterns
exceeded Lucene's determinization work limit, and the resulting
TooComplexToDeterminizeException failed the search even on indices where
not a single field would have been excluded.
Resolve the vector fields first and return early when the mapping has
none, which skips the whole step for such indices.
Then match the remaining patterns one at a time with Regex#simpleMatch
rather than compiling them. Compiling amortizes its cost over the
strings tested against the result, and the only strings tested here are
the vector fields, of which a mapping holds a handful, while a request
can carry arbitrarily many patterns. The build therefore dominated and
was never repaid. Matching individually suits this shape better and,
because nothing is compiled, no work limit applies.
The inference field patterns come from the mapping rather than from the
request, so their number is bounded. They keep a compiled matcher, now
built through Regex#simpleMatcher so that the automaton is skipped when
it is not needed.