Skip to content

Commit

Permalink
Remove final uses of minimum compatible version from TransportVersion (
Browse files Browse the repository at this point in the history
…#93453)

Migrate existing test uses to TransportVersionUtils
  • Loading branch information
thecoop committed Feb 16, 2023
1 parent 6c89215 commit 3053b17
Show file tree
Hide file tree
Showing 39 changed files with 129 additions and 137 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;
import org.elasticsearch.mocksocket.MockServerSocket;
import org.elasticsearch.test.TransportVersionUtils;
import org.elasticsearch.test.transport.MockTransportService;
import org.elasticsearch.test.transport.StubbableTransport;
import org.elasticsearch.transport.AbstractSimpleTransportTestCase;
Expand Down Expand Up @@ -78,7 +79,7 @@ public void executeHandshake(
if (doHandshake) {
super.executeHandshake(node, channel, profile, listener);
} else {
listener.onResponse(version.calculateMinimumCompatVersion());
listener.onResponse(TransportVersionUtils.minimumCompatibilityVersion(version));
}
}
};
Expand Down
18 changes: 3 additions & 15 deletions server/src/main/java/org/elasticsearch/TransportVersion.java
Original file line number Diff line number Diff line change
Expand Up @@ -247,23 +247,11 @@ public TransportVersion minimumCompatibilityVersion() {
return MINIMUM_COMPATIBLE;
}

@Deprecated(forRemoval = true)
public boolean isCompatible(TransportVersion version) {
return onOrAfter(version.calculateMinimumCompatVersion()) && version.onOrAfter(calculateMinimumCompatVersion());
}

private TransportVersion minimumCompatibleVersion;

