Skip to content

Conversation

jimczi
Copy link
Contributor

@jimczi jimczi commented Jun 10, 2025

Ensure the new exclude_vectors parameter is correctly serialized and deserialized when operating in a mixed-version cluster. Given that version 8.19 is not compatible with 9.0, we can assume that all versions on or after 9.1 will support this new parameter.

Ensure the new exclude_vectors parameter is correctly serialized and deserialized when operating in a mixed-version cluster.
Given that version 8.19 is not compatible with 9.0, we can assume that all versions on or after 9.1 will support this new parameter.
@jimczi jimczi added >non-issue auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Search Relevance/Vectors Vector search v8.19.0 labels Jun 10, 2025
@elasticsearchmachine elasticsearchmachine added the Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch label Jun 10, 2025
@elasticsearchmachine
Copy link
Collaborator

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

@benwtrent
Copy link
Member

Given that version 8.19 is not compatible with 9.0,

I don't understand how this is broken.

@thecoop could you take a look here?

@benwtrent benwtrent requested a review from thecoop June 10, 2025 11:26
@jimczi
Copy link
Contributor Author

jimczi commented Jun 10, 2025

I don't understand how this is broken.

Using isPatchFrom is not enough since 9.1 can also send or receive data from 8.19. That's why I replaced isPatchFrom with onOrAfter. This is not technically correct, lots of versions, after the one I added, are not compatible with this change. However, the next version compatible with 8.19 is 9.1 so we can ignore them and keep the logic simple.

@benwtrent
Copy link
Member

Using isPatchFrom is not enough since 9.1 can also send or receive data from 8.19.

I don't understand why this isn't enough. 9.1 should negotiate the appropriate transport version, and 9.1 should know about the patch version.

@thecoop
Copy link
Member

thecoop commented Jun 10, 2025

Changing to onOrAfter is more semantically correct, as from the perspective of 8.19, the functionality represented by that version never goes away (there will never be a future transport version seen by 8.19 for which the isPatchFrom will turn the functionality off). The code in 9.1 is unchanged by this PR, and that knows how to handle the patches. In practice, it makes very little difference. As Ben says, 9.1 & 8.19 will negotiate a transport version that 8.19 understands anyway, and 9.0 is banned from forming a cluster with 8.19.

Copy link
Member

@benwtrent benwtrent left a comment

Choose a reason for hiding this comment

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

Thank you @thecoop for digging into this.

Looks good to me @jimczi

@elasticsearchmachine elasticsearchmachine merged commit c2484d6 into elastic:8.19 Jun 11, 2025
15 checks passed
@jimczi jimczi deleted the 8_19_exclude_vectors_bwc branch June 11, 2025 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) >non-issue :Search Relevance/Vectors Vector search Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch v8.19.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants