Skip to content

Conversation

@mridula-s109
Copy link
Contributor

@mridula-s109 mridula-s109 commented Sep 15, 2025

🚨 Please check with search inference team on when to merge this - we're working out some rate limiting issues ahead of GA of ELSER on EIS

Summary

Implements dynamic default selection for semantic_text fields to automatically use ELSER on EIS
(.elser-2-elastic) when available, with graceful fallback to ML nodes (.elser-2-elasticsearch).

Problem

Currently, semantic_text fields are hardcoded to default to .elser-2-elasticsearch (ML nodes). This misses
opportunities for better performance and cost efficiency when EIS is available.

Solution

  • Dynamic Detection: Uses ModelRegistry.containsDefaultConfigId() to detect EIS availability
  • Smart Fallback: Automatically selects .elser-2-elastic when available, falls back to
    .elser-2-elasticsearch
  • Zero Configuration: Works transparently without requiring user changes
  • Full Compatibility: On-prem users continue using ML nodes until they enable cloud-connected mode

Changes

  • Added getPreferredElserInferenceId() method for dynamic selection logic
  • Updated semantic_text field mapper to use dynamic default instead of hardcoded value
  • Added comprehensive tests for both EIS available/unavailable scenarios

Testing

  • ✅ All existing tests pass (backward compatibility verified)
  • ✅ New test covers dynamic selection logic
  • ✅ Manual testing confirms proper fallback behavior

Impact

  • Cloud users: Automatically get better performance with EIS
  • On-prem users: No changes, continue using ML nodes seamlessly
  • Existing fields: Completely unaffected, no migration needed

@mridula-s109
Copy link
Contributor Author

Hey @ioanatia! 👋

Just implemented the dynamic ELSER default selection for semantic_text fields.

The approach is intentionally minimal -leverages existing ModelRegistry.containsDefaultConfigId() for
EIS detection rather than building new infrastructure. New semantic_text fields automatically default
to .elser-2-elastic when available, gracefully fall back to .elser-2-elasticsearch when not.

A few things I'd especially appreciate feedback on:

  • Does the overall approach make sense?
  • Is the error handling sufficient (graceful fallback when ModelRegistry fails)?
  • Any concerns about the scope or missing edge cases?

Still draft mode, but wanted to get your input before finalizing. Thanks!

@ioanatia
Copy link
Contributor

relying on modelRegistry.containsDefaultConfigId(EIS_ELSER_INFERENCE_ID) seems to be right.

but I think we need more tests.
Maybe take a look at some of the tests from https://github.com/elastic/elasticsearch/tree/main/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/elastic and see if we can mock the presence of EIS in tests.
Then we can properly test that we pick the right inference ID - by first creating an index with a semantic_text field (and unspecified inference ID) and then requesting the mapping of this new index. We should see the default EIS endpoint.

@ioanatia
Copy link
Contributor

In terms of tests, what we can also do is to add another mock service in https://github.com/elastic/elasticsearch/tree/main/x-pack/plugin/inference/qa/test-service-plugin/src/main/java/org/elasticsearch/xpack/inference/mock

and then use it in yaml tests.

the InferenceService allows to register default endpoints:

