Skip to content

Commit

Permalink
[Version] Don't spoof major for 3.0+ clusters (#2722) (#2749)
Browse files Browse the repository at this point in the history
Changes version comparison logic to only translate major version when
comparing with legacy 7x versions. This is needed beginning in 2.0 so
that when running 2.0+ versions in bwc mode for 3.0+ upgrades, node
versions no longer have to translate major version or spoof to legacy
versions.

Signed-off-by: Nicholas Walter Knize <[email protected]>
(cherry picked from commit 2d716ad)
  • Loading branch information
opensearch-trigger-bot[bot] authored Apr 4, 2022
1 parent 8b4d879 commit 64b6105
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 21 deletions.
42 changes: 23 additions & 19 deletions server/src/main/java/org/opensearch/Version.java
Original file line number Diff line number Diff line change
Expand Up @@ -279,11 +279,16 @@ public boolean onOrBefore(Version version) {
return version.id >= id;
}

// LegacyESVersion major 7 is equivalent to Version major 1
public int compareMajor(Version other) {
int m = major == 1 ? 7 : major == 2 ? 8 : major;
int om = other.major == 1 ? 7 : other.major == 2 ? 8 : other.major;
return Integer.compare(m, om);
// comparing Legacy 7x for bwc
// todo: remove the following when removing legacy support in 3.0.0
if (major == 7 || other.major == 7 || major == 6 || other.major == 6) {
// opensearch v1.x and v2.x need major translation to compare w/ legacy versions
int m = major == 1 ? 7 : major == 2 ? 8 : major;
int om = other.major == 1 ? 7 : other.major == 2 ? 8 : other.major;
return Integer.compare(m, om);
}
return Integer.compare(major, other.major);
}

@Override
Expand Down Expand Up @@ -339,12 +344,9 @@ protected Version computeMinCompatVersion() {
} else if (major == 6) {
// force the minimum compatibility for version 6 to 5.6 since we don't reference version 5 anymore
return LegacyESVersion.fromId(5060099);
}
/*
* TODO - uncomment this logic from OpenSearch version 3 onwards
*
else if (major >= 3) {
} else if (major >= 3 && major < 5) {
// all major versions from 3 onwards are compatible with last minor series of the previous major
// todo: remove 5 check when removing LegacyESVersionTests
Version bwcVersion = null;

for (int i = DeclaredVersionsHolder.DECLARED_VERSIONS.size() - 1; i >= 0; i--) {
Expand All @@ -358,7 +360,6 @@ else if (major >= 3) {
}
return bwcVersion == null ? this : bwcVersion;
}
*/

return Version.min(this, fromId(maskId((int) major * 1000000 + 0 * 10000 + 99)));
}
Expand Down Expand Up @@ -396,6 +397,10 @@ private Version computeMinIndexCompatVersion() {
bwcMajor = major - 1;
}
final int bwcMinor = 0;
if (major == 3) {
return Version.min(this, fromId((bwcMajor * 1000000 + bwcMinor * 10000 + 99) ^ MASK));
}
// todo remove below when LegacyESVersion is removed in 3.0
return Version.min(this, fromId((bwcMajor * 1000000 + bwcMinor * 10000 + 99)));
}

Expand All @@ -409,16 +414,15 @@ public boolean isCompatible(Version version) {
// OpenSearch version 2 is the functional equivalent of predecessor unreleased version "8"
// todo refactor this logic after removing deprecated features
int a = major;
if (major == 1) {
a = 7;
} else if (major == 2) {
a = 8;
}
int b = version.major;
if (version.major == 1) {
b = 7;
} else if (version.major == 2) {
b = 8;

if (a == 7 || b == 7 || a == 6 || b == 6) {
if (major <= 2) {
a += 6; // for legacy compatibility up to version 2.x (to compare minCompat)
}
if (version.major <= 2) {
b += 6; // for legacy compatibility up to version 2.x (to compare minCompat)
}
}

assert compatible == false || Math.max(a, b) - Math.min(a, b) <= 1;
Expand Down
4 changes: 2 additions & 2 deletions server/src/test/java/org/opensearch/LegacyESVersionTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ public void testVersionComparison() {

// compare opensearch version to LegacyESVersion
assertThat(Version.V_1_0_0.compareMajor(LegacyESVersion.V_7_0_0), is(0));
assertThat(Version.V_1_0_0.compareMajor(LegacyESVersion.fromString("6.3.0")), is(1));
assertThat(LegacyESVersion.fromString("6.3.0").compareMajor(Version.V_1_0_0), is(-1));
assertThat(Version.V_2_0_0.compareMajor(LegacyESVersion.fromString("7.3.0")), is(1));
assertThat(LegacyESVersion.fromString("7.3.0").compareMajor(Version.V_2_0_0), is(-1));
}

public void testMin() {
Expand Down
20 changes: 20 additions & 0 deletions server/src/test/java/org/opensearch/VersionTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,26 @@ public void testOpenSearchMinIndexCompatVersion() {
assertEquals(expected.revision, actual.revision);
}

/** test first version of opensearch compatibility that does not support legacy versions */
public void testOpenSearchPreLegacyRemoval() {
Version opensearchVersion = Version.fromString("3.0.0");
int opensearchMajor = opensearchVersion.major;
List<Version> candidates = VersionUtils.allOpenSearchVersions();
Version expectedMinIndexCompat = VersionUtils.getFirstVersionOfMajor(candidates, opensearchMajor - 1);
Version actualMinIndexCompat = opensearchVersion.minimumIndexCompatibilityVersion();

Version expectedMinCompat = VersionUtils.lastFirstReleasedMinorFromMajor(VersionUtils.allOpenSearchVersions(), opensearchMajor - 1);
Version actualMinCompat = opensearchVersion.minimumCompatibilityVersion();
// since some legacy versions still support build (alpha, beta, RC) we check major minor revision only
assertEquals(expectedMinIndexCompat.major, actualMinIndexCompat.major);
assertEquals(expectedMinIndexCompat.minor, actualMinIndexCompat.minor);
assertEquals(expectedMinIndexCompat.revision, actualMinIndexCompat.revision);

assertEquals(expectedMinCompat.major, actualMinCompat.major);
assertEquals(expectedMinCompat.minor, actualMinCompat.minor);
assertEquals(expectedMinCompat.revision, actualMinCompat.revision);
}

public void testToString() {
assertEquals("2.0.0-beta1", Version.fromString("2.0.0-beta1").toString());
assertEquals("5.0.0-alpha1", Version.fromId(5000001).toString());
Expand Down

0 comments on commit 64b6105

Please sign in to comment.