ahmarsuhail commented on code in PR #7295:
URL: https://github.com/apache/hadoop/pull/7295#discussion_r1923575039


##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/S3AStoreImpl.java:
##########
@@ -230,7 +232,23 @@ public class S3AStoreImpl
   @Override
   protected void serviceInit(final Configuration conf) throws Exception {
 
-    objectInputStreamFactory = createStreamFactory(conf);
+    if(conf.getBoolean(ANALYTICS_ACCELERATOR_ENABLED_KEY, 
ANALYTICS_ACCELERATOR_ENABLED_DEFAULT)) {
+      boolean analyticsAcceleratorCRTEnabled = 
conf.getBoolean(ANALYTICS_ACCELERATOR_CRT_ENABLED,
+                      ANALYTICS_ACCELERATOR_CRT_ENABLED_DEFAULT);
+      final S3AsyncClient s3AsyncClient;
+      LOG.info("Using S3SeekableInputStream");
+      if(analyticsAcceleratorCRTEnabled) {
+        LOG.info("Using S3 CRT client for analytics accelerator S3");
+        s3AsyncClient = S3CrtAsyncClient.builder().maxConcurrency(600).build();

Review Comment:
   Agree with Fuat, we can't merge with client creation here. 



##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/S3AStoreImpl.java:
##########
@@ -230,7 +232,23 @@ public class S3AStoreImpl
   @Override
   protected void serviceInit(final Configuration conf) throws Exception {
 
-    objectInputStreamFactory = createStreamFactory(conf);
+    if(conf.getBoolean(ANALYTICS_ACCELERATOR_ENABLED_KEY, 
ANALYTICS_ACCELERATOR_ENABLED_DEFAULT)) {

Review Comment:
   this should go into the `StreamIntegration.createStreamFactory()` method in 
my opinion. And over there, you want to use the `InputStreamType.Analytics` in 
the conf check. 
   
   You can update the enum, so that the factory method takes parameters. 
Something like:
   
   ```
     public Function<Configuration, ObjectInputStreamFactory> 
factory(FactoryParameters factoryParameters) {
       return factory;
     }
   
   ```
    
   In the createStreamFactory method, you can do:
   
   ```
       if (conf.getEnum(INPUT_STREAM_TYPE, defaultStream) == 
InputStreamType.Analytics) {
          new FactoryParameters.withS3Client()
       }
   
   ```
   
   Let's discuss in case it's not clear!



-- 
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]

Reply via email to