Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Lucene version to 9.11 #109219

Merged
merged 221 commits into from
Jun 12, 2024
Merged

Conversation

benwtrent
Copy link
Member

No description provided.

@benwtrent benwtrent marked this pull request as ready for review June 5, 2024 11:42
@benwtrent benwtrent requested review from a team as code owners June 5, 2024 11:42
@elasticsearchmachine elasticsearchmachine added the Team:Search Meta label for search team label Jun 5, 2024
@elasticsearchmachine
Copy link
Collaborator

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

@@ -1,5 +1,5 @@
elasticsearch = 8.15.0
lucene = 9.10.0
lucene = 9.11.0-snapshot-d433394b292
Copy link
Contributor

Choose a reason for hiding this comment

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

might wanna flip this on the Lucene RC build (rather than a snapshot)

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, I tried this, and added the RC location as mvn location, but now ES is looking in that location for ALL packages, and its failing the build. Any ideas on how to simply add a secondary mvn location to look for packages instead of overriding?

build.gradle Outdated Show resolved Hide resolved
Copy link
Contributor

@iverase iverase left a comment

Choose a reason for hiding this comment

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

I am investigating a possible issue in lucene 9.11, please do not merge.

@benwtrent
Copy link
Member Author

While still investigating here: apache/lucene#13475

I have disabled intra-merge concurrency by always returning a same thread executor.

@benwtrent
Copy link
Member Author

@martijnvg all the failures are around the new dynamic setting being changed.

Likely root cause: java.lang.IllegalArgumentException: Setting index.mapper.dynamic was removed after version 6.0.0 |  
-- | --
  | at org.elasticsearch.index.mapper.MapperService.<init>(MapperService.java:208) |  
  | at org.elasticsearch.cluster.metadata.IndexMetadataVerifier.checkMappingsCompatibility(IndexMetadataVerifier.java:177) |  
  | at org.elasticsearch.cluster.metadata.IndexMetadataVerifier.verifyIndexMetadata(IndexMetadataVerifier.java:87) |  
  | at org.elasticsearch.gateway.GatewayMetaState.upgradeMetadata(GatewayMetaState.java:279) |  
  | at org.elasticsearch.gateway.GatewayMetaState.upgradeMetadataForNode(GatewayMetaState.java:265) |  
  | at org.elasticsearch.gateway.GatewayMetaState.start(GatewayMetaState.java:178) |  
  | at org.elasticsearch.node.Node.start(Node.java:1187) |  
  | at org.elasticsearch.bootstrap.Bootstrap.start(Bootstrap.java:335) |  
  | at org.elasticsearch.bootstrap.Bootstrap.init(Bootstrap.java:443) |  
  | at org.elasticsearch.bootstrap.Elasticsearch.init(Elasticsearch.java:166) |  
  | at org.elasticsearch.bootstrap.Elasticsearch.execute(Elasticsearch.java:157) |  
  | at org.elasticsearch.cli.EnvironmentAwareCommand.execute(EnvironmentAwareCommand.java:77) |  
  | at org.elasticsearch.cli.Command.mainWithoutErrorHandling(Command.java:112) |  
  | at org.elasticsearch.cli.Command.main(Command.java:77) |  
  | at org.elasticsearch.bootstrap.Elasticsearch.main(Elasticsearch.java:122) |  
  | at org.elasticsearch.bootstrap.Elasticsearch.main(Elasticsearch.java:80)

Now, I am not sure why these particular BWC tests are running against much older versions than 7.17.22, but they are.

Here is an example failure: https://gradle-enterprise.elastic.co/s/7wyrf5fogxcqq

@martijnvg
Copy link
Member

@benwtrent Looks like the test ran against 7.12.1, this upgrade test should only run with 7.17.latest as old version. I will update this test to skip if old version is any other version.

@martijnvg
Copy link
Member

I opened #109574

@benwtrent
Copy link
Member Author

@martijnvg I realize that it ran against old versions and its weird to me, but there must be a reason? I am pretty sure versions have to be specifically set somewhere.

Do we know why these old versions are being tested against?

@martijnvg
Copy link
Member

Do we know why these old versions are being tested against?

I don't know. I actually surprised like you too. The PR that added these tests didn't have a test-full-bwc label, because I thought we were only testing against 7.17.latest for 7.x bwc tests and not against older 7.x minors. When I saw this test failure, I thought my assumption was just wrong. But right now I'm not sure anymore.

@martijnvg
Copy link
Member

We do document that from 7.x to 8.x, a cluster must first be on 7.17.x before doing the actual 8.x upgrade: https://www.elastic.co/guide/en/elastic-stack/8.14/upgrading-elastic-stack.html#prepare-to-upgrade

So I don't think we need to test upgrading from 7.16 or earlier to 8.x?

@benwtrent
Copy link
Member Author

@martijnvg I am not 100% a PR gets tested against 7.17 at all unless the test-full-bwc tag is added :/

@mark-vieira
Copy link
Contributor

mark-vieira commented Jun 11, 2024

We do document that from 7.x to 8.x, a cluster must first be on 7.17.x before doing the actual 8.x upgrade: https://www.elastic.co/guide/en/elastic-stack/8.14/upgrading-elastic-stack.html#prepare-to-upgrade

So I don't think we need to test upgrading from 7.16 or earlier to 8.x?

We do, because we test the upgrade path you describe. So we don't do rolling upgrade tests, but we still run full cluster restart tests that start a 7.16 cluster, upgrade to 7.17, and then upgrade to the current version.

@martijnvg I am not 100% a PR gets tested against 7.17 at all unless the test-full-bwc tag is added :/

Every PR is tested against the HEAD of the 7.17 branch, even without that label. The label will add testing for previously released version of Elasticsearch.

@benwtrent benwtrent merged commit fdd183d into elastic:main Jun 12, 2024
25 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cloud-deploy Publish cloud docker image for Cloud-First-Testing >feature :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team test-arm Pull Requests that should be tested against arm agents test-full-bwc Trigger full BWC version matrix tests test-windows Trigger CI checks on Windows v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants