Restore model registry validation for the semantic text field #127285
Conversation
jimczi
added
>enhancement
:Search Relevance/Search
|
Pinging @elastic/es-search-relevance (Team:Search Relevance) |
elasticsearchmachine
added
the
Team:Search Relevance
|
Hi @jimczi, I've created a changelog YAML for you. |
There was a problem hiding this comment.
Thanks for putting this PR together. I'm not sure I quite understand based on the commit history what was causing this bug - it looks like the changes after the revert were pretty minimal. Would it be possible to walk me through how this fixed the original test failures so I can better understand if we want to proactively add more tests? Thanks!
...resources/rest-api-spec/test/inference/50_semantic_text_query_inference_endpoint_changes.yml
Outdated
Show resolved
Hide resolved
The issue comes up when an inference endpoint gets created after the semantic field that points to it. In that case, the model only gets resolved when the mapping is updated. What’s happening in this bug is that the first ingestion triggers that update. We get the model definition from the bulk request and try to do a dynamic mapping update. Dynamic updates run on the master node. There, it parses the current mapping (which doesn’t have the model settings yet) and merges it with the update. But since the model was already resolved via the registry during the initial parse, the system sees the update as a no-op which isn’t allowed during bulk requests. The fix is to skip setting the model settings in the mapping if the model was only resolved through the registry. We still use the resolved model to build sub-fields and check the settings, but we don’t actually include it in the field mapping just yet. Then, when the first ingestion happens, the model settings get properly added through that dynamic update. |
...resources/rest-api-spec/test/inference/50_semantic_text_query_inference_endpoint_changes.yml
Outdated
Show resolved
Hide resolved
...e/src/yamlRestTest/resources/rest-api-spec/test/inference/30_semantic_text_inference_bwc.yml
Outdated
Show resolved
Hide resolved
|
@elasticmachine run elasticsearch-ci/part-3 |
|
@elasticmachine run elasticsearch-ci |
This PR is a partial backport of elastic#127285 that fixes the validation of the inference id when mappings are restored or dynamically updated. This change doesn't include defaulting semantic text dense vector to BBQ since it requires elastic#124581 to be backported first.
This PR is a partial backport of elastic#127285 that fixes the validation of the inference id when mappings are restored or dynamically updated. This change doesn't include defaulting semantic text dense vector to BBQ since it requires elastic#124581 to be backported first.
This PR restores the changes reverted in #127075 and fixed the bug that caused the test failures.
The resolved model is now only used for validation during the creation of the semantic text field
and the final value is set on the first ingestion to ensure consistency.
This change also sets the default for semantic_text field using dense vectors to BBQ.