From 242b6208b03368f3ffa40b783358aba12f8a432e Mon Sep 17 00:00:00 2001 From: Ahmar Suhail Date: Wed, 9 Apr 2025 13:12:39 +0100 Subject: [PATCH 1/4] skip AAL tests if encryption is set --- ...3AContractAnalyticsStreamVectoredRead.java | 17 +++++++++++++++++ ...tS3AAnalyticsAcceleratorStreamReading.java | 1 + .../apache/hadoop/fs/s3a/S3ATestUtils.java | 19 +++++++++++++++++++ 3 files changed, 37 insertions(+) diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractAnalyticsStreamVectoredRead.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractAnalyticsStreamVectoredRead.java index a2b053783bcd2..9962ad5622fb4 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractAnalyticsStreamVectoredRead.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractAnalyticsStreamVectoredRead.java @@ -22,7 +22,11 @@ import org.apache.hadoop.fs.contract.AbstractContractVectoredReadTest; import org.apache.hadoop.fs.contract.AbstractFSContract; +import java.io.IOException; +import java.io.UncheckedIOException; + import static org.apache.hadoop.fs.s3a.S3ATestUtils.enableAnalyticsAccelerator; +import static org.apache.hadoop.fs.s3a.S3ATestUtils.skipIfEncryptionSet; /** * S3A contract tests for vectored reads with the Analytics stream. The analytics stream does @@ -44,6 +48,19 @@ public ITestS3AContractAnalyticsStreamVectoredRead(String bufferType) { protected Configuration createConfiguration() { Configuration conf = super.createConfiguration(); enableAnalyticsAccelerator(conf); + + // If encryption is set, some AAL tests will fail. This is because AAL caches the head request response, and uses + // the eTag when making a GET request. When using encryption, the eTag is no longer a hash of the object content, + // and is not always the same when the same object is created multiple times. This test creates the file + // vectored_file.txt before running each test, which will have a different eTag when using encryption, leading to + // preconditioned failures. This issue is tracked in: + // https://github.com/awslabs/analytics-accelerator-s3/issues/218 + try { + skipIfEncryptionSet(conf); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + conf.set("fs.contract.vector-io-early-eof-check", "false"); return conf; } diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AAnalyticsAcceleratorStreamReading.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AAnalyticsAcceleratorStreamReading.java index 9a0afdf651315..816ec90646f8a 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AAnalyticsAcceleratorStreamReading.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AAnalyticsAcceleratorStreamReading.java @@ -69,6 +69,7 @@ public class ITestS3AAnalyticsAcceleratorStreamReading extends AbstractS3ATestBa @Before public void setUp() throws Exception { super.setup(); + skipIfClientSideEncryption(); externalTestFile = getExternalData(getConfiguration()); } diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java index 53e4a68cbb60b..178114816f768 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java @@ -105,6 +105,7 @@ import static org.apache.hadoop.fs.contract.ContractTestUtils.createFile; import static org.apache.hadoop.fs.impl.FlagSet.createFlagSet; +import static org.apache.hadoop.fs.s3a.S3AEncryptionMethods.SSE_S3; import static org.apache.hadoop.fs.s3a.impl.streams.InputStreamType.Analytics; import static org.apache.hadoop.fs.s3a.impl.streams.InputStreamType.Prefetch; import static org.apache.hadoop.fs.s3a.impl.CallableSupplier.submit; @@ -1709,6 +1710,24 @@ public static void skipIfEncryptionNotSet(Configuration configuration, } } + /** + * Skip a test if encryption algorithm is not empty, or if it is set to anything other than AES256. + * + * @param configuration configuration + * @throws IOException if the secret lookup fails. + */ + public static void skipIfEncryptionSet(Configuration configuration) throws IOException { + String bucket = getTestBucketName(configuration); + final EncryptionSecrets secrets = buildEncryptionSecrets(bucket, configuration); + S3AEncryptionMethods s3AEncryptionMethods = secrets.getEncryptionMethod(); + + if (s3AEncryptionMethods.getMethod().equals(SSE_S3.getMethod()) || s3AEncryptionMethods.getMethod().isEmpty()) { + return; + } + + skip("Encryption method is set to " + s3AEncryptionMethods.getMethod()); + } + /** * Get the input stream statistics of an input stream. * Raises an exception if the inner stream is not an S3A input stream From 924d82600aa747f14fa52bc20677f186852a4c19 Mon Sep 17 00:00:00 2001 From: Ahmar Suhail Date: Thu, 10 Apr 2025 10:05:53 +0100 Subject: [PATCH 2/4] review comments --- ...S3AContractAnalyticsStreamVectoredRead.java | 13 ++----------- .../org/apache/hadoop/fs/s3a/S3ATestUtils.java | 18 +++++++++++------- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractAnalyticsStreamVectoredRead.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractAnalyticsStreamVectoredRead.java index 9962ad5622fb4..fb1695b1ce279 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractAnalyticsStreamVectoredRead.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractAnalyticsStreamVectoredRead.java @@ -22,11 +22,8 @@ import org.apache.hadoop.fs.contract.AbstractContractVectoredReadTest; import org.apache.hadoop.fs.contract.AbstractFSContract; -import java.io.IOException; -import java.io.UncheckedIOException; - import static org.apache.hadoop.fs.s3a.S3ATestUtils.enableAnalyticsAccelerator; -import static org.apache.hadoop.fs.s3a.S3ATestUtils.skipIfEncryptionSet; +import static org.apache.hadoop.fs.s3a.S3ATestUtils.skipForAnyEncryptionExceptSSES3; /** * S3A contract tests for vectored reads with the Analytics stream. The analytics stream does @@ -48,19 +45,13 @@ public ITestS3AContractAnalyticsStreamVectoredRead(String bufferType) { protected Configuration createConfiguration() { Configuration conf = super.createConfiguration(); enableAnalyticsAccelerator(conf); - // If encryption is set, some AAL tests will fail. This is because AAL caches the head request response, and uses // the eTag when making a GET request. When using encryption, the eTag is no longer a hash of the object content, // and is not always the same when the same object is created multiple times. This test creates the file // vectored_file.txt before running each test, which will have a different eTag when using encryption, leading to // preconditioned failures. This issue is tracked in: // https://github.com/awslabs/analytics-accelerator-s3/issues/218 - try { - skipIfEncryptionSet(conf); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - + skipForAnyEncryptionExceptSSES3(conf); conf.set("fs.contract.vector-io-early-eof-check", "false"); return conf; } diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java index 178114816f768..81351c0214bd2 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java @@ -1714,18 +1714,22 @@ public static void skipIfEncryptionNotSet(Configuration configuration, * Skip a test if encryption algorithm is not empty, or if it is set to anything other than AES256. * * @param configuration configuration - * @throws IOException if the secret lookup fails. */ - public static void skipIfEncryptionSet(Configuration configuration) throws IOException { + public static void skipForAnyEncryptionExceptSSES3(Configuration configuration) { String bucket = getTestBucketName(configuration); - final EncryptionSecrets secrets = buildEncryptionSecrets(bucket, configuration); - S3AEncryptionMethods s3AEncryptionMethods = secrets.getEncryptionMethod(); + try { + final EncryptionSecrets secrets = buildEncryptionSecrets(bucket, configuration); + S3AEncryptionMethods s3AEncryptionMethods = secrets.getEncryptionMethod(); - if (s3AEncryptionMethods.getMethod().equals(SSE_S3.getMethod()) || s3AEncryptionMethods.getMethod().isEmpty()) { - return; + if (s3AEncryptionMethods.getMethod().equals(SSE_S3.getMethod()) || s3AEncryptionMethods.getMethod().isEmpty()) { + return; + } + + skip("Encryption method is set to " + s3AEncryptionMethods.getMethod()); + } catch (IOException e) { + throw new UncheckedIOException(e); } - skip("Encryption method is set to " + s3AEncryptionMethods.getMethod()); } /** From 112222a27d7ee7a55441942a1e12e2bba1588b21 Mon Sep 17 00:00:00 2001 From: Ahmar Suhail Date: Thu, 10 Apr 2025 17:43:57 +0100 Subject: [PATCH 3/4] checkstyle --- .../ITestS3AContractAnalyticsStreamVectoredRead.java | 10 ++++++---- .../java/org/apache/hadoop/fs/s3a/S3ATestUtils.java | 6 ++++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractAnalyticsStreamVectoredRead.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractAnalyticsStreamVectoredRead.java index fb1695b1ce279..5ae3f76fab9d8 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractAnalyticsStreamVectoredRead.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractAnalyticsStreamVectoredRead.java @@ -26,10 +26,12 @@ import static org.apache.hadoop.fs.s3a.S3ATestUtils.skipForAnyEncryptionExceptSSES3; /** - * S3A contract tests for vectored reads with the Analytics stream. The analytics stream does - * not explicitly implement the vectoredRead() method, or currently do and vectored-read specific - * optimisations (such as range coalescing). However, this test ensures that the base implementation - * of readVectored {@link org.apache.hadoop.fs.PositionedReadable} still works. + * S3A contract tests for vectored reads with the Analytics stream. + * The analytics stream does not explicitly implement the vectoredRead() method, + * or currently do and vectored-read specific optimisations + * (such as range coalescing). However, this test ensures that the base + * implementation of readVectored {@link org.apache.hadoop.fs.PositionedReadable} + * still works. */ public class ITestS3AContractAnalyticsStreamVectoredRead extends AbstractContractVectoredReadTest { diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java index 81351c0214bd2..41c098a76dbcd 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java @@ -1711,7 +1711,8 @@ public static void skipIfEncryptionNotSet(Configuration configuration, } /** - * Skip a test if encryption algorithm is not empty, or if it is set to anything other than AES256. + * Skip a test if encryption algorithm is not empty, or if it is set to + * anything other than AES256. * * @param configuration configuration */ @@ -1721,7 +1722,8 @@ public static void skipForAnyEncryptionExceptSSES3(Configuration configuration) final EncryptionSecrets secrets = buildEncryptionSecrets(bucket, configuration); S3AEncryptionMethods s3AEncryptionMethods = secrets.getEncryptionMethod(); - if (s3AEncryptionMethods.getMethod().equals(SSE_S3.getMethod()) || s3AEncryptionMethods.getMethod().isEmpty()) { + if (s3AEncryptionMethods.getMethod().equals(SSE_S3.getMethod()) + || s3AEncryptionMethods.getMethod().isEmpty()) { return; } From fd098dba0878fed765fe9b01840a1c67ef10b975 Mon Sep 17 00:00:00 2001 From: Ahmar Suhail Date: Fri, 11 Apr 2025 10:40:06 +0100 Subject: [PATCH 4/4] more checkstyle --- ...ITestS3AContractAnalyticsStreamVectoredRead.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractAnalyticsStreamVectoredRead.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractAnalyticsStreamVectoredRead.java index 5ae3f76fab9d8..8cf182680c350 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractAnalyticsStreamVectoredRead.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractAnalyticsStreamVectoredRead.java @@ -47,11 +47,14 @@ public ITestS3AContractAnalyticsStreamVectoredRead(String bufferType) { protected Configuration createConfiguration() { Configuration conf = super.createConfiguration(); enableAnalyticsAccelerator(conf); - // If encryption is set, some AAL tests will fail. This is because AAL caches the head request response, and uses - // the eTag when making a GET request. When using encryption, the eTag is no longer a hash of the object content, - // and is not always the same when the same object is created multiple times. This test creates the file - // vectored_file.txt before running each test, which will have a different eTag when using encryption, leading to - // preconditioned failures. This issue is tracked in: + // If encryption is set, some AAL tests will fail. + // This is because AAL caches the head request response, and uses + // the eTag when making a GET request. When using encryption, the eTag is + // no longer a hash of the object content, and is not always the same when + // the same object is created multiple times. This test creates the file + // vectored_file.txt before running each test, which will have a + // different eTag when using encryption, leading to preconditioned failures. + // This issue is tracked in: // https://github.com/awslabs/analytics-accelerator-s3/issues/218 skipForAnyEncryptionExceptSSES3(conf); conf.set("fs.contract.vector-io-early-eof-check", "false");