mukund-thakur commented on code in PR #3289:
URL: https://github.com/apache/hadoop/pull/3289#discussion_r890553440


##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java:
##########
@@ -1599,69 +1604,88 @@ public FSDataOutputStream create(Path f, FsPermission 
permission,
       boolean overwrite, int bufferSize, short replication, long blockSize,
       Progressable progress) throws IOException {
     final Path path = qualify(f);
+
     // the span will be picked up inside the output stream
     return trackDurationAndSpan(INVOCATION_CREATE, path, () ->

Review Comment:
   Isn't Audit span suppossed to be already present.
   



##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java:
##########
@@ -3612,7 +3697,7 @@ public boolean createEmptyDir(Path path, StoreContext 
storeContext)
           new MkdirOperation(
               storeContext,
               path,
-              createMkdirOperationCallbacks()));
+              createMkdirOperationCallbacks(), false));

Review Comment:
   Why not isMagicPath() call here? Are we sure that createEmptyDir() will 
never be called during Magic Comitter operations?



##########
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:
   Why are we changing async to sync call here?



##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/commit/MagicCommitIntegration.java:
##########
@@ -38,8 +39,8 @@
  * in this case:
  * <ol>
  *   <li>{@link #isMagicCommitPath(Path)} will always return false.</li>

Review Comment:
   nit: new method isUnderMagicPath needs to be added here as well.



##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/WriteOperationHelper.java:
##########
@@ -257,17 +257,25 @@ public PutObjectRequest createPutObjectRequest(String 
destKey,
    * Create a {@link PutObjectRequest} request to upload a file.
    * @param dest key to PUT to.
    * @param sourceFile source file
+   * @param headers optional map of custom headers.
    * @return the request
    */
   @Retries.OnceRaw
-  public PutObjectRequest createPutObjectRequest(String dest,
-      File sourceFile) {
+  public PutObjectRequest createPutObjectRequest(
+      String dest,
+      File sourceFile,
+      @Nullable final Map<String, String> headers) {

Review Comment:
   Why nullable? Empty map should be fine ?



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