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

Remove LegacyESVersion.V_6_3_x constants #1691

Merged
merged 1 commit into from
Dec 17, 2021

Conversation

nknize
Copy link
Collaborator

@nknize nknize commented Dec 9, 2021

This PR removes LegacyESVersion.V_6_3_x constants including all
pre-release versions and bug fixes.

closes #1688

@nknize nknize added v2.0.0 Version 2.0.0 non-issue bugs / unexpected behaviors that end up non issues; audit trail simple changes that aren't issues labels Dec 9, 2021
@nknize nknize requested a review from a team as a code owner December 9, 2021 22:31
@opensearch-ci-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 55dc879c063d8fef7410ea101587c3aaa72bbf95
Log 1418

Reports 1418

@adnapibar
Copy link
Contributor

❌   Gradle Check failure 55dc879 Log 1418

Reports 1418

The log file is about 1GB 😮

@nknize
Copy link
Collaborator Author

nknize commented Dec 13, 2021

The log file is about 1GB

There's a logger issue. Going to revert that change for now and address separately.

@nknize nknize force-pushed the remove/6.3Constant branch from 55dc879 to a7154ae Compare December 16, 2021 20:24
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success a7154aebe49b5ac20578bda08233c9cef70a6f7a
Log 1545

Reports 1545

@nknize
Copy link
Collaborator Author

nknize commented Dec 16, 2021

@adnapibar To ensure this isn't a random success I'm going to keep splitting this into a few separate removal PRs until this is isolated to just the transport layer.

@nknize nknize force-pushed the remove/6.3Constant branch from a7154ae to db91366 Compare December 17, 2021 00:49
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure db91366f761e29524c4822a63b54f68e936b1717
Log 1558

Reports 1558

@@ -243,8 +243,6 @@ public ShardSearchRequest(StreamInput in) throws IOException {
clusterAlias = in.readOptionalString();
if (in.getVersion().onOrAfter(LegacyESVersion.V_7_0_0)) {
allowPartialSearchResults = in.readBoolean();
} else if (in.getVersion().onOrAfter(LegacyESVersion.V_6_3_0)) {
Copy link
Collaborator

@reta reta Dec 17, 2021

Choose a reason for hiding this comment

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

@nknize I think we have an issue here, the in.readOptionalBoolean(); should be followed for the version prior to LegacyESVersion.V_7_0_0, it looks like we have to use V_6_4_0 instead of removing the condition altogether:

} else if (in.getVersion().onOrAfter(LegacyESVersion.V_6_4_0)) {
    allowPartialSearchResults = in.readOptionalBoolean();
}

WDYT?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That logic path won't ever be executed.

minIndexCompatVersion for OpenSearch 2.0.0 is ES 7.0.0 and minCompatVersion is ES 7.10.0 so a Mixed cluster of OpenSearch 2.0.0 and ES 6.x nodes or indexes is not possible. The first if condition in this version will always be true.

This PR is the next in a line of incremental "remove all 6.x logic" PRs.

This commit removes LegacyESVersion.V_6_3_x constants including all
pre-release versions and bug fixes.

Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
@nknize nknize force-pushed the remove/6.3Constant branch from db91366 to 036520c Compare December 17, 2021 17:54
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 036520c
Log 1576

Reports 1576

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
non-issue bugs / unexpected behaviors that end up non issues; audit trail simple changes that aren't issues v2.0.0 Version 2.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove LegacyESVersion.V_6_3_x constants
4 participants