/**
* Placeholder for code calling {@code minimumCompatibilityVersion} on arbitrary Version instances.
* Code calling this should be refactored to not do this.
* Returns {@code true} if the specified version is compatible with this running version of Elasticsearch.
*/
@Deprecated(forRemoval = true)
public TransportVersion calculateMinimumCompatVersion() {
if (minimumCompatibleVersion == null) {
minimumCompatibleVersion = Version.findVersion(this).minimumCompatibilityVersion().transportVersion;
}
return minimumCompatibleVersion;
public static boolean isCompatible(TransportVersion version) {
return version.onOrAfter(MINIMUM_COMPATIBLE);
}

public boolean after(TransportVersion version) {
Expand Down
14 changes: 0 additions & 14 deletions server/src/main/java/org/elasticsearch/Version.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import java.util.Locale;
import java.util.Map;
import java.util.NavigableMap;
import java.util.NoSuchElementException;
import java.util.Objects;
import java.util.TreeMap;

Expand Down Expand Up @@ -196,19 +195,6 @@ public static Version readVersion(StreamInput in) throws IOException {
return fromId(in.readVInt());
}

/**
* Returns the highest Version that has this or a lesser TransportVersion.
*/
@Deprecated
static Version findVersion(TransportVersion transportVersion) {
return VERSION_IDS.descendingMap()
.values()
.stream()
.filter(v -> v.transportVersion.compareTo(transportVersion) <= 0)
.findFirst()
.orElseThrow(() -> new NoSuchElementException("No valid Version found")); // only if transportVersion < 0 ?????
}

public static Version fromId(int id) {
final Version known = VERSION_IDS.get(id);
if (known != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ public String getWriteableName() {

@Override
public TransportVersion getMinimalSupportedVersion() {
return TransportVersion.CURRENT.minimumCompatibilityVersion();
return TransportVersion.MINIMUM_COMPATIBLE;
}

public static NamedDiff<Custom> readDiffFrom(StreamInput in) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ public static NamedDiff<Custom> readDiffFrom(StreamInput in) throws IOException

@Override
public TransportVersion getMinimalSupportedVersion() {
return TransportVersion.CURRENT.minimumCompatibilityVersion();
return TransportVersion.MINIMUM_COMPATIBLE;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ public String getWriteableName() {

@Override
public TransportVersion getMinimalSupportedVersion() {
return TransportVersion.CURRENT.minimumCompatibilityVersion();
return TransportVersion.MINIMUM_COMPATIBLE;
}

private static final TransportVersion DIFFABLE_VERSION = TransportVersion.V_8_5_0;
Expand Down Expand Up @@ -1614,7 +1614,7 @@ public SnapshotsInProgress apply(Custom part) {

@Override
public TransportVersion getMinimalSupportedVersion() {
return TransportVersion.CURRENT.minimumCompatibilityVersion();
return TransportVersion.MINIMUM_COMPATIBLE;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public String getWriteableName() {

@Override
public TransportVersion getMinimalSupportedVersion() {
return TransportVersion.CURRENT.minimumCompatibilityVersion();
return TransportVersion.MINIMUM_COMPATIBLE;
}

@Override
Expand Down Expand Up @@ -334,7 +334,7 @@ public String getWriteableName() {

@Override
public TransportVersion getMinimalSupportedVersion() {
return TransportVersion.CURRENT.minimumCompatibilityVersion();
return TransportVersion.MINIMUM_COMPATIBLE;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ public String getWriteableName() {

@Override
public TransportVersion getMinimalSupportedVersion() {
return TransportVersion.CURRENT.minimumCompatibilityVersion();
return TransportVersion.MINIMUM_COMPATIBLE;
}

public RepositoriesMetadata(StreamInput in) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public String getWriteableName() {

@Override
public TransportVersion getMinimalSupportedVersion() {
return TransportVersion.CURRENT.minimumCompatibilityVersion();
return TransportVersion.MINIMUM_COMPATIBLE;
}

public Map<String, PipelineConfiguration> getPipelines() {
Expand Down Expand Up @@ -149,7 +149,7 @@ public String getWriteableName() {

@Override
public TransportVersion getMinimalSupportedVersion() {
return TransportVersion.CURRENT.minimumCompatibilityVersion();
return TransportVersion.MINIMUM_COMPATIBLE;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ public long getNumberOfTasksOnNode(String nodeId, String taskName) {

@Override
public TransportVersion getMinimalSupportedVersion() {
return TransportVersion.CURRENT.minimumCompatibilityVersion();
return TransportVersion.MINIMUM_COMPATIBLE;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ public void writeTo(StreamOutput out) throws IOException {

@Override
public TransportVersion getMinimalSupportedVersion() {
return TransportVersion.CURRENT.minimumCompatibilityVersion();
return TransportVersion.MINIMUM_COMPATIBLE;
}
}

Expand Down Expand Up @@ -280,7 +280,7 @@ public String getWriteableName() {

@Override
public TransportVersion getMinimalSupportedVersion() {
return TransportVersion.CURRENT.minimumCompatibilityVersion();
return TransportVersion.MINIMUM_COMPATIBLE;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public int internalDecode(ReleasableBytesReference reference, Consumer<Object> f
} else {
totalNetworkSize = messageLength + TcpHeader.BYTES_REQUIRED_FOR_MESSAGE_SIZE;

Header header = readHeader(version, messageLength, reference);
Header header = readHeader(messageLength, reference);
bytesConsumed += headerBytesToRead;
if (header.isCompressed()) {
isCompressed = true;
Expand Down Expand Up @@ -171,8 +171,7 @@ private static int headerBytesToRead(BytesReference reference) {
}
}

// exposed for use in tests
static Header readHeader(TransportVersion version, int networkMessageSize, BytesReference bytesReference) throws IOException {
private static Header readHeader(int networkMessageSize, BytesReference bytesReference) throws IOException {
try (StreamInput streamInput = bytesReference.streamInput()) {
streamInput.skip(TcpHeader.BYTES_REQUIRED_FOR_MESSAGE_SIZE);
long requestId = streamInput.readLong();
Expand All @@ -183,7 +182,7 @@ static Header readHeader(TransportVersion version, int networkMessageSize, Bytes
if (header.isHandshake()) {
checkHandshakeVersionCompatibility(header.getVersion());
} else {
checkVersionCompatibility(header.getVersion(), version);
checkVersionCompatibility(header.getVersion());
}

if (header.getVersion().onOrAfter(TcpHeader.VERSION_WITH_HEADER_SIZE)) {
Expand Down Expand Up @@ -216,10 +215,14 @@ static void checkHandshakeVersionCompatibility(TransportVersion handshakeVersion
}
}

static void checkVersionCompatibility(TransportVersion remoteVersion, TransportVersion currentVersion) {
if (remoteVersion.isCompatible(currentVersion) == false) {
static void checkVersionCompatibility(TransportVersion remoteVersion) {
if (TransportVersion.isCompatible(remoteVersion) == false) {
throw new IllegalStateException(
"Received message from unsupported version: [" + remoteVersion + "] minimal compatible version is: [" + currentVersion + "]"
"Received message from unsupported version: ["
+ remoteVersion
+ "] minimal compatible version is: ["
+ TransportVersion.MINIMUM_COMPATIBLE
+ "]"
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ void sendHandshake(
ActionListener<TransportVersion> listener
) {
numHandshakes.inc();
final HandshakeResponseHandler handler = new HandshakeResponseHandler(requestId, version, listener);
final HandshakeResponseHandler handler = new HandshakeResponseHandler(requestId, listener);
pendingHandshakes.put(requestId, handler);
channel.addCloseListener(
ActionListener.running(() -> handler.handleLocalException(new TransportException("handshake failed because connection reset")))
Expand Down Expand Up @@ -211,13 +211,11 @@ long getNumHandshakes() {
private class HandshakeResponseHandler implements TransportResponseHandler<HandshakeResponse> {

private final long requestId;
private final TransportVersion currentVersion;
private final ActionListener<TransportVersion> listener;
private final AtomicBoolean isDone = new AtomicBoolean(false);

private HandshakeResponseHandler(long requestId, TransportVersion currentVersion, ActionListener<TransportVersion> listener) {
private HandshakeResponseHandler(long requestId, ActionListener<TransportVersion> listener) {
this.requestId = requestId;
this.currentVersion = currentVersion;
this.listener = listener;
}

Expand All @@ -230,13 +228,13 @@ public HandshakeResponse read(StreamInput in) throws IOException {
public void handleResponse(HandshakeResponse response) {
if (isDone.compareAndSet(false, true)) {
TransportVersion responseVersion = response.responseVersion;
if (currentVersion.isCompatible(responseVersion) == false) {
if (TransportVersion.isCompatible(responseVersion) == false) {
listener.onFailure(
new IllegalStateException(
"Received message from unsupported version: ["
+ responseVersion
+ "] minimal compatible version is: ["
+ currentVersion.calculateMinimumCompatVersion()
+ TransportVersion.MINIMUM_COMPATIBLE
+ "]"
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,11 +176,4 @@ public void testToString() {
assertEquals("2000099", TransportVersion.fromId(2_00_00_99).toString());
assertEquals("5000099", TransportVersion.fromId(5_00_00_99).toString());
}

public void testMinCompatVersion() {
Version minCompat = Version.fromId(TransportVersion.V_8_0_0.calculateMinimumCompatVersion().id);
assertEquals(7, minCompat.major);
assertEquals("This needs to be updated when 7.18 is released", 17, minCompat.minor);
assertEquals(0, minCompat.revision);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public void testSerialization() throws IOException {
MultiGetShardRequest multiGetShardRequest = createTestInstance(randomBoolean());

BytesStreamOutput out = new BytesStreamOutput();
TransportVersion minVersion = TransportVersion.CURRENT.minimumCompatibilityVersion();
TransportVersion minVersion = TransportVersion.MINIMUM_COMPATIBLE;
if (multiGetShardRequest.isForceSyntheticSource()) {
minVersion = TransportVersion.V_8_4_0;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ public static NamedDiff<Custom> readDiffFrom(StreamInput in) throws IOException

@Override
public TransportVersion getMinimalSupportedVersion() {
return TransportVersion.CURRENT.minimumCompatibilityVersion();
return TransportVersion.MINIMUM_COMPATIBLE;
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -372,25 +372,23 @@ public void testVersionIncompatibilityDecodeException() throws IOException {
public void testCheckVersionCompatibility() {
try {
InboundDecoder.checkVersionCompatibility(
TransportVersionUtils.randomVersionBetween(
random(),
TransportVersion.CURRENT.minimumCompatibilityVersion(),
TransportVersion.CURRENT
),
TransportVersion.CURRENT
TransportVersionUtils.randomVersionBetween(random(), TransportVersion.MINIMUM_COMPATIBLE, TransportVersion.CURRENT)
);
} catch (IllegalStateException e) {
throw new AssertionError(e);
}

TransportVersion version = TransportVersion.CURRENT;
TransportVersion invalid = TransportVersionUtils.getPreviousVersion(version.minimumCompatibilityVersion());
TransportVersion invalid = TransportVersionUtils.getPreviousVersion(TransportVersion.MINIMUM_COMPATIBLE);
try {
InboundDecoder.checkVersionCompatibility(invalid, version);
InboundDecoder.checkVersionCompatibility(invalid);
fail();
} catch (IllegalStateException expected) {
assertEquals(
"Received message from unsupported version: [" + invalid + "] minimal compatible version is: [" + version + "]",
"Received message from unsupported version: ["
+ invalid
+ "] minimal compatible version is: ["
+ TransportVersion.MINIMUM_COMPATIBLE
+ "]",
expected.getMessage()
);
}
Expand Down

0 comments on commit 3053b17

Please sign in to comment.