Default new semantic_text fields to use BBQ when models are compatible #126629

kderusso · 2025-04-10T17:31:53Z

While dense_vector fields default to int8_hnsw for now, we have demonstrated significantly better performance in text embedding models with bbq_hnsw. As this is the primary use case for semantic_text, we are making this the default quantization method for semantic_text fields using text embedding models. Models that are incompatible with BBQ due to dimention count or element type will continue to use the dense_vector quantization defaults.

elasticsearchmachine · 2025-04-10T17:32:16Z

Pinging @elastic/search-eng (Team:SearchOrg)

elasticsearchmachine · 2025-04-10T17:32:16Z

Pinging @elastic/search-relevance (Team:Search - Relevance)

elasticsearchmachine · 2025-04-10T17:32:17Z

Hi @kderusso, I've created a changelog YAML for you.

elasticsearchmachine · 2025-04-10T17:32:53Z

Pinging @elastic/es-search-relevance (Team:Search Relevance)

jimczi

I appreciate that we're keeping index options as an implementation detail of the semantic text field. That said, with this change, existing semantic text fields will start using BBQ immediately. I'm in favor of this behavior (rather than reserving this change for only new indices), but I wanted to explicitly call it out for awareness before we proceed with the merge since that's not exactly what we discussed.

Mikep86

I think updating existing semantic text fields to use BBQ could cause major problems. For example, take a look at the Int8 HNSW update logic. It only allows updates to Int4 HNSW. The end effect of this change is that every semantic text field using Int8 HNSW could start failing on document ingest.

jimczi · 2025-04-11T08:20:27Z

Yep, that was too good to be true, @Mikep86.
Let’s go ahead and apply the logic under a new index version for now.
@elastic/es-search-relevance moving from int8 to BBQ, is that something we could eventually apply to a live index as well?

kderusso · 2025-04-11T12:54:00Z

Good catch @Mikep86 - Added a gate on IndexVersion.

...nference/src/main/java/org/elasticsearch/xpack/inference/mapper/SemanticTextFieldMapper.java

...nce/src/test/java/org/elasticsearch/xpack/inference/mapper/SemanticTextFieldMapperTests.java

Mikep86

Nice index version solution in SemanticTextFieldMapperTests, I believe that covers our needs and we can revert changes to getRandomCompatibleIndexVersion.

Mikep86 · 2025-04-15T18:35:27Z

...nce/src/test/java/org/elasticsearch/xpack/inference/mapper/SemanticTextFieldMapperTests.java

Nice solution here that both constrains the index versions and maintains randomness 🙌 !

Mikep86 · 2025-04-15T18:43:19Z

...ava/org/elasticsearch/xpack/inference/mapper/SemanticInferenceMetadataFieldsMapperTests.java

@@ -114,18 +117,18 @@ public MappedFieldType getMappedFieldType() {
    }

    static IndexVersion getRandomCompatibleIndexVersion(boolean useLegacyFormat) {
+        return getRandomCompatibleIndexVersion(useLegacyFormat, IndexVersion.current());


Based on your changes in SemanticTextFieldMapperTests, I don't think we need any changes to getRandomCompatibleIndexVersion? These changes create some usability challenges:

maxVersion is only selectively used, which could be confusing to future devs.

What if maxVersion is less than IndexVersions.UPGRADE_TO_LUCENE_10_0_0 or IndexVersions.INFERENCE_METADATA_FIELDS? This will result in an error, and one that is thrown based on a randomization factor as well, which will be very confusing.

We need this because otherwise one of the cases in SemanticInferenceMetadataFieldMapperTests#isEnabled will fail as it depends on the old, less inclusive legacy format logic. You can see the modifications to the test for this here: 6cfe39a

I could have removed the test, but that didn't feel right. I think this is OK, if we need to extend it later for additional tests we can do so as needed.

What I'm confused about is why reverting changes to both getRandomCompatibleIndexVersion and testIsEnabled isn't an option. This should work fine as this is the state of the test before this PR, no?

And that approach should work with your changes in SemanticTextFieldMapperTests, so I see it as the best path for the moment.

We already have some followup work on this test, why don't we roll that into the scope?

Mikep86 · 2025-04-15T18:45:36Z

...nce/src/test/java/org/elasticsearch/xpack/inference/mapper/SemanticTextFieldMapperTests.java

+            IndexVersions.INFERENCE_METADATA_FIELDS,
+            IndexVersionUtils.getPreviousVersion(IndexVersions.SEMANTIC_TEXT_DEFAULTS_TO_BBQ)
+        );
+        assertSemanticTextField(mapperService, "field", true, null, SemanticTextFieldMapper.defaultSemanticDenseIndexOptions());


