From 29f9dbd0cc8765f765f79256548d52f864477653 Mon Sep 17 00:00:00 2001 From: Patrick Doyle Date: Thu, 20 Feb 2025 13:45:42 -0500 Subject: [PATCH 1/2] Reinstate ByteSizeValue transport changes --- .../org/elasticsearch/TransportVersions.java | 1 + .../common/unit/ByteSizeValue.java | 17 ++++-- .../common/unit/ByteSizeValueTests.java | 59 ++++++++++--------- 3 files changed, 44 insertions(+), 33 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index 364c624813da6..5206666e08e83 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -182,6 +182,7 @@ static TransportVersion def(int id) { public static final TransportVersion REMOVE_DESIRED_NODE_VERSION_90 = def(9_000_0_03); public static final TransportVersion ESQL_DRIVER_TASK_DESCRIPTION_90 = def(9_000_0_04); public static final TransportVersion REMOVE_ALL_APPLICABLE_SELECTOR_9_0 = def(9_000_0_05); + public static final TransportVersion BYTE_SIZE_VALUE_ALWAYS_USES_BYTES_90 = def(9_000_0_06); /* * STOP! READ THIS FIRST! No, really, diff --git a/server/src/main/java/org/elasticsearch/common/unit/ByteSizeValue.java b/server/src/main/java/org/elasticsearch/common/unit/ByteSizeValue.java index 23d76abdec2f2..17f390b57c3b5 100644 --- a/server/src/main/java/org/elasticsearch/common/unit/ByteSizeValue.java +++ b/server/src/main/java/org/elasticsearch/common/unit/ByteSizeValue.java @@ -26,6 +26,7 @@ import java.util.Objects; import static org.elasticsearch.TransportVersions.BYTE_SIZE_VALUE_ALWAYS_USES_BYTES_1; +import static org.elasticsearch.TransportVersions.BYTE_SIZE_VALUE_ALWAYS_USES_BYTES_90; import static org.elasticsearch.TransportVersions.REVERT_BYTE_SIZE_VALUE_ALWAYS_USES_BYTES_1; import static org.elasticsearch.common.unit.ByteSizeUnit.BYTES; import static org.elasticsearch.common.unit.ByteSizeUnit.GB; @@ -113,8 +114,7 @@ static ByteSizeValue newByteSizeValue(long sizeInBytes, ByteSizeUnit desiredUnit public static ByteSizeValue readFrom(StreamInput in) throws IOException { long size = in.readZLong(); ByteSizeUnit unit = ByteSizeUnit.readFrom(in); - TransportVersion tv = in.getTransportVersion(); - if (tv.onOrAfter(BYTE_SIZE_VALUE_ALWAYS_USES_BYTES_1) && tv.before(REVERT_BYTE_SIZE_VALUE_ALWAYS_USES_BYTES_1)) { + if (alwaysUseBytes(in.getTransportVersion())) { return newByteSizeValue(size, unit); } else { return of(size, unit); @@ -123,8 +123,7 @@ public static ByteSizeValue readFrom(StreamInput in) throws IOException { @Override public void writeTo(StreamOutput out) throws IOException { - TransportVersion tv = out.getTransportVersion(); - if (tv.onOrAfter(BYTE_SIZE_VALUE_ALWAYS_USES_BYTES_1) && tv.before(REVERT_BYTE_SIZE_VALUE_ALWAYS_USES_BYTES_1)) { + if (alwaysUseBytes(out.getTransportVersion())) { out.writeZLong(sizeInBytes); } else { out.writeZLong(Math.divideExact(sizeInBytes, desiredUnit.toBytes(1))); @@ -132,6 +131,16 @@ public void writeTo(StreamOutput out) throws IOException { desiredUnit.writeTo(out); } + private static boolean alwaysUseBytes(TransportVersion tv) { + if (tv.between(BYTE_SIZE_VALUE_ALWAYS_USES_BYTES_1, REVERT_BYTE_SIZE_VALUE_ALWAYS_USES_BYTES_1)) { + return true; + } else if (tv.isPatchFrom(BYTE_SIZE_VALUE_ALWAYS_USES_BYTES_90)) { + return true; + } else { + return false; + } + } + ByteSizeValue(long sizeInBytes, ByteSizeUnit desiredUnit) { this.sizeInBytes = sizeInBytes; this.desiredUnit = desiredUnit; diff --git a/server/src/test/java/org/elasticsearch/common/unit/ByteSizeValueTests.java b/server/src/test/java/org/elasticsearch/common/unit/ByteSizeValueTests.java index 79b06926cdef6..6fee41a757db2 100644 --- a/server/src/test/java/org/elasticsearch/common/unit/ByteSizeValueTests.java +++ b/server/src/test/java/org/elasticsearch/common/unit/ByteSizeValueTests.java @@ -11,7 +11,6 @@ import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.TransportVersion; -import org.elasticsearch.TransportVersions; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.Writeable.Reader; import org.elasticsearch.test.AbstractWireSerializingTestCase; @@ -22,6 +21,9 @@ import java.util.List; import java.util.function.Function; +import static org.elasticsearch.TransportVersions.BYTE_SIZE_VALUE_ALWAYS_USES_BYTES_90; +import static org.elasticsearch.TransportVersions.INITIAL_ELASTICSEARCH_9_0; +import static org.elasticsearch.TransportVersions.V_8_16_0; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; @@ -520,44 +522,43 @@ protected void assertEqualInstances(ByteSizeValue expectedInstance, ByteSizeValu public void testBWCTransportFormat() throws IOException { var tenMegs = ByteSizeValue.ofMb(10); - try (BytesStreamOutput expected = new BytesStreamOutput(); BytesStreamOutput actual = new BytesStreamOutput()) { - expected.writeZLong(10); - ByteSizeUnit.MB.writeTo(expected); - actual.setTransportVersion(TransportVersions.V_8_16_0); - tenMegs.writeTo(actual); - assertArrayEquals( - "Size denominated in the desired unit for backward compatibility", - expected.bytes().array(), - actual.bytes().array() - ); + for (var tv : List.of(V_8_16_0, INITIAL_ELASTICSEARCH_9_0)) { + try (BytesStreamOutput expected = new BytesStreamOutput(); BytesStreamOutput actual = new BytesStreamOutput()) { + expected.writeZLong(10); + ByteSizeUnit.MB.writeTo(expected); + actual.setTransportVersion(tv); + tenMegs.writeTo(actual); + assertArrayEquals( + "Size denominated in the desired unit for backward compatibility", + expected.bytes().array(), + actual.bytes().array() + ); + } } } - /** - * @see TransportVersions#REVERT_BYTE_SIZE_VALUE_ALWAYS_USES_BYTES_1 - */ - @AwaitsFix(bugUrl = "https://elasticco.atlassian.net/browse/ES-10585") public void testTwoDigitTransportRoundTrips() throws IOException { - TransportVersion tv = TransportVersion.current(); - for (var desiredUnit : ByteSizeUnit.values()) { - if (desiredUnit == ByteSizeUnit.BYTES) { - continue; - } - checkTransportRoundTrip(ByteSizeValue.parseBytesSizeValue("23" + desiredUnit.getSuffix(), "test"), tv); - for (int tenths = 1; tenths <= 9; tenths++) { - checkTransportRoundTrip(ByteSizeValue.parseBytesSizeValue("23." + tenths + desiredUnit.getSuffix(), "test"), tv); - for (int hundredths = 1; hundredths <= 9; hundredths++) { - checkTransportRoundTrip( - ByteSizeValue.parseBytesSizeValue("23." + tenths + hundredths + desiredUnit.getSuffix(), "test"), - tv - ); + for (var tv : List.of(TransportVersion.current(), BYTE_SIZE_VALUE_ALWAYS_USES_BYTES_90)) { + for (var desiredUnit : ByteSizeUnit.values()) { + if (desiredUnit == ByteSizeUnit.BYTES) { + continue; + } + checkTransportRoundTrip(ByteSizeValue.parseBytesSizeValue("23" + desiredUnit.getSuffix(), "test"), tv); + for (int tenths = 1; tenths <= 9; tenths++) { + checkTransportRoundTrip(ByteSizeValue.parseBytesSizeValue("23." + tenths + desiredUnit.getSuffix(), "test"), tv); + for (int hundredths = 1; hundredths <= 9; hundredths++) { + checkTransportRoundTrip( + ByteSizeValue.parseBytesSizeValue("23." + tenths + hundredths + desiredUnit.getSuffix(), "test"), + tv + ); + } } } } } public void testIntegerTransportRoundTrips() throws IOException { - for (var tv : List.of(TransportVersion.current(), TransportVersions.V_8_16_0)) { + for (var tv : List.of(TransportVersion.current(), V_8_16_0)) { checkTransportRoundTrip(ByteSizeValue.ONE, tv); checkTransportRoundTrip(ByteSizeValue.ZERO, tv); checkTransportRoundTrip(ByteSizeValue.MINUS_ONE, tv); From e0ecc3c07ac93fca2b5c1cff0069182e6c9512a4 Mon Sep 17 00:00:00 2001 From: Patrick Doyle Date: Fri, 21 Feb 2025 07:46:05 -0500 Subject: [PATCH 2/2] Refactor alwaysUseBytes --- .../org/elasticsearch/common/unit/ByteSizeValue.java | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/unit/ByteSizeValue.java b/server/src/main/java/org/elasticsearch/common/unit/ByteSizeValue.java index 17f390b57c3b5..577df5bb10d45 100644 --- a/server/src/main/java/org/elasticsearch/common/unit/ByteSizeValue.java +++ b/server/src/main/java/org/elasticsearch/common/unit/ByteSizeValue.java @@ -132,13 +132,8 @@ public void writeTo(StreamOutput out) throws IOException { } private static boolean alwaysUseBytes(TransportVersion tv) { - if (tv.between(BYTE_SIZE_VALUE_ALWAYS_USES_BYTES_1, REVERT_BYTE_SIZE_VALUE_ALWAYS_USES_BYTES_1)) { - return true; - } else if (tv.isPatchFrom(BYTE_SIZE_VALUE_ALWAYS_USES_BYTES_90)) { - return true; - } else { - return false; - } + return tv.isPatchFrom(BYTE_SIZE_VALUE_ALWAYS_USES_BYTES_90) + || tv.between(BYTE_SIZE_VALUE_ALWAYS_USES_BYTES_1, REVERT_BYTE_SIZE_VALUE_ALWAYS_USES_BYTES_1); } ByteSizeValue(long sizeInBytes, ByteSizeUnit desiredUnit) {