From 6d818506b879a36b25f3af64df4ce3ad8d2bf3af Mon Sep 17 00:00:00 2001 From: Nick Knize Date: Mon, 31 Jul 2023 12:53:38 -0500 Subject: [PATCH 1/3] [Remove] Deprecated Fractional ByteSizeValue support (#9005) This commit removes the deprecation warning for fractional bytes size values and, instead, throws an OpenSearchParseException if a user passes in a fraction byte value (e.g., 5.2b).. This leniency was deprecated a long time ago in Legacy 6.2 so the removal should come as no surprise to users. Signed-off-by: Nicholas Walter Knize (cherry picked from commit 0b1f875d34c7d723afd41396234f8f2003c6be2c) --- CHANGELOG.md | 4 +++- .../ingest/common/BytesProcessorTests.java | 15 ++++++------ .../opensearch/common/unit/ByteSizeValue.java | 24 ++++--------------- .../common/unit/ByteSizeValueTests.java | 10 ++++---- 4 files changed, 20 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bd92b8b184bec..19286297c0cdc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Bump `org.gradle.test-retry` from 1.5.3 to 1.5.4 ([#8842](https://github.com/opensearch-project/OpenSearch/pull/8842)) - Bump `com.netflix.nebula.ospackage-base` from 11.3.0 to 11.4.0 ([#8838](https://github.com/opensearch-project/OpenSearch/pull/8838)) - Bump `com.google.http-client:google-http-client-gson` from 1.43.2 to 1.43.3 ([#8840](https://github.com/opensearch-project/OpenSearch/pull/8840)) -- OpenJDK Update (July 2023 Patch releases) ([#8869](https://github.com/opensearch-project/OpenSearch/pull/8869) +- OpenJDK Update (July 2023 Patch releases) ([#8869](https://github.com/opensearch-project/OpenSearch/pull/8869)) - Bump `hadoop` libraries from 3.3.4 to 3.3.6 ([#6995](https://github.com/opensearch-project/OpenSearch/pull/6995)) - Bump `com.gradle.enterprise` from 3.13.3 to 3.14.1 ([#8996](https://github.com/opensearch-project/OpenSearch/pull/8996)) - Bump `org.apache.commons:commons-lang3` from 3.12.0 to 3.13.0 ([#8995](https://github.com/opensearch-project/OpenSearch/pull/8995)) @@ -37,6 +37,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Replace the deprecated IndexReader APIs with new storedFields() & termVectors() ([#7792](https://github.com/opensearch-project/OpenSearch/pull/7792)) - [Remote Store] Add support to restore only unassigned shards of an index ([#8792](https://github.com/opensearch-project/OpenSearch/pull/8792)) - Add safeguard limits for file cache during node level allocation ([#8208](https://github.com/opensearch-project/OpenSearch/pull/8208)) +- Add support for aggregation profiler with concurrent aggregation ([#8801](https://github.com/opensearch-project/OpenSearch/pull/8801)) +- [Remove] Deprecated Fractional ByteSizeValue support #9005 ([#9005](https://github.com/opensearch-project/OpenSearch/pull/9005)) ### Deprecated diff --git a/modules/ingest-common/src/test/java/org/opensearch/ingest/common/BytesProcessorTests.java b/modules/ingest-common/src/test/java/org/opensearch/ingest/common/BytesProcessorTests.java index bbd9ff4c8b912..385d77418ee7b 100644 --- a/modules/ingest-common/src/test/java/org/opensearch/ingest/common/BytesProcessorTests.java +++ b/modules/ingest-common/src/test/java/org/opensearch/ingest/common/BytesProcessorTests.java @@ -33,6 +33,7 @@ package org.opensearch.ingest.common; import org.opensearch.OpenSearchException; +import org.opensearch.OpenSearchParseException; import org.opensearch.common.unit.ByteSizeUnit; import org.opensearch.common.unit.ByteSizeValue; import org.opensearch.ingest.IngestDocument; @@ -40,8 +41,6 @@ import org.opensearch.ingest.RandomDocumentPicks; import org.hamcrest.CoreMatchers; -import static org.hamcrest.Matchers.equalTo; - public class BytesProcessorTests extends AbstractStringProcessorTestCase { private String modifiedInput; @@ -101,14 +100,16 @@ public void testMissingUnits() { assertThat(exception.getMessage(), CoreMatchers.containsString("unit is missing or unrecognized")); } - public void testFractional() throws Exception { + public void testFractional() { IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random()); String fieldName = RandomDocumentPicks.addRandomField(random(), ingestDocument, "1.1kb"); Processor processor = newProcessor(fieldName, randomBoolean(), fieldName); - processor.execute(ingestDocument); - assertThat(ingestDocument.getFieldValue(fieldName, expectedResultType()), equalTo(1126L)); - assertWarnings( - "Fractional bytes values are deprecated. Use non-fractional bytes values instead: [1.1kb] found for setting " + "[Ingest Field]" + OpenSearchParseException e = expectThrows(OpenSearchParseException.class, () -> processor.execute(ingestDocument)); + assertThat( + e.getMessage(), + CoreMatchers.containsString( + "Fractional bytes values have been deprecated since Legacy 6.2. " + "Use non-fractional bytes values instead:" + ) ); } } diff --git a/server/src/main/java/org/opensearch/common/unit/ByteSizeValue.java b/server/src/main/java/org/opensearch/common/unit/ByteSizeValue.java index a123c79464727..7343915a52c0c 100644 --- a/server/src/main/java/org/opensearch/common/unit/ByteSizeValue.java +++ b/server/src/main/java/org/opensearch/common/unit/ByteSizeValue.java @@ -37,9 +37,6 @@ import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; import org.opensearch.core.common.io.stream.Writeable; -import org.opensearch.common.logging.DeprecationLogger; -import org.opensearch.common.logging.LogConfigurator; -import org.opensearch.common.network.NetworkService; import org.opensearch.core.xcontent.ToXContentFragment; import org.opensearch.core.xcontent.XContentBuilder; @@ -54,17 +51,6 @@ */ public class ByteSizeValue implements Writeable, Comparable, ToXContentFragment { - /** - * We have to lazy initialize the deprecation logger as otherwise a static logger here would be constructed before logging is configured - * leading to a runtime failure (see {@link LogConfigurator#checkErrorListener()} ). The premature construction would come from any - * {@link ByteSizeValue} object constructed in, for example, settings in {@link NetworkService}. - * - * @opensearch.internal - */ - static class DeprecationLoggerHolder { - static DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(ByteSizeValue.class); - } - public static final ByteSizeValue ZERO = new ByteSizeValue(0, ByteSizeUnit.BYTES); private final long size; @@ -262,14 +248,14 @@ private static ByteSizeValue parse( return new ByteSizeValue(Long.parseLong(s), unit); } catch (final NumberFormatException e) { try { - final double doubleValue = Double.parseDouble(s); - DeprecationLoggerHolder.deprecationLogger.deprecate( - "fractional_byte_values", - "Fractional bytes values are deprecated. Use non-fractional bytes values instead: [{}] found for setting [{}]", + Double.parseDouble(s); + throw new OpenSearchParseException( + "Failed to parse bytes value [{}]. Fractional bytes values have been " + + "deprecated since Legacy 6.2. Use non-fractional bytes values instead: found for setting [{}]", + e, initialInput, settingName ); - return new ByteSizeValue((long) (doubleValue * unit.toBytes(1))); } catch (final NumberFormatException ignored) { throw new OpenSearchParseException("failed to parse [{}]", e, initialInput); } diff --git a/server/src/test/java/org/opensearch/common/unit/ByteSizeValueTests.java b/server/src/test/java/org/opensearch/common/unit/ByteSizeValueTests.java index 99c1feb78527f..7f6f753209a61 100644 --- a/server/src/test/java/org/opensearch/common/unit/ByteSizeValueTests.java +++ b/server/src/test/java/org/opensearch/common/unit/ByteSizeValueTests.java @@ -336,12 +336,10 @@ public void testParseInvalidNumber() throws IOException { public void testParseFractionalNumber() throws IOException { ByteSizeUnit unit = randomValueOtherThan(ByteSizeUnit.BYTES, () -> randomFrom(ByteSizeUnit.values())); String fractionalValue = "23.5" + unit.getSuffix(); - ByteSizeValue instance = ByteSizeValue.parseBytesSizeValue(fractionalValue, "test"); - assertEquals(fractionalValue, instance.toString()); - assertWarnings( - "Fractional bytes values are deprecated. Use non-fractional bytes values instead: [" - + fractionalValue - + "] found for setting [test]" + // test exception is thrown: fractional byte size values has been deprecated since Legacy 6.2 + OpenSearchParseException e = expectThrows( + OpenSearchParseException.class, + () -> ByteSizeValue.parseBytesSizeValue(fractionalValue, "test") ); } From 57f01c7e2e6277fd2f9ed7e76f7f1e407126ba25 Mon Sep 17 00:00:00 2001 From: Nicholas Walter Knize Date: Mon, 31 Jul 2023 14:13:45 -0500 Subject: [PATCH 2/3] [Bug] Fix Setting#byteSizeSetting to return whole value string This fixes a bug where the Setting#byteSizeSetting static method returned fractional byte values when calling .toString inside the lambda. The change is to directly return the bytes value as a string instead of using ByteSizeValue#toString which tries to be smart and by pretty printing in a more human-friendly format. Signed-off-by: Nicholas Walter Knize --- .../src/main/java/org/opensearch/common/settings/Setting.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/common/settings/Setting.java b/server/src/main/java/org/opensearch/common/settings/Setting.java index 3bf2988e88e5a..2544539453f77 100644 --- a/server/src/main/java/org/opensearch/common/settings/Setting.java +++ b/server/src/main/java/org/opensearch/common/settings/Setting.java @@ -2047,7 +2047,7 @@ static boolean parseBoolean(String b, String key, boolean isFiltered) { } public static Setting byteSizeSetting(String key, ByteSizeValue value, Property... properties) { - return byteSizeSetting(key, (s) -> value.toString(), properties); + return byteSizeSetting(key, (s) -> value.getBytes() + "b", properties); } public static Setting byteSizeSetting(String key, Setting fallbackSetting, Property... properties) { From 03772f7bd1d318eae35bfa4f9f6941c1cf32554b Mon Sep 17 00:00:00 2001 From: Nicholas Walter Knize Date: Mon, 31 Jul 2023 14:28:35 -0500 Subject: [PATCH 3/3] switch from string literal to static units Signed-off-by: Nicholas Walter Knize --- .../src/main/java/org/opensearch/common/settings/Setting.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/common/settings/Setting.java b/server/src/main/java/org/opensearch/common/settings/Setting.java index 2544539453f77..86d1d4f90ed18 100644 --- a/server/src/main/java/org/opensearch/common/settings/Setting.java +++ b/server/src/main/java/org/opensearch/common/settings/Setting.java @@ -40,6 +40,7 @@ import org.opensearch.common.Nullable; import org.opensearch.common.Strings; import org.opensearch.common.collect.Tuple; +import org.opensearch.common.unit.ByteSizeUnit; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; import org.opensearch.core.common.io.stream.Writeable; @@ -2047,7 +2048,7 @@ static boolean parseBoolean(String b, String key, boolean isFiltered) { } public static Setting byteSizeSetting(String key, ByteSizeValue value, Property... properties) { - return byteSizeSetting(key, (s) -> value.getBytes() + "b", properties); + return byteSizeSetting(key, (s) -> value.getBytes() + ByteSizeUnit.BYTES.getSuffix(), properties); } public static Setting byteSizeSetting(String key, Setting fallbackSetting, Property... properties) {