I believe this should be defaultDenseVectorIndexOptions?

Good catch. Updated the test, though it's failing right now. It's picking up the correct index options, but in the debugger it's not getting passed in to the mapper. Troubleshooting this now.

So in SemanticTextFieldMapperTests#createMapperService, we set the index version created as a setting, and then create the mapping with those specified settings here.

In the MapperServiceTestCase, this calls getVersion(), which always returns IndexVersion.current().

I fixed this, so we ensure that we can pass in the actual version we want to create. This makes this test do the right thing and pass, but there’s a side effect in that a few different existing tests fail due to things like created on a 7.17 index, not having an indexed field, etc. To be clear I believe this is a test error and not an error with semantic_text code.

For this PR, just to get it in and over the line, I hacked in a fix that will not propagate the correct index version, so the tests work the same as they always have. We should make some time to address it. Commit here: 4a7fad9

…ith current index version

Mikep86

Looks good, thanks for the (many) iterations here!

elasticsearchmachine · 2025-04-17T12:26:21Z

💔 Backport failed

Status	Branch	Result
❌	8.x	Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 126629

elastic#126629) * Default new semantic_text fields to use BBQ when models are compatible * Update docs/changelog/126629.yaml * Gate default BBQ by IndexVersion * Cleanup from PR feedback * PR feedback * Fix test * Fix test * PR feedback * Update test to test correct options * Hack alert: Fix issue where mapper service was always being created with current index version (cherry picked from commit a72883e) # Conflicts: # server/src/main/java/org/elasticsearch/index/IndexVersions.java # server/src/main/java/org/elasticsearch/index/mapper/vectors/DenseVectorFieldMapper.java # x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/mapper/SemanticInferenceMetadataFieldsMapperTests.java # x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/mapper/SemanticTextFieldMapperTests.java

kderusso · 2025-04-17T14:34:31Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Questions ?

Please refer to the Backport tool documentation

kderusso · 2025-04-17T18:19:33Z

Note: #124581 was not backported, so I have not backported this to 8.x. This means that unless the previous PR is backported, this change will only be in 9.1+.

We can revisit this decision later on, but for now I've made the appropriate label changes on this PR.

