Skip to content

Conversation

@rjernst
Copy link
Member

@rjernst rjernst commented Jul 14, 2023

The index and transport versions are low level details of how a node behaves in a cluster. They were recently added to the main endpoint response, but they are too low level and should be moved to another endpoint TBD.

This commit removes those versions from the main endpoint response. Due to the fact lucene version is now derived from index version, this commit also adds an explicit lucene version member to the main response. Note that while this effectively reverts #97386 and #96900, the reading/writing transport code remains temporarily.

The index and transport versions are low level details of how a node
behaves in a cluster. They were recently added to the main endpoint
response, but they are too low level and should be moved to another
endpoint TBD.

This commit removes those versions from the main endpoint response. Due
to the fact lucene version is now derived from index version, this
commit also adds an explicit lucene version member to the main response.
@rjernst rjernst added >non-issue :Core/Infra/Core Core issues without another label labels Jul 14, 2023
@rjernst rjernst requested a review from thecoop July 14, 2023 01:52
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@elasticsearchmachine elasticsearchmachine added Team:Core/Infra Meta label for core/infra team v8.10.0 labels Jul 14, 2023
@rjernst
Copy link
Member Author

rjernst commented Jul 14, 2023

@thecoop I'm not sure how we can modify the verify version constants tests going forward. The test is built on the assumption we keep a full mapping of version to lucene version for every version of ES, but that is no longer the case. You changed the test to infer the lucene version from index version, but by removing index version from the root endpoint, we no longer have that. I'm wondering if this test still makes sense to keep. WDYT?

@thecoop
Copy link
Member

thecoop commented Jul 17, 2023

I think if the test doesn't make sense anymore it can go. We can still test that the lucene version it reports is the most recent one, but any more than that doesn't make sense.

@rjernst
Copy link
Member Author

rjernst commented Jul 17, 2023

The lucene version it reports won't be the latest. This test is a bwc test that checks the version of lucene reported by older versions of ES matches what lucene version we have in our local code for that older version. I guess we can migrate this test to use a new diagnostic api that returns index version, but until then I don't think it makes sense since we have decoupled Version from Lucene version.

@rjernst
Copy link
Member Author

rjernst commented Jul 17, 2023

I will disable the test here. I opened #97736 so we don't forget to re-enable the test once we have a better way to find index version through an api.

@rjernst rjernst requested a review from thecoop July 18, 2023 00:17
@rjernst rjernst merged commit 3f8f718 into elastic:main Jul 18, 2023
@rjernst rjernst added the v8.9.0 label Jul 18, 2023
@rjernst rjernst deleted the buildinfo/remove_internal_versions branch July 18, 2023 13:37
rjernst added a commit to rjernst/elasticsearch that referenced this pull request Jul 18, 2023
This is a partial backport of elastic#97675.

The transport version is a low level detail of how a node behaves in a cluster. It was recently added to the main endpoint response, but is too low level and should be moved to another endpoint TBD. This commit removes it from the main response, though keeps it as part of the wire format for the MainResponse to avoid any need for a bwc change to the format.
elasticsearchmachine pushed a commit that referenced this pull request Jul 18, 2023
This is a partial backport of #97675.

The transport version is a low level detail of how a node behaves in a
cluster. It was recently added to the main endpoint response, but is too
low level and should be moved to another endpoint TBD. This commit
removes it from the main response, though keeps it as part of the wire
format for the MainResponse to avoid any need for a bwc change to the
format.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Core/Infra/Core Core issues without another label >non-issue Team:Core/Infra Meta label for core/infra team v8.9.0 v8.10.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants