[ https://issues.apache.org/jira/browse/HADOOP-18565?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17702812#comment-17702812 ]
ASF GitHub Bot commented on HADOOP-18565: ----------------------------------------- ahmarsuhail commented on code in PR #5421: URL: https://github.com/apache/hadoop/pull/5421#discussion_r1142313258 ########## hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java: ########## @@ -834,58 +834,28 @@ protected static S3AStorageStatistics createStorageStatistics( * @throws UnknownStoreException the bucket is absent * @throws IOException any other problem talking to S3 */ - // TODO: Review: this used to call doesBucketExist in v1, which does not check permissions, - // not even read access. @Retries.RetryTranslated protected void verifyBucketExists() throws UnknownStoreException, IOException { if (!invoker.retry("doesBucketExist", bucket, true, trackDurationOfOperation(getDurationTrackerFactory(), STORE_EXISTS_PROBE.getSymbol(), () -> { try { + if (bucketRegions.containsKey(bucket)) { + return true; + } s3Client.headBucket(HeadBucketRequest.builder().bucket(bucket).build()); return true; - } catch (NoSuchBucketException e) { - return false; - } - }))) { - throw new UnknownStoreException("s3a://" + bucket + "/", " Bucket does " + "not exist"); - } - } - - /** - * Verify that the bucket exists. This will correctly throw an exception - * when credentials are invalid. - * TODO: Review. May be redundant in v2. - * Retry policy: retrying, translated. - * @throws UnknownStoreException the bucket is absent - * @throws IOException any other problem talking to S3 - */ - @Retries.RetryTranslated - protected void verifyBucketExistsV2() - throws UnknownStoreException, IOException { - if (!invoker.retry("doesBucketExistV2", bucket, true, - trackDurationOfOperation(getDurationTrackerFactory(), - STORE_EXISTS_PROBE.getSymbol(), - () -> { - // Bug in SDK always returns `true` for AccessPoint ARNs with `doesBucketExistV2()` - // expanding implementation to use ARNs and buckets correctly - try { - s3Client.getBucketAcl(GetBucketAclRequest.builder() - .bucket(bucket) - .build()); } catch (AwsServiceException ex) { int statusCode = ex.statusCode(); if (statusCode == SC_404_NOT_FOUND || - (statusCode == SC_403_FORBIDDEN && - ex.getMessage().contains(AP_INACCESSIBLE))) { + (statusCode == SC_403_FORBIDDEN && accessPoint != null)) { return false; } } return true; }))) { - throw new UnknownStoreException("s3a://" + bucket + "/", " Bucket does " - + "not exist"); + throw new UnknownStoreException("s3a://" + bucket + "/", " Bucket does " + "not exist"); Review Comment: have included the endpoint. but wondering, in the `getS3Region()` probe, we build a `s3Client` with the region `eu-west-2` to do a region probe. This client does not have an endpoint configured, so this probe will fail for on-prem and third party stores. is there anything we need to do for this? if the region is not set, `getS3Region()` will fail before any bucket probing is done > AWS SDK V2 - Complete outstanding items > --------------------------------------- > > Key: HADOOP-18565 > URL: https://issues.apache.org/jira/browse/HADOOP-18565 > Project: Hadoop Common > Issue Type: Sub-task > Components: fs/s3 > Affects Versions: 3.4.0 > Reporter: Ahmar Suhail > Priority: Major > Labels: pull-request-available > > The following work remains to complete the SDK upgrade work: > * S3A allows users configure to custom signers, add in support for this. > * Remove SDK V1 bundle dependency > * Update `getRegion()` logic to use retries. > * Add in progress listeners for `S3ABlockOutputStream` > * Fix any failing tests. -- This message was sent by Atlassian Jira (v8.20.10#820010) --------------------------------------------------------------------- To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org