…ompatible (elastic#126629)" This reverts commit a72883e.

Default new semantic_text fields to use BBQ when models are compatible

d4ddcd0

kderusso added >enhancement auto-backport Automatically create backport pull requests when merged :SearchOrg/Relevance Label for the Search (solution/org) Relevance team v8.19.0 v9.1.0 labels Apr 10, 2025

Update docs/changelog/126629.yaml

add2b72

kderusso added the :Search Relevance/Search Catch all for Search Relevance label Apr 10, 2025

kderusso requested review from jimczi and a team April 10, 2025 17:33

jimczi approved these changes Apr 10, 2025

View reviewed changes

Merge branch 'main' into kderusso/semantic-text-default-bbq

071a7bd

Mikep86 reviewed Apr 10, 2025

View reviewed changes

Gate default BBQ by IndexVersion

589f657

kderusso requested a review from Mikep86 April 11, 2025 12:54

Mikep86 reviewed Apr 11, 2025

View reviewed changes

kderusso added 4 commits April 14, 2025 16:45

Cleanup from PR feedback

10f3a86

PR feedback

6bc674c

Fix test

b318ffe

Fix test

6cfe39a

kderusso requested a review from Mikep86 April 15, 2025 15:09

Merge from main

be9b7b4

Mikep86 reviewed Apr 15, 2025

View reviewed changes

PR feedback

998b3b9

kderusso added 3 commits April 16, 2025 09:48

Update test to test correct options

1460038

Hack alert: Fix issue where mapper service was always being created w…

4a7fad9

…ith current index version

Merge branch 'main' into kderusso/semantic-text-default-bbq

06cf079

kderusso requested a review from Mikep86 April 16, 2025 20:43

Merge branch 'main' into kderusso/semantic-text-default-bbq

7742bfc

Mikep86 approved these changes Apr 16, 2025

View reviewed changes

kderusso merged commit a72883e into elastic:main Apr 17, 2025
17 checks passed

elasticsearchmachine added the backport pending label Apr 17, 2025

kderusso mentioned this pull request Apr 17, 2025

[8.x] Default new semantic_text fields to use BBQ when models are compatible (#126629) #127005

Closed

kderusso removed the v8.19.0 label Apr 17, 2025

kderusso added a commit to kderusso/elasticsearch that referenced this pull request Apr 18, 2025

Revert "Default new semantic_text fields to use BBQ when models are c…

b5b4e1f

…ompatible (elastic#126629)" This reverts commit a72883e.

kderusso mentioned this pull request Apr 18, 2025

Revert semantic_text model registry changes #127075

Merged

Mikep86 mentioned this pull request Apr 30, 2025

[8.19] Fix inference model validation for the semantic text field #127559

Merged

kderusso mentioned this pull request May 6, 2025

Add IndexVersion in preparation to backport default BBQ #127756

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Default new semantic_text fields to use BBQ when models are compatible #126629

Default new semantic_text fields to use BBQ when models are compatible #126629

kderusso commented Apr 10, 2025

elasticsearchmachine commented Apr 10, 2025

elasticsearchmachine commented Apr 10, 2025

elasticsearchmachine commented Apr 10, 2025

elasticsearchmachine commented Apr 10, 2025

jimczi left a comment •

edited

Loading

Mikep86 left a comment •

edited

Loading

jimczi commented Apr 11, 2025

kderusso commented Apr 11, 2025

Mikep86 left a comment

Mikep86 Apr 15, 2025

Mikep86 Apr 15, 2025

kderusso Apr 16, 2025

Mikep86 Apr 16, 2025

kderusso Apr 16, 2025

Mikep86 Apr 15, 2025

kderusso Apr 16, 2025

kderusso Apr 16, 2025

Mikep86 left a comment

elasticsearchmachine commented Apr 17, 2025

kderusso commented Apr 17, 2025

kderusso commented Apr 17, 2025

Default new semantic_text fields to use BBQ when models are compatible #126629

Default new semantic_text fields to use BBQ when models are compatible #126629

Conversation

kderusso commented Apr 10, 2025

elasticsearchmachine commented Apr 10, 2025

elasticsearchmachine commented Apr 10, 2025

elasticsearchmachine commented Apr 10, 2025

elasticsearchmachine commented Apr 10, 2025

jimczi left a comment • edited Loading

Choose a reason for hiding this comment

Mikep86 left a comment • edited Loading

Choose a reason for hiding this comment

jimczi commented Apr 11, 2025

kderusso commented Apr 11, 2025

Mikep86 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mikep86 left a comment

Choose a reason for hiding this comment

elasticsearchmachine commented Apr 17, 2025

💔 Backport failed

kderusso commented Apr 17, 2025

💚 All backports created successfully

Questions ?

kderusso commented Apr 17, 2025

jimczi left a comment •

edited

Loading

Mikep86 left a comment •

edited

Loading