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

Include 8.x backport version in index version checks for offset encoding #125447

Conversation

jordan-powers
Copy link
Contributor

This patch updates the index version checks in FieldArrayContext#getOffsetsFieldName to be aware of indices with offset encodings possibly created on 8.x versions of elasticsearch.

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-storage-engine (Team:StorageEngine)

@@ -136,6 +136,8 @@ private static Version parseUnchecked(String version) {
public static final IndexVersion LOGSB_OPTIONAL_SORTING_ON_HOST_NAME_BACKPORT = def(8_525_0_00, parseUnchecked("9.12.1"));
public static final IndexVersion USE_SYNTHETIC_SOURCE_FOR_RECOVERY_BY_DEFAULT_BACKPORT = def(8_526_0_00, parseUnchecked("9.12.1"));
public static final IndexVersion SYNTHETIC_SOURCE_STORE_ARRAYS_NATIVELY_KEYWORD_BACKPORT_8_X = def(8_527_0_00, Version.LUCENE_9_12_1);
public static final IndexVersion SYNTHETIC_SOURCE_STORE_ARRAYS_NATIVELY_NUMBER_BACKPORT_8_X = def(8_528_0_00, Version.LUCENE_9_12_1);
public static final IndexVersion SYNTHETIC_SOURCE_STORE_ARRAYS_NATIVELY_BOOLEAN_BACKPORT_8_X = def(8_529_0_00, Version.LUCENE_9_12_1);
Copy link
Member

Choose a reason for hiding this comment

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

I think this version doesn't exist in the 8.x branch?

Thinking more about this, given the 8.19.0 has not been released yet. I think we reuse the SYNTHETIC_SOURCE_STORE_ARRAYS_NATIVELY_KEYWORD_BACKPORT_8_X index version. Also in 8.x branch, however I see that SYNTHETIC_SOURCE_STORE_ARRAYS_NATIVELY_NUMBER was added. So In think here we can just use SYNTHETIC_SOURCE_STORE_ARRAYS_NATIVELY_NUMBER in places where SYNTHETIC_SOURCE_STORE_ARRAYS_NATIVELY_BOOLEAN_BACKPORT_8_X is used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I've been backporting the PRs adding offset encoding, I've been creating new index versions for each backport. The index version SYNTHETIC_SOURCE_STORE_ARRAYS_NATIVELY_BOOLEAN_BACKPORT_8_X (8_529) is still pending in backport PR #125596.

Instead of adding a new version, should I update #125596 to instead use 8_528 for the boolean field mapper, since there hasn't been an 8.x release yet?

If so, should I also use 8_528 for future offset encoding backports (unsigned long and scaled float)?

Then I would update this PR to use 8_528 for both numeric and boolean fields.

Copy link
Contributor Author

@jordan-powers jordan-powers Mar 25, 2025

Choose a reason for hiding this comment

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

OK, I opened #125610 to use 8_527 for all the 8.x offset encoding implementations. I also updated the version checks here to always just look at 8_527. Which is what we were already doing, so this PR is essentially just renaming the constant.

Per elastic#125610 we only need 1 index version for the 8.x offset encoding
implementations (since there haven't been any 8.x releases).
Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

LGTM

@jordan-powers jordan-powers merged commit 24734e6 into elastic:main Mar 25, 2025
17 checks passed
@jordan-powers jordan-powers deleted the offset-numeric-arrays-index-version-fix branch March 25, 2025 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants