Skip to content

Commit 0d57e10

Browse files
alexey-ivanov-esalexey-ivanov-es
authored
TransportVersionUtils#randomVersionBetween does not work with version extensions (106119) (#116198)
Introduces a new extension method to VersionExtension enabling extensions to provide additional versions and creates method TransportVersion.getAllVersions returning all transport versions defined by Elasticsearch and the extension. This ensures that TransportVersion.current() always returns the correct current (latest) transport version even if it is defined by an extension. Fixes #106119
1 parent 27b07b3 commit 0d57e10

File tree

9 files changed

+93
-89
lines changed

9 files changed

+93
-89
lines changed

‎server/src/main/java/org/elasticsearch/TransportVersion.java‎

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,14 @@
1616
import org.elasticsearch.plugins.ExtensionLoader;
1717

1818
import java.io.IOException;
19+
import java.util.Collection;
20+
import java.util.Collections;
21+
import java.util.List;
22+
import java.util.Map;
1923
import java.util.ServiceLoader;
24+
import java.util.function.Function;
25+
import java.util.stream.Collectors;
26+
import java.util.stream.Stream;
2027

2128
/**
2229
* Represents the version of the wire protocol used to communicate between a pair of ES nodes.
@@ -56,8 +63,14 @@ public static TransportVersion readVersion(StreamInput in) throws IOException {
5663
return fromId(in.readVInt());
5764
}
5865

66+
/**
67+
* Finds a {@code TransportVersion} by its id.
68+
* If a transport version with the specified ID does not exist,
69+
* this method creates and returns a new instance of {@code TransportVersion} with the specified ID.
70+
* The new instance is not registered in {@code TransportVersion.getAllVersions}.
71+
*/
5972
public static TransportVersion fromId(int id) {
60-
TransportVersion known = TransportVersions.VERSION_IDS.get(id);
73+
TransportVersion known = VersionsHolder.ALL_VERSIONS_MAP.get(id);
6174
if (known != null) {
6275
return known;
6376
}
@@ -95,7 +108,14 @@ public static boolean isCompatible(TransportVersion version) {
95108
* This should be the transport version with the highest id.
96109
*/
97110
public static TransportVersion current() {
98-
return CurrentHolder.CURRENT;
111+
return VersionsHolder.CURRENT;
112+
}
113+
114+
/**
115+
* Sorted list of all defined transport versions
116+
*/
117+
public static List<TransportVersion> getAllVersions() {
118+
return VersionsHolder.ALL_VERSIONS;
99119
}
100120

101121
public static TransportVersion fromString(String str) {
@@ -139,16 +159,25 @@ public String toString() {
139159
return Integer.toString(id);
140160
}
141161

142-
private static class CurrentHolder {
143-
private static final TransportVersion CURRENT = findCurrent();
162+
private static class VersionsHolder {
163+
private static final List<TransportVersion> ALL_VERSIONS;
164+
private static final Map<Integer, TransportVersion> ALL_VERSIONS_MAP;
165+
private static final TransportVersion CURRENT;
166+
167+
static {
168+
Collection<TransportVersion> extendedVersions = ExtensionLoader.loadSingleton(ServiceLoader.load(VersionExtension.class))
169+
.map(VersionExtension::getTransportVersions)
170+
.orElse(Collections.emptyList());
171+
172+
if (extendedVersions.isEmpty()) {
173+
ALL_VERSIONS = TransportVersions.DEFINED_VERSIONS;
174+
} else {
175+
ALL_VERSIONS = Stream.concat(TransportVersions.DEFINED_VERSIONS.stream(), extendedVersions.stream()).sorted().toList();
176+
}
177+
178+
ALL_VERSIONS_MAP = ALL_VERSIONS.stream().collect(Collectors.toUnmodifiableMap(TransportVersion::id, Function.identity()));
144179

145-
// finds the pluggable current version
146-
private static TransportVersion findCurrent() {
147-
var version = ExtensionLoader.loadSingleton(ServiceLoader.load(VersionExtension.class))
148-
.map(e -> e.getCurrentTransportVersion(TransportVersions.LATEST_DEFINED))
149-
.orElse(TransportVersions.LATEST_DEFINED);
150-
assert version.onOrAfter(TransportVersions.LATEST_DEFINED);
151-
return version;
180+
CURRENT = ALL_VERSIONS.getLast();
152181
}
153182
}
154183
}

‎server/src/main/java/org/elasticsearch/TransportVersions.java‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,12 @@
1313
import org.elasticsearch.core.UpdateForV9;
1414

1515
import java.lang.reflect.Field;
16-
import java.util.Collection;
16+
import java.util.ArrayList;
1717
import java.util.Collections;
1818
import java.util.HashMap;
19+
import java.util.List;
1920
import java.util.Map;
20-
import java.util.NavigableMap;
2121
import java.util.Set;
22-
import java.util.TreeMap;
2322
import java.util.TreeSet;
2423
import java.util.function.IntFunction;
2524

@@ -209,21 +208,24 @@ static TransportVersion def(int id) {
209208
*/
210209
public static final TransportVersion MINIMUM_CCS_VERSION = V_8_15_0;
211210

212-
static final NavigableMap<Integer, TransportVersion> VERSION_IDS = getAllVersionIds(TransportVersions.class);
211+
/**
212+
* Sorted list of all versions defined in this class
213+
*/
214+
static final List<TransportVersion> DEFINED_VERSIONS = collectAllVersionIdsDefinedInClass(TransportVersions.class);
213215

214-
// the highest transport version constant defined in this file, used as a fallback for TransportVersion.current()
216+
// the highest transport version constant defined
215217
static final TransportVersion LATEST_DEFINED;
216218
static {
217-
LATEST_DEFINED = VERSION_IDS.lastEntry().getValue();
219+
LATEST_DEFINED = DEFINED_VERSIONS.getLast();
218220

219221
// see comment on IDS field
220222
// now we're registered all the transport versions, we can clear the map
221223
IDS = null;
222224
}
223225

224-
public static NavigableMap<Integer, TransportVersion> getAllVersionIds(Class<?> cls) {
226+
public static List<TransportVersion> collectAllVersionIdsDefinedInClass(Class<?> cls) {
225227
Map<Integer, String> versionIdFields = new HashMap<>();
226-
NavigableMap<Integer, TransportVersion> builder = new TreeMap<>();
228+
List<TransportVersion> definedTransportVersions = new ArrayList<>();
227229

228230
Set<String> ignore = Set.of("ZERO", "CURRENT", "MINIMUM_COMPATIBLE", "MINIMUM_CCS_VERSION");
229231

@@ -240,7 +242,7 @@ public static NavigableMap<Integer, TransportVersion> getAllVersionIds(Class<?>
240242
} catch (IllegalAccessException e) {
241243
throw new AssertionError(e);
242244
}
243-
builder.put(version.id(), version);
245+
definedTransportVersions.add(version);
244246

245247
if (Assertions.ENABLED) {
246248
// check the version number is unique
@@ -257,11 +259,9 @@ public static NavigableMap<Integer, TransportVersion> getAllVersionIds(Class<?>
257259
}
258260
}
259261

260-
return Collections.unmodifiableNavigableMap(builder);
261-
}
262+
Collections.sort(definedTransportVersions);
262263

263-
static Collection<TransportVersion> getAllVersions() {
264-
return VERSION_IDS.values();
264+
return List.copyOf(definedTransportVersions);
265265
}
266266

267267
static final IntFunction<String> VERSION_LOOKUP = ReleaseVersions.generateVersionsLookup(TransportVersions.class, LATEST_DEFINED.id());

‎server/src/main/java/org/elasticsearch/internal/VersionExtension.java‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,16 @@
1212
import org.elasticsearch.TransportVersion;
1313
import org.elasticsearch.index.IndexVersion;
1414

15+
import java.util.List;
16+
1517
/**
1618
* Allows plugging in current version elements.
1719
*/
1820
public interface VersionExtension {
1921
/**
20-
* Returns the {@link TransportVersion} that Elasticsearch should use.
21-
* <p>
22-
* This must be at least as high as the given fallback.
23-
* @param fallback The latest transport version from server
22+
* Returns list of {@link TransportVersion} defined by extension
2423
*/
25-
TransportVersion getCurrentTransportVersion(TransportVersion fallback);
24+
List<TransportVersion> getTransportVersions();
2625

2726
/**
2827
* Returns the {@link IndexVersion} that Elasticsearch should use.

‎server/src/test/java/org/elasticsearch/TransportVersionTests.java‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
import java.lang.reflect.Modifier;
1616
import java.util.Collections;
17-
import java.util.Map;
17+
import java.util.List;
1818
import java.util.Set;
1919
import java.util.TreeSet;
2020
import java.util.regex.Matcher;
@@ -69,21 +69,20 @@ public static class DuplicatedIdFakeVersion {
6969

7070
public void testStaticTransportVersionChecks() {
7171
assertThat(
72-
TransportVersions.getAllVersionIds(CorrectFakeVersion.class),
72+
TransportVersions.collectAllVersionIdsDefinedInClass(CorrectFakeVersion.class),
7373
equalTo(
74-
Map.of(
75-
199,
76-
CorrectFakeVersion.V_0_00_01,
77-
2,
74+
List.of(
7875
CorrectFakeVersion.V_0_000_002,
79-
3,
8076
CorrectFakeVersion.V_0_000_003,
81-
4,
82-
CorrectFakeVersion.V_0_000_004
77+
CorrectFakeVersion.V_0_000_004,
78+
CorrectFakeVersion.V_0_00_01
8379
)
8480
)
8581
);
86-
AssertionError e = expectThrows(AssertionError.class, () -> TransportVersions.getAllVersionIds(DuplicatedIdFakeVersion.class));
82+
AssertionError e = expectThrows(
83+
AssertionError.class,
84+
() -> TransportVersions.collectAllVersionIdsDefinedInClass(DuplicatedIdFakeVersion.class)
85+
);
8786
assertThat(e.getMessage(), containsString("have the same version number"));
8887
}
8988

@@ -186,7 +185,7 @@ public void testVersionConstantPresent() {
186185
}
187186

188187
public void testCURRENTIsLatest() {
189-
assertThat(Collections.max(TransportVersions.getAllVersions()), is(TransportVersion.current()));
188+
assertThat(Collections.max(TransportVersion.getAllVersions()), is(TransportVersion.current()));
190189
}
191190

192191
public void testToReleaseVersion() {
@@ -210,7 +209,7 @@ public void testToString() {
210209
public void testDenseTransportVersions() {
211210
Set<Integer> missingVersions = new TreeSet<>();
212211
TransportVersion previous = null;
213-
for (var tv : TransportVersions.getAllVersions()) {
212+
for (var tv : TransportVersion.getAllVersions()) {
214213
if (tv.before(TransportVersions.V_8_16_0)) {
215214
continue;
216215
}

‎test/framework/src/main/java/org/elasticsearch/KnownTransportVersions.java‎

Lines changed: 0 additions & 22 deletions
This file was deleted.

‎test/framework/src/main/java/org/elasticsearch/test/BWCVersions.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,13 @@
1515
import java.util.Collections;
1616
import java.util.List;
1717

18-
import static org.elasticsearch.KnownTransportVersions.ALL_VERSIONS;
19-
2018
public final class BWCVersions {
2119
private BWCVersions() {}
2220

2321
public static List<TransportVersion> getAllBWCVersions() {
24-
int minCompatVersion = Collections.binarySearch(ALL_VERSIONS, TransportVersions.MINIMUM_COMPATIBLE);
25-
return ALL_VERSIONS.subList(minCompatVersion, ALL_VERSIONS.size());
22+
List<TransportVersion> allVersions = TransportVersion.getAllVersions();
23+
int minCompatVersion = Collections.binarySearch(allVersions, TransportVersions.MINIMUM_COMPATIBLE);
24+
return allVersions.subList(minCompatVersion, allVersions.size());
2625
}
2726

2827
public static final List<TransportVersion> DEFAULT_BWC_VERSIONS = getAllBWCVersions();

‎test/framework/src/main/java/org/elasticsearch/test/TransportVersionUtils.java‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,32 +19,30 @@
1919
import java.util.Set;
2020
import java.util.stream.Collectors;
2121

22-
import static org.elasticsearch.KnownTransportVersions.ALL_VERSIONS;
23-
2422
public class TransportVersionUtils {
2523
/** Returns all released versions */
2624
public static List<TransportVersion> allReleasedVersions() {
27-
return ALL_VERSIONS;
25+
return TransportVersion.getAllVersions();
2826
}
2927

3028
/** Returns the oldest known {@link TransportVersion} */
3129
public static TransportVersion getFirstVersion() {
32-
return ALL_VERSIONS.get(0);
30+
return allReleasedVersions().getFirst();
3331
}
3432

3533
/** Returns a random {@link TransportVersion} from all available versions. */
3634
public static TransportVersion randomVersion() {
37-
return ESTestCase.randomFrom(ALL_VERSIONS);
35+
return ESTestCase.randomFrom(allReleasedVersions());
3836
}
3937

4038
/** Returns a random {@link TransportVersion} from all available versions without the ignore set */
4139
public static TransportVersion randomVersion(Set<TransportVersion> ignore) {
42-
return ESTestCase.randomFrom(ALL_VERSIONS.stream().filter(v -> ignore.contains(v) == false).collect(Collectors.toList()));
40+
return ESTestCase.randomFrom(allReleasedVersions().stream().filter(v -> ignore.contains(v) == false).collect(Collectors.toList()));
4341
}
4442

4543
/** Returns a random {@link TransportVersion} from all available versions. */
4644
public static TransportVersion randomVersion(Random random) {
47-
return ALL_VERSIONS.get(random.nextInt(ALL_VERSIONS.size()));
45+
return allReleasedVersions().get(random.nextInt(allReleasedVersions().size()));
4846
}
4947

5048
/** Returns a random {@link TransportVersion} between <code>minVersion</code> and <code>maxVersion</code> (inclusive). */
@@ -58,12 +56,13 @@ public static TransportVersion randomVersionBetween(
5856
}
5957

6058
int minVersionIndex = 0;
59+
List<TransportVersion> allReleasedVersions = allReleasedVersions();
6160
if (minVersion != null) {
62-
minVersionIndex = Collections.binarySearch(ALL_VERSIONS, minVersion);
61+
minVersionIndex = Collections.binarySearch(allReleasedVersions, minVersion);
6362
}
64-
int maxVersionIndex = ALL_VERSIONS.size() - 1;
63+
int maxVersionIndex = allReleasedVersions.size() - 1;
6564
if (maxVersion != null) {
66-
maxVersionIndex = Collections.binarySearch(ALL_VERSIONS, maxVersion);
65+
maxVersionIndex = Collections.binarySearch(allReleasedVersions, maxVersion);
6766
}
6867
if (minVersionIndex < 0) {
6968
throw new IllegalArgumentException("minVersion [" + minVersion + "] does not exist.");
@@ -72,7 +71,7 @@ public static TransportVersion randomVersionBetween(
7271
} else {
7372
// minVersionIndex is inclusive so need to add 1 to this index
7473
int range = maxVersionIndex + 1 - minVersionIndex;
75-
return ALL_VERSIONS.get(minVersionIndex + random.nextInt(range));
74+
return allReleasedVersions.get(minVersionIndex + random.nextInt(range));
7675
}
7776
}
7877

@@ -83,7 +82,7 @@ public static TransportVersion getPreviousVersion() {
8382
}
8483

8584
public static TransportVersion getPreviousVersion(TransportVersion version) {
86-
int place = Collections.binarySearch(ALL_VERSIONS, version);
85+
int place = Collections.binarySearch(allReleasedVersions(), version);
8786
if (place < 0) {
8887
// version does not exist - need the item before the index this version should be inserted
8988
place = -(place + 1);
@@ -92,15 +91,16 @@ public static TransportVersion getPreviousVersion(TransportVersion version) {
9291
if (place < 1) {
9392
throw new IllegalArgumentException("couldn't find any released versions before [" + version + "]");
9493
}
95-
return ALL_VERSIONS.get(place - 1);
94+
return allReleasedVersions().get(place - 1);
9695
}
9796

9897
public static TransportVersion getNextVersion(TransportVersion version) {
9998
return getNextVersion(version, false);
10099
}
101100

102101
public static TransportVersion getNextVersion(TransportVersion version, boolean createIfNecessary) {
103-
int place = Collections.binarySearch(ALL_VERSIONS, version);
102+
List<TransportVersion> allReleasedVersions = allReleasedVersions();
103+
int place = Collections.binarySearch(allReleasedVersions, version);
104104
if (place < 0) {
105105
// version does not exist - need the item at the index this version should be inserted
106106
place = -(place + 1);
@@ -109,15 +109,15 @@ public static TransportVersion getNextVersion(TransportVersion version, boolean
109109
place++;
110110
}
111111

112-
if (place < 0 || place >= ALL_VERSIONS.size()) {
112+
if (place < 0 || place >= allReleasedVersions.size()) {
113113
if (createIfNecessary) {
114114
// create a new transport version one greater than specified
115115
return new TransportVersion(version.id() + 1);
116116
} else {
117117
throw new IllegalArgumentException("couldn't find any released versions after [" + version + "]");
118118
}
119119
}
120-
return ALL_VERSIONS.get(place);
120+
return allReleasedVersions.get(place);
121121
}
122122

123123
/** Returns a random {@code TransportVersion} that is compatible with {@link TransportVersion#current()} */

‎x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/AbstractBWCSerializationTestCase.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,14 @@
1616
import java.util.Collections;
1717
import java.util.List;
1818

19-
import static org.elasticsearch.KnownTransportVersions.ALL_VERSIONS;
2019
import static org.hamcrest.Matchers.equalTo;
2120

2221
public abstract class AbstractBWCSerializationTestCase<T extends Writeable & ToXContent> extends AbstractXContentSerializingTestCase<T> {
2322

2423
private static List<TransportVersion> getAllBWCVersions() {
25-
int minCompatVersion = Collections.binarySearch(ALL_VERSIONS, TransportVersions.MINIMUM_COMPATIBLE);
26-
return ALL_VERSIONS.subList(minCompatVersion, ALL_VERSIONS.size());
24+
List<TransportVersion> allVersions = TransportVersion.getAllVersions();
25+
int minCompatVersion = Collections.binarySearch(allVersions, TransportVersions.MINIMUM_COMPATIBLE);
26+
return allVersions.subList(minCompatVersion, allVersions.size());
2727
}
2828

2929
private static final List<TransportVersion> DEFAULT_BWC_VERSIONS = getAllBWCVersions();

0 commit comments

Comments
 (0)