[ 
https://issues.apache.org/jira/browse/HADOOP-17023?focusedWorklogId=480090&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-480090
 ]

ASF GitHub Bot logged work on HADOOP-17023:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 08/Sep/20 12:20
            Start Date: 08/Sep/20 12:20
    Worklog Time Spent: 10m 
      Work Description: steveloughran commented on a change in pull request 
#2257:
URL: https://github.com/apache/hadoop/pull/2257#discussion_r484868210



##########
File path: 
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3Guard.java
##########
@@ -362,14 +340,16 @@ public static BulkOperationState initiateBulkWrite(
    * @return Final result of directory listing.
    * @throws IOException if metadata store update failed
    */
-  public static S3AFileStatus[] dirListingUnion(MetadataStore ms, Path path,
-      List<S3AFileStatus> backingStatuses, DirListingMetadata dirMeta,
-      boolean isAuthoritative, ITtlTimeProvider timeProvider)
-      throws IOException {
+  public static RemoteIterator<S3AFileStatus> dirListingUnion(
+          MetadataStore ms, Path path,
+          RemoteIterator<S3AFileStatus> backingStatuses,
+          DirListingMetadata dirMeta, boolean isAuthoritative,
+          ITtlTimeProvider timeProvider, Listing listing)

Review comment:
       we are into callback country here. Could you 
   
   1. create an interface (say in S3Guard) for 
createProvidedFileStatusIteratorForS3Guard  (it could contain all params for 
`createProvidedFileStatusIterator` oterh than the dir meta list, 2. Have 
Listing implement it
   3. Pass the interface in, not the reference to Listing. 
   
   I'm just trying to move away from having all the classes in here having 
intimate knowledge of every other class. An interface with a single method lets 
us control exactly what S3Guard does. 
   
   You could use a Lambda-expression if you want -for a single operation it 
would work, just take `Function<S3AFileStatus[], RemoteIterator<S3AFileStatus>` 
as a parameter and pass the operation in that way. But, given you use this 
operation in tests as well as production code, you should still implement this 
function as a single method in Listing




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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Issue Time Tracking
-------------------

    Worklog Id:     (was: 480090)
    Time Spent: 2h  (was: 1h 50m)

> Tune listStatus() api of s3a.
> -----------------------------
>
>                 Key: HADOOP-17023
>                 URL: https://issues.apache.org/jira/browse/HADOOP-17023
>             Project: Hadoop Common
>          Issue Type: Sub-task
>          Components: fs/s3
>    Affects Versions: 3.2.1
>            Reporter: Mukund Thakur
>            Assignee: Mukund Thakur
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 2h
>  Remaining Estimate: 0h
>
> Similar optimisation which was done for listLocatedSttaus() 
> https://issues.apache.org/jira/browse/HADOOP-16465  can done for listStatus() 
> api as well. 
> This is going to reduce the number of remote calls in case of directory 
> listing.
>  
> CC [~ste...@apache.org] [~shwethags]



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to