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

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

                Author: ASF GitHub Bot
            Created on: 07/Jun/22 12:25
            Start Date: 07/Jun/22 12:25
    Worklog Time Spent: 10m 
      Work Description: steveloughran commented on code in PR #3289:
URL: https://github.com/apache/hadoop/pull/3289#discussion_r891151772


##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java:
##########
@@ -3970,63 +4062,64 @@ InitiateMultipartUploadResult initiateMultipartUpload(
 
   /**
    * Perform post-write actions.
-   * <p></p>
+   * <p>
    * This operation MUST be called after any PUT/multipart PUT completes
    * successfully.
-   * <p></p>
-   * The actions include:
-   * <ol>
-   *   <li>
-   *     Calling
-   *     {@link #deleteUnnecessaryFakeDirectories(Path)}
-   *     if directory markers are not being retained.
-   *   </li>
-   *   <li>
-   *     Updating any metadata store with details on the newly created
-   *     object.
-   *     </li>
-   * </ol>
+   * <p>
+   * The actions include calling
+   * {@link #deleteUnnecessaryFakeDirectories(Path)}
+   * if directory markers are not being retained.
    * @param key key written to
    * @param length  total length of file written
    * @param eTag eTag of the written object
    * @param versionId S3 object versionId of the written object
+   * @param putOptions put object options
    */
   @InterfaceAudience.Private
-  @Retries.RetryTranslated("Except if failOnMetadataWriteError=false, in which"
-      + " case RetryExceptionsSwallowed")
-  void finishedWrite(String key, long length, String eTag, String versionId) {
+  @Retries.RetryExceptionsSwallowed
+  void finishedWrite(
+      String key,
+      long length,
+      String eTag,
+      String versionId,
+      PutObjectOptions putOptions) {
     LOG.debug("Finished write to {}, len {}. etag {}, version {}",
         key, length, eTag, versionId);
-    Path p = keyToQualifiedPath(key);
     Preconditions.checkArgument(length >= 0, "content length is negative");
-    // kick off an async delete
-    CompletableFuture<?> deletion;
-    if (!keepDirectoryMarkers(p)) {
-      deletion = submit(
-          unboundedThreadPool, getActiveAuditSpan(),
-          () -> {
-            deleteUnnecessaryFakeDirectories(
-                p.getParent()
-            );
-            return null;
-          });
-    } else {
-      deletion = null;
+    if (!putOptions.isKeepMarkers()) {

Review Comment:
   async was to mix the delete call with the s3guard update for minor 
performance benefits. without s3guard, no possibilities of overlapping IO, just 
risk of waiting for a spare thread. it has become counterproductive





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

    Worklog Id:     (was: 779055)
    Time Spent: 9h 40m  (was: 9.5h)

> Improve Magic Committer Performance
> -----------------------------------
>
>                 Key: HADOOP-17833
>                 URL: https://issues.apache.org/jira/browse/HADOOP-17833
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: fs/s3
>    Affects Versions: 3.3.1
>            Reporter: Steve Loughran
>            Assignee: Steve Loughran
>            Priority: Minor
>              Labels: pull-request-available
>          Time Spent: 9h 40m
>  Remaining Estimate: 0h
>
> Magic committer tasks can be slow because every file created with 
> overwrite=false triggers a HEAD (verify there's no file) and a LIST (that 
> there's no dir). And because of delayed manifestations, it may not behave as 
> expected.
> ParquetOutputFormat is one example of a library which does this.
> we could fix parquet to use overwrite=true, but (a) there may be surprises in 
> other uses (b) it'd still leave the list and (c) do nothing for other formats 
> call
> Proposed: createFile() under a magic path to skip all probes for file/dir at 
> end of path
> Only a single task attempt Will be writing to that directory and it should 
> know what it is doing. If there is conflicting file names and parts across 
> tasks that won't even get picked up at this point. Oh and none of the 
> committers ever check for this: you'll get the last file manifested (s3a) or 
> renamed (file)
> If we skip the checks we will save 2 HTTP requests/file.



--
This message was sent by Atlassian Jira
(v8.20.7#820007)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to