diqiu50 commented on code in PR #14517:
URL: https://github.com/apache/gravitino/pull/14517#discussion_r4442805006
##########
catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueExceptionConverter.java:
##########
@@ -133,11 +182,110 @@ static ConnectionFailedException
toConnectionException(SdkException e) {
awsErrorDetail((GlueException) e));
}
+ String detail =
+ e instanceof GlueException
+ ? awsErrorDetail((GlueException) e)
+ : StringUtils.defaultIfBlank(
+ ExceptionMessages.usefulMessage(e),
e.getClass().getSimpleName());
+ return new ConnectionFailedException(
+ e,
+ "Failed to connect to AWS Glue at %s: %s",
+ GlueClientProvider.describeEndpoint(client),
+ detail);
+ }
+
+ /**
+ * Converts the failures that do not depend on the object being operated on:
missing, rejected or
+ * mis-scoped credentials, throttling, invalid input, access denial and an
unreachable endpoint.
+ *
+ * @param e the SDK failure
+ * @param context description of the operation context for error messages
+ * @param client the Glue client that made the call, used to name its region
and endpoint; may be
+ * null
+ * @return the converted exception, or null when the failure is specific to
the object (for
+ * example not found or already exists) and must be converted by the
caller
+ */
+ @Nullable
+ static RuntimeException toClientException(
+ SdkException e, String context, @Nullable GlueClient client) {
+ if (e instanceof SdkClientException) {
+ SdkClientException clientException = (SdkClientException) e;
+ if (isCredentialFailure(clientException)) {
+ return toCredentialException(clientException, context);
+ }
+ String endpoint = GlueClientProvider.describeEndpoint(client);
+ String detail =
+ StringUtils.defaultIfBlank(
+ ExceptionMessages.usefulMessage(e),
e.getClass().getSimpleName());
+ if (e instanceof ApiCallTimeoutException || e instanceof
ApiCallAttemptTimeoutException) {
+ return new ConnectionFailedException(
+ e, "AWS Glue at %s timed out for %s: %s", endpoint, context,
detail);
+ }
+ return new ConnectionFailedException(
+ e, "Failed to reach AWS Glue at %s for %s: %s", endpoint, context,
detail);
+ }
+ if (!(e instanceof GlueException)) {
+ return null;
Review Comment:
Fixed the downgrade in 7e208ae56: an Iceberg `ForbiddenException` whose AWS
cause is not a Glue error (e.g. S3) now stays 403. Mapping S3 service errors by
status code is left for a follow-up on the Glue Iceberg table path, together
with the metadata load failures that are currently swallowed there.
##########
catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueIcebergTableHelper.java:
##########
@@ -188,6 +196,13 @@ static Map<String, String>
buildIcebergCatalogProperties(Map<String, String> con
icebergProps.put(GLUE_ENDPOINT, endpoint);
}
+ // The Iceberg AWS clients take no total call timeout, so bound each
connection and read by
+ // the per-attempt timeout of the catalog's own Glue client.
+ String attemptTimeoutMs =
String.valueOf(GlueClientProvider.apiCallAttemptTimeoutMs(config));
+ icebergProps.put(HTTP_CLIENT_TYPE, "apache");
+ icebergProps.put(HTTP_CLIENT_APACHE_CONNECTION_TIMEOUT_MS,
attemptTimeoutMs);
+ icebergProps.put(HTTP_CLIENT_APACHE_SOCKET_TIMEOUT_MS, attemptTimeoutMs);
Review Comment:
Iceberg 1.11 exposes only HTTP connection/socket timeouts for its AWS
clients; passing our own client needs a custom `client.factory`. For this PR
the docs now state that `aws-glue-api-call-timeout-ms` and
`aws-glue-max-attempts` apply to the catalog's Glue client, while Iceberg's
clients only take the attempt timeout. Full support is planned with the Glue
Iceberg table follow-up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
