steveloughran commented on code in PR #5024:
URL: https://github.com/apache/hadoop/pull/5024#discussion_r1005506560
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AAWSCredentialsProvider.java:
##########
@@ -480,4 +488,151 @@ public void refresh() {
}
}
+ private static final AWSCredentials EXPECTED_CREDENTIALS = new
AWSCredentials() {
+ @Override
+ public String getAWSAccessKeyId() {
+ return "expectedAccessKey";
+ }
+
+ @Override
+ public String getAWSSecretKey() {
+ return "expectedSecret";
+ }
+ };
+
+ /**
+ * Credential provider that takes a long time.
+ */
+ private static class SlowProvider extends AbstractSessionCredentialsProvider
{
+
+ public SlowProvider(@Nullable URI uri, Configuration conf) {
+ super(uri, conf);
+ }
+
+ @Override
+ protected AWSCredentials createCredentials(Configuration config) throws
IOException {
+ // yield to other callers to induce race condition
+ Thread.yield();
+ return EXPECTED_CREDENTIALS;
+ }
+ }
+
+ private static final int CONCURRENT_THREADS = 10;
+
+ @Test
+ public void testConcurrentAuthentication() throws Throwable {
+ Configuration conf =
createProviderConfiguration(SlowProvider.class.getName());
+ Path testFile = getCSVTestPath(conf);
+
+ AWSCredentialProviderList list =
createAWSCredentialProviderSet(testFile.toUri(), conf);
+
+ SlowProvider provider = (SlowProvider) list.getProviders().get(0);
+
+ ExecutorService pool = Executors.newFixedThreadPool(CONCURRENT_THREADS);
+
+ List<Future> results = new ArrayList<>();
+
+ try {
+ assertFalse(
+ "Provider not initialized. isInitialized should be false",
+ provider.isInitialized());
+ assertFalse(
+ "Provider not initialized. hasCredentials should be false",
+ provider.hasCredentials());
+ if (provider.getInitializationException() != null) {
+ throw new AssertionError(
+ "Provider not initialized. getInitializationException should
return null",
+ provider.getInitializationException());
+ }
+
+ for (int i = 0; i < CONCURRENT_THREADS; i++) {
+ results.add(pool.submit(() -> list.getCredentials()));
+ }
+
+ for (Future result : results) {
+ AWSCredentials credentials = (AWSCredentials) result.get();
+ assertEquals(credentials.getAWSAccessKeyId(), "expectedAccessKey");
+ assertEquals(credentials.getAWSSecretKey(), "expectedSecret");
+ }
+ } finally {
+ pool.awaitTermination(10, TimeUnit.SECONDS);
+ pool.shutdown();
+ }
+
+ assertTrue(
+ "Provider initialized without errors. isInitialized should be true",
+ provider.isInitialized());
+ assertTrue(
+ "Provider initialized without errors. hasCredentials should be true",
+ provider.hasCredentials());
+ if (provider.getInitializationException() != null) {
+ throw new AssertionError(
+ "Provider initialized without errors. getInitializationException
should return null",
+ provider.getInitializationException());
+ }
+ }
+
+ /**
+ * Credential provider with error.
+ */
+ private static class ErrorProvider extends
AbstractSessionCredentialsProvider {
+
+ public ErrorProvider(@Nullable URI uri, Configuration conf) {
+ super(uri, conf);
+ }
+
+ @Override
+ protected AWSCredentials createCredentials(Configuration config) throws
IOException {
+ throw new IOException("expected error");
+ }
+ }
+
+ @Test
+ public void testConcurrentAuthenticationError() throws Throwable {
+ Configuration conf =
createProviderConfiguration(ErrorProvider.class.getName());
+ Path testFile = getCSVTestPath(conf);
+
+ AWSCredentialProviderList list =
createAWSCredentialProviderSet(testFile.toUri(), conf);
+ ErrorProvider provider = (ErrorProvider) list.getProviders().get(0);
+
+ ExecutorService pool = Executors.newFixedThreadPool(CONCURRENT_THREADS);
+
+ List<Future> results = new ArrayList<>();
Review Comment:
make list of Future<AWSCredentials> and javac should not complain on L620
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AAWSCredentialsProvider.java:
##########
@@ -480,4 +488,151 @@ public void refresh() {
}
}
+ private static final AWSCredentials EXPECTED_CREDENTIALS = new
AWSCredentials() {
+ @Override
+ public String getAWSAccessKeyId() {
+ return "expectedAccessKey";
+ }
+
+ @Override
+ public String getAWSSecretKey() {
+ return "expectedSecret";
+ }
+ };
+
+ /**
+ * Credential provider that takes a long time.
+ */
+ private static class SlowProvider extends AbstractSessionCredentialsProvider
{
+
+ public SlowProvider(@Nullable URI uri, Configuration conf) {
+ super(uri, conf);
+ }
+
+ @Override
+ protected AWSCredentials createCredentials(Configuration config) throws
IOException {
+ // yield to other callers to induce race condition
+ Thread.yield();
+ return EXPECTED_CREDENTIALS;
+ }
+ }
+
+ private static final int CONCURRENT_THREADS = 10;
+
+ @Test
+ public void testConcurrentAuthentication() throws Throwable {
+ Configuration conf =
createProviderConfiguration(SlowProvider.class.getName());
+ Path testFile = getCSVTestPath(conf);
+
+ AWSCredentialProviderList list =
createAWSCredentialProviderSet(testFile.toUri(), conf);
+
+ SlowProvider provider = (SlowProvider) list.getProviders().get(0);
+
+ ExecutorService pool = Executors.newFixedThreadPool(CONCURRENT_THREADS);
+
+ List<Future> results = new ArrayList<>();
+
+ try {
+ assertFalse(
+ "Provider not initialized. isInitialized should be false",
+ provider.isInitialized());
+ assertFalse(
+ "Provider not initialized. hasCredentials should be false",
+ provider.hasCredentials());
+ if (provider.getInitializationException() != null) {
+ throw new AssertionError(
+ "Provider not initialized. getInitializationException should
return null",
+ provider.getInitializationException());
+ }
+
+ for (int i = 0; i < CONCURRENT_THREADS; i++) {
+ results.add(pool.submit(() -> list.getCredentials()));
+ }
+
+ for (Future result : results) {
+ AWSCredentials credentials = (AWSCredentials) result.get();
+ assertEquals(credentials.getAWSAccessKeyId(), "expectedAccessKey");
+ assertEquals(credentials.getAWSSecretKey(), "expectedSecret");
+ }
+ } finally {
+ pool.awaitTermination(10, TimeUnit.SECONDS);
+ pool.shutdown();
+ }
+
+ assertTrue(
+ "Provider initialized without errors. isInitialized should be true",
+ provider.isInitialized());
+ assertTrue(
+ "Provider initialized without errors. hasCredentials should be true",
+ provider.hasCredentials());
+ if (provider.getInitializationException() != null) {
+ throw new AssertionError(
+ "Provider initialized without errors. getInitializationException
should return null",
+ provider.getInitializationException());
+ }
+ }
+
+ /**
+ * Credential provider with error.
+ */
+ private static class ErrorProvider extends
AbstractSessionCredentialsProvider {
+
+ public ErrorProvider(@Nullable URI uri, Configuration conf) {
Review Comment:
i see a style complaint about " Redundant 'public' modifier. "...but it does
need to be public for the reflection-based construction, doesn't it?
make these classes package private (as the existing ones are) and the
complaints should go away. maybe.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
