This is an automated email from the ASF dual-hosted git repository. mhubail pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/asterixdb.git
commit 36feebe503a816e21b95fdc85491eafaffda1153 Author: Hussain Towaileb <[email protected]> AuthorDate: Fri Mar 28 05:19:50 2025 +0300 [NO ISSUE][EXT]: Fail early in COPY TO if writing issue was encountered - user model changes: no - storage format changes: no - interface changes: no Details: - fail early if writing issue was encountered. - report whatever issue encountered during writing correctly instead of only checking for not found bucket. - this also fixes cases where we assumed the issue encountered is a not found bucket, but it is something else Ext-ref: MB-66031 Change-Id: I954a60004f104aea4787075a94a069daca305fde Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19562 Reviewed-by: Michael Blow <[email protected]> Integration-Tests: Jenkins <[email protected]> Tested-by: Jenkins <[email protected]> --- .../bucket-does-not-exist/test.000.update.sqlpp | 36 ++++++++++++++++++++++ .../runtimets/testsuite_external_dataset_s3.xml | 11 +++++++ .../AbstractCloudExternalFileWriterFactory.java | 36 +++++++++++++--------- .../cloud/writer/GCSExternalFileWriterFactory.java | 15 +++------ .../cloud/writer/S3ExternalFileWriterFactory.java | 15 +++------ .../common/exceptions/CompilationException.java | 27 ++++++++++------ .../apache/hyracks/api/util/ExceptionUtils.java | 26 ++++++++++++++++ 7 files changed, 122 insertions(+), 44 deletions(-) diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/copy-to/negative/bucket-does-not-exist/test.000.update.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/copy-to/negative/bucket-does-not-exist/test.000.update.sqlpp new file mode 100644 index 0000000000..a4c4658fe7 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/copy-to/negative/bucket-does-not-exist/test.000.update.sqlpp @@ -0,0 +1,36 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +COPY ( +SELECT VALUE x FROM [ +{"id":1,"name":"Macbook1","amount":123.2,"accountNumber":345.34}, +{"id":3,"name":"Macbook3","amount":789.1,"accountNumber":678.9}, +{"id":4,"name":"Mac|,book4","amount":234.5,"accountNumber":567.89} +] AS x +) AS toWrite +TO %adapter% +PATH (%pathprefix% "copy-to-result", toWrite.id) +WITH { + %template_colons%, + %additionalProperties% + "format":"json" +} + + + 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 970ccdd405..38ebf01c97 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 @@ -187,6 +187,17 @@ <expected-warn>ASX0065: Length of the file path 'ford/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa [...] </compilation-unit> </test-case> + <test-case FilePath="copy-to/negative"> + <compilation-unit name="bucket-does-not-exist"> + <output-dir compare="Text">bucket-does-not-exist</output-dir> + <placeholder name="adapter" value="S3" /> + <placeholder name="pathprefix" value="" /> + <placeholder name="path_prefix" value="" /> + <placeholder name="additionalProperties" value='"container":"i-do-not-exist",' /> + <placeholder name="additional_Properties" value='("container"="i-do-not-exist")' /> + <expected-error>External sink error. software.amazon.awssdk.services.s3.model.NoSuchBucketException: The specified bucket does not exist</expected-error> + </compilation-unit> + </test-case> <test-case FilePath="copy-to/negative"> <compilation-unit name="non-empty-folder"> <output-dir compare="Text">non-empty-folder</output-dir> diff --git a/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/writer/AbstractCloudExternalFileWriterFactory.java b/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/writer/AbstractCloudExternalFileWriterFactory.java index 198b3ad81b..1de64d0031 100644 --- a/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/writer/AbstractCloudExternalFileWriterFactory.java +++ b/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/writer/AbstractCloudExternalFileWriterFactory.java @@ -24,6 +24,7 @@ import static org.apache.hyracks.api.util.ExceptionUtils.getMessageOrToString; import java.io.IOException; import java.util.Collections; import java.util.Map; +import java.util.Optional; import java.util.Random; import org.apache.asterix.cloud.IWriteBufferProvider; @@ -39,12 +40,11 @@ import org.apache.asterix.runtime.writer.IExternalFileWriterFactory; import org.apache.hyracks.algebricks.common.exceptions.AlgebricksException; import org.apache.hyracks.api.exceptions.HyracksDataException; import org.apache.hyracks.api.exceptions.SourceLocation; -import org.apache.hyracks.api.util.ExceptionUtils; import org.apache.hyracks.data.std.primitive.LongPointable; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -abstract class AbstractCloudExternalFileWriterFactory implements IExternalFileWriterFactory { +abstract class AbstractCloudExternalFileWriterFactory<T extends Throwable> implements IExternalFileWriterFactory { private static final long serialVersionUID = -6204498482419719403L; private static final Logger LOGGER = LogManager.getLogger(); @@ -63,9 +63,15 @@ abstract class AbstractCloudExternalFileWriterFactory implements IExternalFileWr abstract ICloudClient createCloudClient(IApplicationContext appCtx) throws CompilationException; - abstract boolean isNoContainerFoundException(IOException e); - - abstract boolean isSdkException(Throwable e); + /** + * Certain failures are wrapped in our exceptions as IO failures, this method checks if the original failure + * is reported from the external SDK, and if so, returns it. This is to ensure that the failure + * reported by the external SDK is the one reported back as the issue. + * + * @param ex failure thrown + * @return the throwable if it is an SDK exception, null otherwise + */ + abstract Optional<T> getSdkException(Throwable ex); final void buildClient(IApplicationContext appCtx) throws HyracksDataException { try { @@ -91,18 +97,17 @@ abstract class AbstractCloudExternalFileWriterFactory implements IExternalFileWr try { doValidate(testClient, bucket); - } catch (IOException e) { - if (isNoContainerFoundException(e)) { - throw CompilationException.create(ErrorCode.EXTERNAL_SOURCE_CONTAINER_NOT_FOUND, bucket); - } else { - throw CompilationException.create(ErrorCode.EXTERNAL_SINK_ERROR, - ExceptionUtils.getMessageOrToString(e)); - } } catch (Throwable e) { - if (isSdkException(e)) { - throw CompilationException.create(ErrorCode.EXTERNAL_SINK_ERROR, e, getMessageOrToString(e)); + Optional<T> sdkException = getSdkException(e); + if (sdkException.isPresent()) { + Throwable actualException = sdkException.get(); + throw CompilationException.create(ErrorCode.EXTERNAL_SINK_ERROR, actualException, + getMessageOrToString(actualException)); } - throw e; + if (e instanceof AlgebricksException algebricksException) { + throw algebricksException; + } + throw CompilationException.create(ErrorCode.EXTERNAL_SINK_ERROR, e, getMessageOrToString(e)); } } @@ -145,6 +150,7 @@ abstract class AbstractCloudExternalFileWriterFactory implements IExternalFileWr } catch (HyracksDataException e) { writer.abort(); aborted = true; + throw e; } finally { if (writer != null && !aborted) { writer.finish(); diff --git a/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/writer/GCSExternalFileWriterFactory.java b/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/writer/GCSExternalFileWriterFactory.java index 02e28810c8..49946a2e14 100644 --- a/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/writer/GCSExternalFileWriterFactory.java +++ b/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/writer/GCSExternalFileWriterFactory.java @@ -18,7 +18,7 @@ */ package org.apache.asterix.cloud.writer; -import java.io.IOException; +import java.util.Optional; import org.apache.asterix.cloud.clients.ICloudClient; import org.apache.asterix.cloud.clients.ICloudGuardian; @@ -39,11 +39,11 @@ import org.apache.hyracks.api.context.IEvaluatorContext; import org.apache.hyracks.api.context.IHyracksTaskContext; import org.apache.hyracks.api.exceptions.HyracksDataException; import org.apache.hyracks.api.exceptions.IWarningCollector; +import org.apache.hyracks.api.util.ExceptionUtils; import com.google.cloud.BaseServiceException; -import com.google.cloud.storage.StorageException; -public final class GCSExternalFileWriterFactory extends AbstractCloudExternalFileWriterFactory { +public final class GCSExternalFileWriterFactory extends AbstractCloudExternalFileWriterFactory<BaseServiceException> { private static final long serialVersionUID = 1L; static final char SEPARATOR = '/'; public static final IExternalFileWriterFactoryProvider PROVIDER = new IExternalFileWriterFactoryProvider() { @@ -71,13 +71,8 @@ public final class GCSExternalFileWriterFactory extends AbstractCloudExternalFil } @Override - boolean isNoContainerFoundException(IOException e) { - return e.getCause() instanceof StorageException; - } - - @Override - boolean isSdkException(Throwable e) { - return e instanceof BaseServiceException; + Optional<BaseServiceException> getSdkException(Throwable ex) { + return ExceptionUtils.getCauseOfType(ex, BaseServiceException.class); } @Override diff --git a/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/writer/S3ExternalFileWriterFactory.java b/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/writer/S3ExternalFileWriterFactory.java index 6e5333ff0a..42f7fbbd0f 100644 --- a/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/writer/S3ExternalFileWriterFactory.java +++ b/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/writer/S3ExternalFileWriterFactory.java @@ -18,7 +18,7 @@ */ package org.apache.asterix.cloud.writer; -import java.io.IOException; +import java.util.Optional; import org.apache.asterix.cloud.clients.ICloudClient; import org.apache.asterix.cloud.clients.ICloudGuardian; @@ -39,11 +39,11 @@ import org.apache.hyracks.api.context.IEvaluatorContext; import org.apache.hyracks.api.context.IHyracksTaskContext; import org.apache.hyracks.api.exceptions.HyracksDataException; import org.apache.hyracks.api.exceptions.IWarningCollector; +import org.apache.hyracks.api.util.ExceptionUtils; import software.amazon.awssdk.core.exception.SdkException; -import software.amazon.awssdk.services.s3.model.NoSuchBucketException; -public final class S3ExternalFileWriterFactory extends AbstractCloudExternalFileWriterFactory { +public final class S3ExternalFileWriterFactory extends AbstractCloudExternalFileWriterFactory<SdkException> { private static final long serialVersionUID = 4551318140901866805L; static final char SEPARATOR = '/'; public static final IExternalFileWriterFactoryProvider PROVIDER = new IExternalFileWriterFactoryProvider() { @@ -71,13 +71,8 @@ public final class S3ExternalFileWriterFactory extends AbstractCloudExternalFile } @Override - boolean isNoContainerFoundException(IOException e) { - return e.getCause() instanceof NoSuchBucketException; - } - - @Override - boolean isSdkException(Throwable e) { - return e instanceof SdkException; + Optional<SdkException> getSdkException(Throwable ex) { + return ExceptionUtils.getCauseOfType(ex, SdkException.class); } @Override diff --git a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/exceptions/CompilationException.java b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/exceptions/CompilationException.java index 75fb18dc0e..ed29eb50b8 100644 --- a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/exceptions/CompilationException.java +++ b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/exceptions/CompilationException.java @@ -27,20 +27,21 @@ import org.apache.hyracks.api.exceptions.SourceLocation; public class CompilationException extends AlgebricksException { private static final long serialVersionUID = 1L; - public static CompilationException create(ErrorCode error, SourceLocation sourceLoc, Serializable... params) { - return new CompilationException(error, sourceLoc, params); + public static CompilationException create(ErrorCode error, Serializable... params) { + return create(error, null, null, params); } - public static CompilationException create(ErrorCode error, Serializable... params) { - return create(error, null, params); + public static CompilationException create(ErrorCode error, Throwable th, Serializable... params) { + return create(error, th, null, params); } - public CompilationException(ErrorCode error, Throwable cause, SourceLocation sourceLoc, Serializable... params) { - super(error, cause, sourceLoc, params); + public static CompilationException create(ErrorCode error, SourceLocation sourceLoc, Serializable... params) { + return create(error, null, sourceLoc, params); } - public CompilationException(ErrorCode error, SourceLocation sourceLoc, Serializable... params) { - this(error, null, sourceLoc, params); + public static CompilationException create(ErrorCode error, Throwable th, SourceLocation sourceLoc, + Serializable... params) { + return new CompilationException(error, th, sourceLoc, params); } public CompilationException(ErrorCode error, Serializable... params) { @@ -51,6 +52,14 @@ public class CompilationException extends AlgebricksException { this(errorCode, cause, null, params); } + public CompilationException(ErrorCode error, SourceLocation sourceLoc, Serializable... params) { + this(error, null, sourceLoc, params); + } + + public CompilationException(ErrorCode error, Throwable cause, SourceLocation sourceLoc, Serializable... params) { + super(error, cause, sourceLoc, params); + } + /** * @deprecated (Don't use this and provide an error code. This exists for the current exceptions and * those exceptions need to adopt error code as well.) @@ -81,4 +90,4 @@ public class CompilationException extends AlgebricksException { super(message, cause); } -} +} \ No newline at end of file diff --git a/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/util/ExceptionUtils.java b/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/util/ExceptionUtils.java index ddba3f0e29..859fd40b9f 100644 --- a/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/util/ExceptionUtils.java +++ b/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/util/ExceptionUtils.java @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.function.Function; import java.util.function.Predicate; @@ -222,4 +223,29 @@ public class ExceptionUtils { public static boolean isErrorCode(HyracksDataException throwable, ErrorCode code) { return throwable.getError().isPresent() && throwable.getError().get() == code; } + + /** + * Checks if the specific type T exception is in the causes of the current throwable, and if so returns it, + * otherwise returns null + * + * @param throwable throwable + * @param targetType exception being targeted + * @return targetType exception if found, null otherwise + * @param <T> type of exception being targeted + */ + public static <T extends Throwable> Optional<T> getCauseOfType(Throwable throwable, Class<T> targetType) { + if (throwable == null || targetType == null) { + return Optional.empty(); + } + + Throwable cause = throwable; + while (cause != null) { + if (targetType.isInstance(cause)) { + return Optional.of(targetType.cast(cause)); + } + cause = cause.getCause(); + } + + return Optional.empty(); + } }