default List<DefaultConfigId> defaultConfigIds() {

I think we can mock the default endpoints we have in EIS and use the same name in the mock inference service.
Let me know if you need any help!

@mridula-s109
Copy link
Contributor Author

FYI folks we're working on some rate limiting issues with ELSER and we may want to defer this.

Please check with @maxjakob or myself before merging.

Thanks @Mikep86! All comments addressed. Have verified about the default assertion in the SemanticTextEISDefaultIT, there is no flakiness or underlying issue at the moment. Since we're holding off on merging due to the ELSER rate limiting work, no rush on further review. Take your time! 😊

@mridula-s109
Copy link
Contributor Author

@ioanatia @Mikep86 quick update: I’ve addressed the review feedback and pushed the latest changes.
Just a heads-up we’ve scheduled a short sync with @seanhandley and @maxjakob on Wednesday, October 22 (1:00–1:15 PM) to proceed with merging this PR.
Please let me know if there’s anything else you’d like me to adjust before then. 🙌

Copy link
Contributor

@Mikep86 Mikep86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM to merge once we get the 👍 from @seanhandley and @maxjakob

@liranabn
Copy link

@mridula-s109 , do we have a separate PR to update the docs?
specifically here

If you don’t specify an inference endpoint, the inference_id field defaults to .elser-2-elasticsearch, a preconfigured endpoint for the elasticsearch service.

@seanhandley
Copy link

Great work @mridula-s109 !

We've decided to wait anther week while we investigate a bug report on our side. Once it's mitigated we can hopefully merge this. I've pushed our calendar reminder back to next week to accommodate this.

@mridula-s109
Copy link
Contributor Author

Great work @mridula-s109 !

We've decided to wait anther week while we investigate a bug report on our side. Once it's mitigated we can hopefully merge this. I've pushed our calendar reminder back to next week to accommodate this.

Thanks for the update @seanhandley! 👍

No problem at all, waiting another week makes sense to ensure everything is stable. I'll keep an eye out for next week's calendar reminder.

In the meantime, @liranabn raised a good point about doc updates. I'll use this week to add the documentation changes directly to this PR (updating the section that mentions .elser-2-elasticsearch as the default). Will push those updates in the next day or two.

@mridula-s109
Copy link
Contributor Author

@mridula-s109 , do we have a separate PR to update the docs? specifically here

If you don’t specify an inference endpoint, the inference_id field defaults to .elser-2-elasticsearch, a preconfigured endpoint for the elasticsearch service.

Are there any other doc pages or files that reference the .elser-2-elasticsearch default that I should also update?@Mikep86 @ioanatia Want to make sure I catch all the places that need changes. Thanks!

@leemthompo
Copy link
Contributor

leemthompo commented Oct 21, 2025

@mridula-s109

  1. Istvan already has a PR lined up for the elasticsearch repo, which you can check out: Adds ELSER on EIS examples to semantic_text #135359

  2. there are a couple of pages in the docs-content repo:

  1. I also see one instance in the spec repo: https://github.com/elastic/elasticsearch-specification/blob/main/specification/_types/mapping/core.ts#L246

@mridula-s109
Copy link
Contributor Author

@mridula-s109

  1. Istvan already has a PR lined up for the elasticsearch repo, which you can check out: Adds ELSER on EIS examples to semantic_text #135359
  2. there are a couple of pages in the docs-content repo:
  1. I also see one instance in the spec repo: https://github.com/elastic/elasticsearch-specification/blob/main/specification/_types/mapping/core.ts#L246

Thanks @leemthompo for the inputs, i will look into it!

@seanhandley
Copy link

@mridula-s109 We're clear to merge this today 👍 There are other pieces of work we want to do but none of them are blocking this from being merged.

https://github.com/elastic/search-team/issues/11650

cc @sphilipse @liranabn @maxjakob

@mridula-s109
Copy link
Contributor Author

@mridula-s109 We're clear to merge this today 👍 There are other pieces of work we want to do but none of them are blocking this from being merged.

elastic/search-team#11650

cc @sphilipse @liranabn @maxjakob

Thanks for letting me know @seanhandley, As the CI has passed as well post updating from main today, i will go ahead with the merge now.

@mridula-s109 mridula-s109 merged commit 6b2c55a into elastic:main Oct 29, 2025
34 checks passed
@mridula-s109
Copy link
Contributor Author

Opening up a followup PR to modify the SemanticFieldMappertests to point to the new constant in SemanticTextFieldMapper.java.

chrisparrinello pushed a commit to chrisparrinello/elasticsearch that referenced this pull request Nov 3, 2025
…#134708)

* Defaulting EIS on ELSER

* Extended testing

* [CI] Auto commit changes from spotless

* Edited to include the default

* Cleaned up the mapper implementation

* COmpile issue

* [CI] Auto commit changes from spotless

* Added tests

* [CI] Auto commit changes from spotless

* Refactored the variable names

* Cleanup done

* Removed unnecessary files

* Unit tests and mock is working

* [CI] Auto commit changes from spotless

* Fix test

* yaml addition failure

* [CI] Auto commit changes from spotless

* Resolved the import issue or duplication of variables in mock

* Resolved PR comments

* Restored error

* Update docs/changelog/134708.yaml

* Integration test

* [CI] Auto commit changes from spotless

* Resolved all PR comments

* [CI] Auto commit changes from spotless

* Cleaned up the redudant reference of TestInferencePlugin

* Included both before and before test

* [CI] Auto commit changes from spotless

* Made changes to accomodate the old constant version string

---------

Co-authored-by: elasticsearchmachine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>enhancement :SearchOrg/Relevance Label for the Search (solution/org) Relevance team Team:Search - Relevance The Search organization Search Relevance team v9.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants