Repository navigation
Conversation
|
I'm +1 to the idea |
|
I think we can't do this and also have the complexity of two different supported min versions. It's too confusing. |
I agree. Now that we are relaxing the minimum-write-supported-version, let's just remove the read-only (binary) minimum version. |
|
Thanks for looking @rmuir and @HoustonPutman !
That's a good point! I see where you are coming from. The split between the two min supported versions is at the moment additional complexity for no additional value, if the two versions align. The reason why I kept it is a bit speculative perhaps: what happens once we make a breaking change that requires bumping? May we still want to support older versions in read-only fashion? That may become unnecessary, given that the main reason why we have two min supported versions in the first place is that we always bumped the min supported version at every major. I think we could state that the expert APIs ( Let me know what you think, I hope I have accurately interpreted your comments. |
Plaase lets just get rid of all this and reduce the confusion. its now obselete. |
| // TODO - WHY ONLY on the first major version? | ||
| params.add(new Object[] {Version.LUCENE_10_0_0, createPattern(INDEX_NAME, SUFFIX)}); | ||
| return params; | ||
| return allInitialMajorVersion(INDEX_NAME, SUFFIX); |
There was a problem hiding this comment.
this is now generalized rather than hardcoded. It should no longer require manual action when min support version changes in the future. Same for the following two test classes. TODO is now obsolete.
|
@rmuir @HoustonPutman @ChrisHegarty I pushed an update and spent more time updating the migrate guide and docs. Are there other places that explain the general compatibility policy, which will need updating? |
ChrisHegarty
left a comment
ChrisHegarty
left a comment
There was a problem hiding this comment.
Thanks @javanna. LGTM
| 9.12.0 | ||
| 9.12.1 | ||
| 9.12.2 | ||
| 9.12.3 |
There was a problem hiding this comment.
all the 9.x versions should have been previously removed I believe, but they were not. That is the reason why I only had to restore that last two minors of the 9.x series. Funnily enough though, the list above of unsupported versions was not comprehensive either.
5 participants
After merging #15012 to main, Lucene no longer strictly requires bumping the minimum supported major version when releasing a new major. That will still be necessary once breaking changes are introduced that require reindexing, but can be avoided when not necessary. This was discussed in the corresponding issue at #13797, yet while the change providing the infra for this important change was merged to main, the minimum supported version in main (future 11) remained 10.
Several people asked if we could lower the minimum supported major version to 9 in main, so that Lucene 11 would be able to support reading and writing indices created by Lucene 9.x and effectively not require reindexing. I find that this would be a great improvement and would help users more quickly and less painfully migrate to Lucene 11 once it comes out. Effectively Lucene 11 would come with the minimum JDK bump requirement and breaking API changes, but no min major version bump.
I did some research on the topic and this is possible today in main as no breaking changes were made at the codecs level. In practice, there does not seem to be any technical reason why we should prevent
IndexWriterfrom opening indices created in 9.x in Lucene 11. From a cost and maintenance perspective, 9.x codecs are already in the codebase and would be shipped with Lucene 11 anyways, to allow users to read such indices using the existing expert APIDirectoryReader#open(IndexCommit commit, int minSupportedMajorVersion).It is a significant policy change that may cause confusion, as the compatibility policy will vary across major versions. Like we discussed in previous threads, reindexing will still be required in future major versions once necessary.
The change is easier than I initially thought, thanks to all the prep made with #15012 and consists of the following steps:
Version.MIN_SUPPORTED_MAJORto 9 (was 10)IndexWriterConfig#setIndexCreatedVersionMajorversion checks (leftover LATEST.major - 1 check)unsupported_versions.txtversions.txtTestIndexWriter,TestMoreTermsBackwardsCompatibility,TestMinSupportedMajorBackwardsCompatibility,TestEmptyIndexBackwardsCompatibility,TestDVUpdateBackwardsCompatibility,TestBinaryBackwardsCompatibility,TestBasicBackwardsCompatibility,TestAncientIndicesCompatibility,BackwardsCompatibilityTestBaseLucene backwards compatibility tests look green against these changes. I was able to also get additional coverage from Elasticsearch tests run against this branch, that showed no compatibility issues. Are there more tests that we'd want to write to ensure this change is safe? Any additional checks to run?
This change can easily be split into multiple PRs to ease reviews. Meanwhile, what is included in this PR is comprehensive and shows the proposed direction so we can start this discussion and collect feedback about it.