>From Michael Blow <[email protected]>: Michael Blow has submitted this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21331?usp=email )
Change subject: [NO ISSUE][*DB][EXT] Replace instances of 'sdk_default' with null ...................................................................... [NO ISSUE][*DB][EXT] Replace instances of 'sdk_default' with null Ext-ref: MB-72281,MB-71732 Change-Id: I2c518522aec7e990baea64eca0514e38aba0a0b2 Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21331 Integration-Tests: Jenkins <[email protected]> Tested-by: Jenkins <[email protected]> Reviewed-by: Michael Blow <[email protected]> Reviewed-by: Hussain Towaileb <[email protected]> --- M asterixdb/asterix-app/src/test/resources/runtimets/results/api/cluster_state_1/cluster_state_1.1.regexadm M asterixdb/asterix-app/src/test/resources/runtimets/results/api/cluster_state_1_full/cluster_state_1_full.1.regexadm M asterixdb/asterix-app/src/test/resources/runtimets/results/api/cluster_state_1_less/cluster_state_1_less.1.regexadm M asterixdb/asterix-app/src/test/resources/runtimets/testsuite_external_dataset_s3.xml M asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/aws/s3/S3ClientConfig.java M asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/CloudProperties.java M asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/aws/s3/S3Constants.java M asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/aws/s3/S3Utils.java M hyracks-fullstack/hyracks/hyracks-cloud/src/main/java/org/apache/hyracks/cloud/io/S3ChecksumBehavior.java M hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/NetworkUtil.java 10 files changed, 50 insertions(+), 44 deletions(-) Approvals: Jenkins: Verified; Verified Hussain Towaileb: Looks good to me, approved Michael Blow: Looks good to me, but someone else must approve diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/api/cluster_state_1/cluster_state_1.1.regexadm b/asterixdb/asterix-app/src/test/resources/runtimets/results/api/cluster_state_1/cluster_state_1.1.regexadm index c35a1df..fff73de 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/results/api/cluster_state_1/cluster_state_1.1.regexadm +++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/api/cluster_state_1/cluster_state_1.1.regexadm @@ -40,7 +40,7 @@ "cloud.storage.prefix" : "", "cloud.storage.region" : "", "cloud.storage.s3.access.key.id" : null, - "cloud.storage.s3.checksum.behavior" : "sdk_default", + "cloud.storage.s3.checksum.behavior" : null, "cloud.storage.s3.client.read.timeout" : -1, "cloud.storage.s3.parallel.downloader.client.type" : "crt", "cloud.storage.s3.secret.access.key" : null, diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/api/cluster_state_1_full/cluster_state_1_full.1.regexadm b/asterixdb/asterix-app/src/test/resources/runtimets/results/api/cluster_state_1_full/cluster_state_1_full.1.regexadm index 95a20a9..58ae04d 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/results/api/cluster_state_1_full/cluster_state_1_full.1.regexadm +++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/api/cluster_state_1_full/cluster_state_1_full.1.regexadm @@ -40,7 +40,7 @@ "cloud.storage.prefix" : "", "cloud.storage.region" : "", "cloud.storage.s3.access.key.id" : null, - "cloud.storage.s3.checksum.behavior" : "sdk_default", + "cloud.storage.s3.checksum.behavior" : null, "cloud.storage.s3.client.read.timeout" : -1, "cloud.storage.s3.parallel.downloader.client.type" : "crt", "cloud.storage.s3.secret.access.key" : null, diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/api/cluster_state_1_less/cluster_state_1_less.1.regexadm b/asterixdb/asterix-app/src/test/resources/runtimets/results/api/cluster_state_1_less/cluster_state_1_less.1.regexadm index 6e23922..13f7b43 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/results/api/cluster_state_1_less/cluster_state_1_less.1.regexadm +++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/api/cluster_state_1_less/cluster_state_1_less.1.regexadm @@ -40,7 +40,7 @@ "cloud.storage.prefix" : "", "cloud.storage.region" : "", "cloud.storage.s3.access.key.id" : null, - "cloud.storage.s3.checksum.behavior" : "sdk_default", + "cloud.storage.s3.checksum.behavior" : null, "cloud.storage.s3.client.read.timeout" : -1, "cloud.storage.s3.parallel.downloader.client.type" : "crt", "cloud.storage.s3.secret.access.key" : null, diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_external_dataset_s3.xml b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_external_dataset_s3.xml index f8ebbbe..d19d5fc 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_external_dataset_s3.xml +++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_external_dataset_s3.xml @@ -1297,7 +1297,7 @@ <test-case FilePath="external-dataset/s3"> <compilation-unit name="checksum-behavior/invalid-value"> <output-dir compare="Text">checksum-behavior/invalid-value</output-dir> - <expected-error>ASX1173: Invalid value for parameter 'checksumBehavior', allowed value(s): when_required, when_supported, sdk_default</expected-error> + <expected-error>ASX1173: Invalid value for parameter 'checksumBehavior', allowed value(s): when_required, when_supported</expected-error> </compilation-unit> </test-case> <test-case FilePath="external-dataset/s3"> diff --git a/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/aws/s3/S3ClientConfig.java b/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/aws/s3/S3ClientConfig.java index 2e9caea..3bc4e62 100644 --- a/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/aws/s3/S3ClientConfig.java +++ b/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/aws/s3/S3ClientConfig.java @@ -65,8 +65,7 @@ Collection<String> certificates, long profilerLogInterval, int writeBufferSize, S3ParallelDownloaderClientType parallelDownloaderClientType, boolean roundRobinDnsResolver) { this(region, endpoint, prefix, anonymousAuth, certificates, profilerLogInterval, writeBufferSize, 1, 0, 0, 0, - false, false, 0, 0, 0, 0, -1, parallelDownloaderClientType, roundRobinDnsResolver, "", "", - S3ChecksumBehavior.SDK_DEFAULT); + false, false, 0, 0, 0, 0, -1, parallelDownloaderClientType, roundRobinDnsResolver, "", "", null); } private S3ClientConfig(String region, String endpoint, String prefix, boolean anonymousAuth, @@ -99,7 +98,7 @@ this.roundRobinDnsResolver = roundRobinDnsResolver; this.accessKeyId = accessKeyId; this.secretAccessKey = secretAccessKey; - this.checksumBehavior = Objects.requireNonNull(checksumBehavior, "checksumBehavior"); + this.checksumBehavior = checksumBehavior; } public static S3ClientConfig of(ICloudProperties cloudProperties) { diff --git a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/CloudProperties.java b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/CloudProperties.java index 1fb9901..d86f654 100644 --- a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/CloudProperties.java +++ b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/CloudProperties.java @@ -102,7 +102,7 @@ CLOUD_STORAGE_S3_ACCESS_KEY_ID(STRING, (String) null), CLOUD_STORAGE_S3_SECRET_ACCESS_KEY(STRING, (String) null), CLOUD_STORAGE_AZURE_CLIENT_ID(STRING, (String) null), - CLOUD_STORAGE_S3_CHECKSUM_BEHAVIOR(STRING, S3ChecksumBehavior.SDK_DEFAULT.stringValue()); + CLOUD_STORAGE_S3_CHECKSUM_BEHAVIOR(STRING, (String) null); private final IOptionType interpreter; private final Object defaultValue; @@ -246,8 +246,8 @@ case CLOUD_STORAGE_S3_CHECKSUM_BEHAVIOR: return "The checksum behavior for S3 requests and responses. Accepted values: " + "'when_required' (only checksums mandated by the operation), " - + "'when_supported' (checksums on all eligible operations, SDK >= 2.30 default), " - + "'sdk_default' (defer to SDK default. " + "Defaults to 'sdk_default'"; + + "'when_supported' (checksums on all eligible operations, SDK >= 2.30 default). " + + "Defaults to SDK default (null/empty)"; default: throw new IllegalStateException("NYI: " + this); } diff --git a/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/aws/s3/S3Constants.java b/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/aws/s3/S3Constants.java index c458dc3..4f8602d 100644 --- a/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/aws/s3/S3Constants.java +++ b/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/aws/s3/S3Constants.java @@ -49,9 +49,6 @@ public static final String HADOOP_SERVICE_END_POINT = "fs.s3a.endpoint"; public static final String HADOOP_REGION = "fs.s3a.endpoint.region"; - // value to indicate we should use whatever the SDK defaults to - public static final String SDK_DEFAULT = "sdk_default"; - // input stream type public static final String INPUT_STREAM_TYPE_FIELD_NAME = "inputStreamType"; public static final String HADOOP_INPUT_STREAM_TYPE = "fs.s3a.input.stream.type"; diff --git a/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/aws/s3/S3Utils.java b/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/aws/s3/S3Utils.java index 7eb227c..212bbea 100644 --- a/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/aws/s3/S3Utils.java +++ b/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/aws/s3/S3Utils.java @@ -74,7 +74,6 @@ import static org.apache.asterix.external.util.aws.s3.S3Constants.HADOOP_TEMPORARY; import static org.apache.asterix.external.util.aws.s3.S3Constants.INPUT_STREAM_TYPE_FIELD_NAME; import static org.apache.asterix.external.util.aws.s3.S3Constants.PATH_STYLE_ADDRESSING_FIELD_NAME; -import static org.apache.asterix.external.util.aws.s3.S3Constants.SDK_DEFAULT; import static org.apache.hyracks.api.util.ExceptionUtils.getMessageOrToString; import static org.apache.hyracks.util.annotations.AiProvenance.Agent.CLAUDE_SONNET_4_6; import static org.apache.hyracks.util.annotations.AiProvenance.Tool.GITHUB_COPILOT; @@ -91,7 +90,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.UUID; import java.util.function.BiPredicate; import java.util.regex.Matcher; @@ -208,7 +206,10 @@ } public static void applyChecksumBehavior(S3ClientBuilder builder, S3ChecksumBehavior behavior) { - Objects.requireNonNull(behavior, "checksumBehavior"); + if (behavior == null) { + // null means use SDK defaults — don't configure anything + return; + } switch (behavior) { case WHEN_REQUIRED: builder.requestChecksumCalculation(RequestChecksumCalculation.WHEN_REQUIRED); @@ -218,9 +219,6 @@ builder.requestChecksumCalculation(RequestChecksumCalculation.WHEN_SUPPORTED); builder.responseChecksumValidation(ResponseChecksumValidation.WHEN_SUPPORTED); break; - case SDK_DEFAULT: - // leave SDK defaults untouched - break; } } @@ -326,14 +324,14 @@ private static void setInputStreamType(Map<String, String> configuration, JobConf jobConf) { String configuredInputStreamType = configuration.get(INPUT_STREAM_TYPE_FIELD_NAME); - if (!Objects.equals(configuredInputStreamType, SDK_DEFAULT)) { + if (configuredInputStreamType != null) { jobConf.set(S3Constants.HADOOP_INPUT_STREAM_TYPE, configuredInputStreamType); } } private static void setChangeDetectionMode(Map<String, String> configuration, JobConf jobConf) { String configuredChangeDetectionMode = configuration.get(CHANGE_DETECTION_MODE_FIELD_NAME); - if (!Objects.equals(configuredChangeDetectionMode, SDK_DEFAULT)) { + if (configuredChangeDetectionMode != null) { jobConf.set(S3Constants.HADOOP_CHANGE_DETECTION_MODE, configuredChangeDetectionMode); } } @@ -760,7 +758,7 @@ * Resolves the effective checksum behavior for external data (link) operations using a two-level priority chain: * <ol> * <li>Per-link {@code checksumBehavior} parameter in the configuration map (explicit override)</li> - * <li>Endpoint-based default: {@link S3ChecksumBehavior#SDK_DEFAULT} for native AWS S3, + * <li>Endpoint-based default: {@code null} (use SDK defaults) for native AWS S3, * {@link S3ChecksumBehavior#WHEN_REQUIRED} for S3-compatible storage</li> * </ol> * Cloud properties are intentionally not consulted here — this method is for external data (link) operations @@ -778,8 +776,8 @@ CHECKSUM_BEHAVIOR_ALLOWED_VALUES); } } - // TODO(mblow): I don't believe this code is reachable any longer (we will always have a checksum behavior configured) - return S3ChecksumBehavior.SDK_DEFAULT; + // null means use SDK defaults + return null; } /** @@ -821,15 +819,15 @@ throws CompilationException { String streamInputType = configuration.get(INPUT_STREAM_TYPE_FIELD_NAME); if (streamInputType == null || streamInputType.isBlank()) { - streamInputType = SDK_DEFAULT; - configuration.put(INPUT_STREAM_TYPE_FIELD_NAME, streamInputType); + // null/empty means use SDK defaults — nothing to validate or set + configuration.remove(INPUT_STREAM_TYPE_FIELD_NAME); + return; } - if (!SDK_DEFAULT.equalsIgnoreCase(streamInputType) - && !HADOOP_INPUT_STREAM_TYPE_VAL_ANALYTICS.equalsIgnoreCase(streamInputType) + if (!HADOOP_INPUT_STREAM_TYPE_VAL_ANALYTICS.equalsIgnoreCase(streamInputType) && !HADOOP_INPUT_STREAM_TYPE_VAL_CLASSIC.equalsIgnoreCase(streamInputType)) { - throw new CompilationException(INVALID_PARAM_VALUE_ALLOWED_VALUE, INPUT_STREAM_TYPE_FIELD_NAME, SDK_DEFAULT - + ", " + HADOOP_INPUT_STREAM_TYPE_VAL_ANALYTICS + ", " + HADOOP_INPUT_STREAM_TYPE_VAL_CLASSIC); + throw new CompilationException(INVALID_PARAM_VALUE_ALLOWED_VALUE, INPUT_STREAM_TYPE_FIELD_NAME, + HADOOP_INPUT_STREAM_TYPE_VAL_ANALYTICS + ", " + HADOOP_INPUT_STREAM_TYPE_VAL_CLASSIC); } configuration.put(INPUT_STREAM_TYPE_FIELD_NAME, streamInputType.toLowerCase()); } @@ -838,18 +836,17 @@ throws CompilationException { String changeDetectionMode = configuration.get(CHANGE_DETECTION_MODE_FIELD_NAME); if (changeDetectionMode == null || changeDetectionMode.isBlank()) { - changeDetectionMode = SDK_DEFAULT; - configuration.put(CHANGE_DETECTION_MODE_FIELD_NAME, changeDetectionMode); + // null/empty means use SDK defaults — nothing to validate or set + configuration.remove(CHANGE_DETECTION_MODE_FIELD_NAME); return; } - if (!SDK_DEFAULT.equalsIgnoreCase(changeDetectionMode) - && !HADOOP_CHANGE_DETECTION_MODE_VAL_NONE.equalsIgnoreCase(changeDetectionMode) + if (!HADOOP_CHANGE_DETECTION_MODE_VAL_NONE.equalsIgnoreCase(changeDetectionMode) && !HADOOP_CHANGE_DETECTION_MODE_VAL_CLIENT.equalsIgnoreCase(changeDetectionMode) && !HADOOP_CHANGE_DETECTION_MODE_VAL_SERVER.equalsIgnoreCase(changeDetectionMode)) { throw new CompilationException(INVALID_PARAM_VALUE_ALLOWED_VALUE, CHANGE_DETECTION_MODE_FIELD_NAME, - SDK_DEFAULT + ", " + HADOOP_CHANGE_DETECTION_MODE_VAL_NONE + ", " - + HADOOP_CHANGE_DETECTION_MODE_VAL_CLIENT + ", " + HADOOP_CHANGE_DETECTION_MODE_VAL_SERVER); + HADOOP_CHANGE_DETECTION_MODE_VAL_NONE + ", " + HADOOP_CHANGE_DETECTION_MODE_VAL_CLIENT + ", " + + HADOOP_CHANGE_DETECTION_MODE_VAL_SERVER); } configuration.put(CHANGE_DETECTION_MODE_FIELD_NAME, changeDetectionMode.toLowerCase()); } diff --git a/hyracks-fullstack/hyracks/hyracks-cloud/src/main/java/org/apache/hyracks/cloud/io/S3ChecksumBehavior.java b/hyracks-fullstack/hyracks/hyracks-cloud/src/main/java/org/apache/hyracks/cloud/io/S3ChecksumBehavior.java index 63969aa..7f3c294 100644 --- a/hyracks-fullstack/hyracks/hyracks-cloud/src/main/java/org/apache/hyracks/cloud/io/S3ChecksumBehavior.java +++ b/hyracks-fullstack/hyracks/hyracks-cloud/src/main/java/org/apache/hyracks/cloud/io/S3ChecksumBehavior.java @@ -34,26 +34,24 @@ /** Calculate/validate checksums only when required by the operation. Safe for S3-compatible endpoints. */ WHEN_REQUIRED, /** Calculate/validate checksums whenever supported — the SDK default since 2.30.0. */ - WHEN_SUPPORTED, - /** Leave the SDK defaults untouched. Appropriate for native AWS S3. */ - SDK_DEFAULT; + WHEN_SUPPORTED; public String stringValue() { return name().toLowerCase(); } - /** Parses the config string (case-insensitive). Returns {@code SDK_DEFAULT} if the input is {@code null}. */ + /** Parses the config string (case-insensitive). Returns {@code null} if the input is {@code null} or empty. */ public static S3ChecksumBehavior fromString(String s) { - if (s == null) { - return SDK_DEFAULT; + if (s == null || s.isEmpty()) { + return null; } for (S3ChecksumBehavior b : values()) { if (b.name().equalsIgnoreCase(s)) { return b; } } - throw new IllegalArgumentException("Unrecognized S3 checksum behavior: '" + s - + "'. Valid values: when_required, when_supported, sdk_default"); + throw new IllegalArgumentException( + "Unrecognized S3 checksum behavior: '" + s + "'. Valid values: when_required, when_supported"); } } diff --git a/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/NetworkUtil.java b/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/NetworkUtil.java index 73a9243..1533f65 100644 --- a/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/NetworkUtil.java +++ b/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/NetworkUtil.java @@ -40,6 +40,8 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import com.google.common.net.InetAddresses; + public class NetworkUtil { private static final Logger LOGGER = LogManager.getLogger(); @@ -234,4 +236,17 @@ public static InetSocketAddress toInetSocketAddress(HttpHost httpHost) { return new InetSocketAddress(httpHost.getAddress(), httpHost.getPort()); } + + /** + * Returns {@code true} if the given hostname string is an IP address literal (IPv4 or IPv6), + * rather than a DNS name. + */ + public static boolean isIpLiteral(String host) { + if (host == null || host.isEmpty()) { + return false; + } + // Strip IPv6 brackets if present + String bare = host.charAt(0) == '[' ? host.substring(1, host.length() - 1) : host; + return InetAddresses.isInetAddress(bare); + } } -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21331?usp=email To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: asterixdb Gerrit-Branch: lumina Gerrit-Change-Id: I2c518522aec7e990baea64eca0514e38aba0a0b2 Gerrit-Change-Number: 21331 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Blow <[email protected]> Gerrit-Reviewer: Hussain Towaileb <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-CC: Anon. E. Moose #1000171
