You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/12/10 20:32:36 UTC

[GitHub] [spark] steveloughran opened a new pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer do…

steveloughran opened a new pull request #30714:
URL: https://github.com/apache/spark/pull/30714


   …n't track bytes
   
   BasicWriteStatsTracker to probe for a custom Xattr if the size of
   the generated file is 0 bytes; if found and parseable use that as
   the declared length of the output.
   
   The matching Hadoop patch HADOOP-17414.
   
   * Returns all S3 object headers as XAttr attributes prefixed "header."
   * Sets the custom header x-hadoop-s3a-magic-data-length to the length of
     the data in the marker file.
   
   As a result, spark job tracking will correctly report the amount of data uploaded
   and yet to materialize.
   
   Testing
   
   New tests in BasicWriteTaskStatsTrackerSuite which use a filter FS to
   implement getXAttr on top of LocalFS; this is used to explore the set of
   options:
   * no XAttr API implementation (existing tests; what callers would see with
     most filesystems)
   * no attribute found (HDFS, ABFS without the attribute)
   * invalid data of different forms
   
   All of these return Some(0) as file length.
   
   External downstream testing has done the full hadoop+spark end
   to end operation, with manual review of logs to verify that the
   data was succesfully collected from the attribute.
   
   Change-Id: I1fd0b9ac2eba1c8c27cbd776739e693a57d38fc3
   
   <!--
   Thanks for sending a pull request!  Here are some tips for you:
     1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
     2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
     3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
     4. Be sure to keep the PR description updated to reflect all changes.
     5. Please write your PR title to summarize what this PR proposes.
     6. If possible, provide a concise example to reproduce the issue for a faster review.
     7. If you want to add a new configuration, please read the guideline first for naming configurations in
        'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
   -->
   
   ### What changes were proposed in this pull request?
   <!--
   Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue. 
   If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
     1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
     2. If you fix some SQL features, you can provide some references of other DBMSes.
     3. If there is design documentation, please add the link.
     4. If there is a discussion in the mailing list, please add the link.
   -->
   
   
   ### Why are the changes needed?
   <!--
   Please clarify why the changes are needed. For instance,
     1. If you propose a new API, clarify the use case for a new API.
     2. If you fix a bug, you can clarify why it is a bug.
   -->
   
   
   ### Does this PR introduce _any_ user-facing change?
   <!--
   Note that it means *any* user-facing change including all aspects such as the documentation fix.
   If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
   If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
   If no, write 'No'.
   -->
   
   
   ### How was this patch tested?
   <!--
   If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
   If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
   If tests were not added, please describe why they were not added and/or why it was difficult to add.
   -->
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-742790790


   **[Test build #132588 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132588/testReport)** for PR 30714 at commit [`132a2e9`](https://github.com/apache/spark/commit/132a2e989778493551e3d84bf0a37a7dd69d013d).


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a change in pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #30714:
URL: https://github.com/apache/spark/pull/30714#discussion_r576458517



##########
File path: docs/cloud-integration.md
##########
@@ -176,12 +181,19 @@ different stores and connectors when renaming directories:
 | Amazon S3     | s3a       | Unsafe                  | O(data) |
 | Azure Storage | wasb      | Safe                    | O(files) |
 | Azure Datalake Gen 2 | abfs | Safe                  | O(1) |
-| Google Cloud Storage | gs        | Safe                    | O(1) |
+| Google Cloud Storage | gs        | Mixed                    | O(files) |
 
 1. As storing temporary files can run up charges; delete
 directories called `"_temporary"` on a regular basis.
 1. For AWS S3, set a limit on how long multipart uploads can remain outstanding.
 This avoids incurring bills from incompleted uploads.
+1. For Google cloud, directory rename is file-by-file. Consider using the v2 committer
+and only write code which generates idemportent output -including filenames,
+as it is *no more unsafe* than the v1 committer, and faster.
+
+```
+spark.hadoop.mapreduce.fileoutputcommitter.algorithm.version 2
+```

Review comment:
       We can remove this repetition (line 194 ~ 196) because we already have this at the beginning of this section.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-779318834


   **[Test build #135156 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135156/testReport)** for PR 30714 at commit [`4b08f49`](https://github.com/apache/spark/commit/4b08f491ae98dc9b035d72daf0e2d817728537b8).


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] steveloughran commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
steveloughran commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-769849804


   I'll update the docs. Also going to cut back on the warnings now that AWS S3 is consistent (And all third party ones were consistent out the box too). This is a good thing for apps like spark (much easier to do workflows across applications), but you still can't safely use the rename committer there as dir rename is non-atomic.
   Fortunately, commit-by-rename is so slow people have a reason to switch to a better one, either the S3A ones or that from EMR :)


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-779434961


   **[Test build #135156 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135156/testReport)** for PR 30714 at commit [`4b08f49`](https://github.com/apache/spark/commit/4b08f491ae98dc9b035d72daf0e2d817728537b8).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-780682115


   **[Test build #135196 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135196/testReport)** for PR 30714 at commit [`81c0a52`](https://github.com/apache/spark/commit/81c0a5244fa5a17184b59df9f08aed0d20868276).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-742790790


   **[Test build #132588 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132588/testReport)** for PR 30714 at commit [`132a2e9`](https://github.com/apache/spark/commit/132a2e989778493551e3d84bf0a37a7dd69d013d).


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-765594768


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134371/
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-780698192


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135196/
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-779396707


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39737/
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HeartSaVioR commented on a change in pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on a change in pull request #30714:
URL: https://github.com/apache/spark/pull/30714#discussion_r575867762



##########
File path: docs/cloud-integration.md
##########
@@ -49,7 +49,6 @@ They cannot be used as a direct replacement for a cluster filesystem such as HDF
 
 Key differences are:
 
-* Changes to stored objects may not be immediately visible, both in directory listings and actual data access.

Review comment:
       The line deletion is based on the fact S3 is now strong consistency, but that also means we only consider these three vendors. (Probably you're considering more like S3 compatible implementations, but you've also mentioned you don't consider the chinese cloud stores so it can't be exhaustive.) Why not explicitly saying it and update the description tied to these vendors? We would never be able to consider all possible implementations and for some minority the description may be wrong. Let's just make it 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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-780572456


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39777/
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-779441425


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135156/
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-765516098


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38958/
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] steveloughran commented on a change in pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #30714:
URL: https://github.com/apache/spark/pull/30714#discussion_r566857654



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/BasicWriteStatsTracker.scala
##########
@@ -66,14 +67,65 @@ class BasicWriteTaskStatsTracker(hadoopConf: Configuration)
   private def getFileSize(filePath: String): Option[Long] = {
     val path = new Path(filePath)
     val fs = path.getFileSystem(hadoopConf)
+    getFileSize(fs, path)
+  }
+
+  /**
+   * Get the size of the file expected to have been written by a worker.
+   * This supports the XAttr in HADOOP-17414 when the "magic committer" adds
+   * a custom HTTP header to the a zero byte marker.
+   * If the output file as returned by getFileStatus > 0 then the length if
+   * returned. For zero-byte files, the (optional) Hadoop FS API getXAttr() is
+   * invoked. If a parseable, non-negative length can be retrieved, this
+   * is returned instead of the length.
+   * @return the file size or None if the file was not found.
+   */
+  private [datasources] def getFileSize(fs: FileSystem, path: Path): Option[Long] = {
+    // the normal file status probe.
     try {
-      Some(fs.getFileStatus(path).getLen())
+      val len = fs.getFileStatus(path).getLen
+      if (len > 0) {
+        return Some(len)
+      }
     } catch {
       case e: FileNotFoundException =>
-        // may arise against eventually consistent object stores
+        // may arise against eventually consistent object stores.
         logDebug(s"File $path is not yet visible", e)
-        None
+        return None
+    }
+
+    // Output File Size is 0. Look to see if it has an attribute
+    // declaring a future-file-length.
+    // Failure of API call, parsing, invalid value all return the
+    // 0 byte length.
+
+    var len = 0L
+    try {
+      val attr = fs.getXAttr(path, BasicWriteJobStatsTracker.FILE_LENGTH_XATTR)
+      if (attr != null && attr.nonEmpty) {
+        val str = new String(attr, StandardCharsets.UTF_8)
+        logInfo(s"File Length statistics for $path retrieved from XAttr: $str")
+        // a non-empty header was found. parse to a long via the java class
+        val l = java.lang.Long.parseLong(str)
+        if (l > 0) {
+          len = l
+        } else {
+          logDebug("Ignoring negative value in Xattr file length")
+        }
+      }
+    } catch {
+      case e: NumberFormatException =>
+        // warn but don't dump the whole stack
+        logInfo(s"Failed to parse" +

Review comment:
       OK. I'd say at info as warn seems overkill for progress report issues.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] tgravescs commented on a change in pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
tgravescs commented on a change in pull request #30714:
URL: https://github.com/apache/spark/pull/30714#discussion_r566884190



##########
File path: docs/cloud-integration.md
##########
@@ -245,17 +260,23 @@ mydataframe.write.format("parquet").save("s3a://bucket/destination")
 
 More details on these committers can be found in the latest Hadoop documentation.
 
+Note: depending upon the committer used, in-progress statistics may be
+under-reported with Hadoop versions before 3.3.1.
+Are

Review comment:
       extra "Are" here - remove




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HeartSaVioR commented on a change in pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on a change in pull request #30714:
URL: https://github.com/apache/spark/pull/30714#discussion_r576400275



##########
File path: docs/cloud-integration.md
##########
@@ -49,7 +49,6 @@ They cannot be used as a direct replacement for a cluster filesystem such as HDF
 
 Key differences are:
 
-* Changes to stored objects may not be immediately visible, both in directory listings and actual data access.

Review comment:
       @steveloughran Thanks for the update!
   @dongjoon-hyun Could you please check whether the comments and new change make sense to you? Thanks in advance!




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-742815051


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37193/
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-779318834


   **[Test build #135156 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135156/testReport)** for PR 30714 at commit [`4b08f49`](https://github.com/apache/spark/commit/4b08f491ae98dc9b035d72daf0e2d817728537b8).


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-780572456


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39777/
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] steveloughran commented on a change in pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #30714:
URL: https://github.com/apache/spark/pull/30714#discussion_r577537838



##########
File path: docs/cloud-integration.md
##########
@@ -176,12 +181,19 @@ different stores and connectors when renaming directories:
 | Amazon S3     | s3a       | Unsafe                  | O(data) |
 | Azure Storage | wasb      | Safe                    | O(files) |
 | Azure Datalake Gen 2 | abfs | Safe                  | O(1) |
-| Google Cloud Storage | gs        | Safe                    | O(1) |
+| Google Cloud Storage | gs        | Mixed                    | O(files) |
 
 1. As storing temporary files can run up charges; delete
 directories called `"_temporary"` on a regular basis.
 1. For AWS S3, set a limit on how long multipart uploads can remain outstanding.
 This avoids incurring bills from incompleted uploads.
+1. For Google cloud, directory rename is file-by-file. Consider using the v2 committer
+and only write code which generates idemportent output -including filenames,
+as it is *no more unsafe* than the v1 committer, and faster.
+
+```
+spark.hadoop.mapreduce.fileoutputcommitter.algorithm.version 2
+```

Review comment:
       done




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-742830904


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/37193/
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-769913553






----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-780515556


   **[Test build #135196 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135196/testReport)** for PR 30714 at commit [`81c0a52`](https://github.com/apache/spark/commit/81c0a5244fa5a17184b59df9f08aed0d20868276).


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a change in pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #30714:
URL: https://github.com/apache/spark/pull/30714#discussion_r540509604



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/BasicWriteStatsTracker.scala
##########
@@ -170,6 +222,8 @@ object BasicWriteJobStatsTracker {
   private val NUM_OUTPUT_BYTES_KEY = "numOutputBytes"
   private val NUM_OUTPUT_ROWS_KEY = "numOutputRows"
   private val NUM_PARTS_KEY = "numParts"
+  /** XAttr key of the data length header added in HADOOP-17414. */

Review comment:
       `HADOOP-17414` seems to be not committed yet and is still targeting 3.3.1.
   Shall we hold one this PR until we have an official Hadoop release?




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-742830904


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/37193/
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-765483327


   Kubernetes integration test status failure
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38958/
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-769913552






----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-779390484


   Kubernetes integration test status failure
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39737/
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-779346684


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39737/
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HeartSaVioR commented on a change in pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on a change in pull request #30714:
URL: https://github.com/apache/spark/pull/30714#discussion_r575867762



##########
File path: docs/cloud-integration.md
##########
@@ -49,7 +49,6 @@ They cannot be used as a direct replacement for a cluster filesystem such as HDF
 
 Key differences are:
 
-* Changes to stored objects may not be immediately visible, both in directory listings and actual data access.

Review comment:
       The line deletion is based on the fact S3 is now strong consistency, but that also means we only consider these three vendors. Why not explicitly saying it and update the description tied to these vendors? We would never be able to consider all possible implementations and for some minority the description may be wrong. Let's just make it 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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] tgravescs commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-781392415


   merged to master, thanks @steveloughran @dongjoon-hyun @HeartSaVioR 


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] steveloughran commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
steveloughran commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-779311023


   I've looked at the stores for which hadoop-trunk has connectors, looks like only openstack swift is inconsistent. Moved the details on consistency down, called out swift and said "consult the docs". After all, alternative implementations of the swift API (I'm thinking IBM's work) probably is consistent. 


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HeartSaVioR commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-779437146


   The doc change looks good to me, but I don't feel qualified to review the code. As @tgravescs already reviewed and approved, it sounds more natural if he can make "sign-off".
   
   @tgravescs Kindly reminder. Thanks!


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] steveloughran commented on a change in pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #30714:
URL: https://github.com/apache/spark/pull/30714#discussion_r566857114



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/BasicWriteStatsTracker.scala
##########
@@ -66,14 +67,65 @@ class BasicWriteTaskStatsTracker(hadoopConf: Configuration)
   private def getFileSize(filePath: String): Option[Long] = {
     val path = new Path(filePath)
     val fs = path.getFileSystem(hadoopConf)
+    getFileSize(fs, path)
+  }
+
+  /**
+   * Get the size of the file expected to have been written by a worker.
+   * This supports the XAttr in HADOOP-17414 when the "magic committer" adds
+   * a custom HTTP header to the a zero byte marker.
+   * If the output file as returned by getFileStatus > 0 then the length if
+   * returned. For zero-byte files, the (optional) Hadoop FS API getXAttr() is
+   * invoked. If a parseable, non-negative length can be retrieved, this
+   * is returned instead of the length.
+   * @return the file size or None if the file was not found.
+   */
+  private [datasources] def getFileSize(fs: FileSystem, path: Path): Option[Long] = {
+    // the normal file status probe.
     try {
-      Some(fs.getFileStatus(path).getLen())
+      val len = fs.getFileStatus(path).getLen
+      if (len > 0) {
+        return Some(len)
+      }
     } catch {
       case e: FileNotFoundException =>
-        // may arise against eventually consistent object stores
+        // may arise against eventually consistent object stores.
         logDebug(s"File $path is not yet visible", e)
-        None
+        return None
+    }
+
+    // Output File Size is 0. Look to see if it has an attribute
+    // declaring a future-file-length.
+    // Failure of API call, parsing, invalid value all return the
+    // 0 byte length.
+
+    var len = 0L
+    try {
+      val attr = fs.getXAttr(path, BasicWriteJobStatsTracker.FILE_LENGTH_XATTR)
+      if (attr != null && attr.nonEmpty) {
+        val str = new String(attr, StandardCharsets.UTF_8)
+        logInfo(s"File Length statistics for $path retrieved from XAttr: $str")

Review comment:
       can put to debug.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-781758407


   I was here too! 😃 


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] steveloughran commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
steveloughran commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-781297083


   @tgravescs can you hit the merge button before some other change breaks this PR? thanks


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] steveloughran commented on a change in pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #30714:
URL: https://github.com/apache/spark/pull/30714#discussion_r566902358



##########
File path: docs/cloud-integration.md
##########
@@ -245,17 +260,23 @@ mydataframe.write.format("parquet").save("s3a://bucket/destination")
 
 More details on these committers can be found in the latest Hadoop documentation.
 
+Note: depending upon the committer used, in-progress statistics may be
+under-reported with Hadoop versions before 3.3.1.
+Are
 ## Further Reading
 
 Here is the documentation on the standard connectors both from Apache and the cloud providers.
 
-* [OpenStack Swift](https://hadoop.apache.org/docs/current/hadoop-openstack/index.html).
+* [OpenStack Swift](https://hadoop.apache.org/docs/current/hadoop-openstack/index.html). Are

Review comment:
       sorry; my fault for having dictation turned on and not paying attention to what the music in the background was saying...




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] steveloughran commented on a change in pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #30714:
URL: https://github.com/apache/spark/pull/30714#discussion_r576286426



##########
File path: docs/cloud-integration.md
##########
@@ -49,7 +49,6 @@ They cannot be used as a direct replacement for a cluster filesystem such as HDF
 
 Key differences are:
 
-* Changes to stored objects may not be immediately visible, both in directory listings and actual data access.

Review comment:
       reviewed the docs, huawei cos and tencent obs both seem consistent. swift is now the outlier. Moving the issue into a paragraph in the "consistency" section




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a change in pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #30714:
URL: https://github.com/apache/spark/pull/30714#discussion_r576458517



##########
File path: docs/cloud-integration.md
##########
@@ -176,12 +181,19 @@ different stores and connectors when renaming directories:
 | Amazon S3     | s3a       | Unsafe                  | O(data) |
 | Azure Storage | wasb      | Safe                    | O(files) |
 | Azure Datalake Gen 2 | abfs | Safe                  | O(1) |
-| Google Cloud Storage | gs        | Safe                    | O(1) |
+| Google Cloud Storage | gs        | Mixed                    | O(files) |
 
 1. As storing temporary files can run up charges; delete
 directories called `"_temporary"` on a regular basis.
 1. For AWS S3, set a limit on how long multipart uploads can remain outstanding.
 This avoids incurring bills from incompleted uploads.
+1. For Google cloud, directory rename is file-by-file. Consider using the v2 committer
+and only write code which generates idemportent output -including filenames,
+as it is *no more unsafe* than the v1 committer, and faster.
+
+```
+spark.hadoop.mapreduce.fileoutputcommitter.algorithm.version 2
+```

Review comment:
       We can remove this repetition (line 194 ~ 196) because we already have this at the beginning of this section (line 149)




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-768235885


   cc @tgravescs @mridulm FYI


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-765428436


   **[Test build #134371 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134371/testReport)** for PR 30714 at commit [`c680138`](https://github.com/apache/spark/commit/c680138c299e721df3e9bc51b90c0aacaa059e24).


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-742897506


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/132588/
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-742830885


   Kubernetes integration test status failure
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37193/
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-765428436


   **[Test build #134371 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134371/testReport)** for PR 30714 at commit [`c680138`](https://github.com/apache/spark/commit/c680138c299e721df3e9bc51b90c0aacaa059e24).


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-765516098


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38958/
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-742896891


   **[Test build #132588 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132588/testReport)** for PR 30714 at commit [`132a2e9`](https://github.com/apache/spark/commit/132a2e989778493551e3d84bf0a37a7dd69d013d).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-779396707


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39737/
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-765594768


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134371/
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-742897506


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/132588/
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a change in pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #30714:
URL: https://github.com/apache/spark/pull/30714#discussion_r576459756



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/BasicWriteTaskStatsTrackerSuite.scala
##########
@@ -221,4 +222,127 @@ class BasicWriteTaskStatsTrackerSuite extends SparkFunSuite {
     write(file, data2)
   }
 
+  /**
+   * Does a length specified in the XAttr header get picked up?
+   */
+  test("Xattr sourced length") {

Review comment:
       `Xattr` -> `XAttr`?




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-766402513


   Thank you for updating. I'm looking forward to using Hadoop 3.3.1!


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] tgravescs commented on a change in pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
tgravescs commented on a change in pull request #30714:
URL: https://github.com/apache/spark/pull/30714#discussion_r566154624



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/BasicWriteStatsTracker.scala
##########
@@ -66,14 +67,65 @@ class BasicWriteTaskStatsTracker(hadoopConf: Configuration)
   private def getFileSize(filePath: String): Option[Long] = {
     val path = new Path(filePath)
     val fs = path.getFileSystem(hadoopConf)
+    getFileSize(fs, path)
+  }
+
+  /**
+   * Get the size of the file expected to have been written by a worker.
+   * This supports the XAttr in HADOOP-17414 when the "magic committer" adds
+   * a custom HTTP header to the a zero byte marker.
+   * If the output file as returned by getFileStatus > 0 then the length if
+   * returned. For zero-byte files, the (optional) Hadoop FS API getXAttr() is
+   * invoked. If a parseable, non-negative length can be retrieved, this
+   * is returned instead of the length.
+   * @return the file size or None if the file was not found.
+   */
+  private [datasources] def getFileSize(fs: FileSystem, path: Path): Option[Long] = {
+    // the normal file status probe.
     try {
-      Some(fs.getFileStatus(path).getLen())
+      val len = fs.getFileStatus(path).getLen
+      if (len > 0) {
+        return Some(len)
+      }
     } catch {
       case e: FileNotFoundException =>
-        // may arise against eventually consistent object stores
+        // may arise against eventually consistent object stores.
         logDebug(s"File $path is not yet visible", e)
-        None
+        return None
+    }
+
+    // Output File Size is 0. Look to see if it has an attribute
+    // declaring a future-file-length.
+    // Failure of API call, parsing, invalid value all return the
+    // 0 byte length.
+
+    var len = 0L
+    try {
+      val attr = fs.getXAttr(path, BasicWriteJobStatsTracker.FILE_LENGTH_XATTR)
+      if (attr != null && attr.nonEmpty) {
+        val str = new String(attr, StandardCharsets.UTF_8)
+        logInfo(s"File Length statistics for $path retrieved from XAttr: $str")
+        // a non-empty header was found. parse to a long via the java class
+        val l = java.lang.Long.parseLong(str)
+        if (l > 0) {
+          len = l
+        } else {
+          logDebug("Ignoring negative value in Xattr file length")
+        }
+      }
+    } catch {
+      case e: NumberFormatException =>
+        // warn but don't dump the whole stack
+        logInfo(s"Failed to parse" +

Review comment:
       do we want this warn instead or perhaps just extend saying file stats may be wrong?




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a change in pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #30714:
URL: https://github.com/apache/spark/pull/30714#discussion_r576459590



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/BasicWriteStatsTracker.scala
##########
@@ -66,14 +67,66 @@ class BasicWriteTaskStatsTracker(hadoopConf: Configuration)
   private def getFileSize(filePath: String): Option[Long] = {
     val path = new Path(filePath)
     val fs = path.getFileSystem(hadoopConf)
+    getFileSize(fs, path)
+  }
+
+  /**
+   * Get the size of the file expected to have been written by a worker.
+   * This supports the XAttr in HADOOP-17414 when the "magic committer" adds
+   * a custom HTTP header to the a zero byte marker.
+   * If the output file as returned by getFileStatus > 0 then the length if
+   * returned. For zero-byte files, the (optional) Hadoop FS API getXAttr() is
+   * invoked. If a parseable, non-negative length can be retrieved, this
+   * is returned instead of the length.
+   * @return the file size or None if the file was not found.
+   */
+  private [datasources] def getFileSize(fs: FileSystem, path: Path): Option[Long] = {
+    // the normal file status probe.
     try {
-      Some(fs.getFileStatus(path).getLen())
+      val len = fs.getFileStatus(path).getLen
+      if (len > 0) {
+        return Some(len)
+      }
     } catch {
       case e: FileNotFoundException =>
-        // may arise against eventually consistent object stores
+        // may arise against eventually consistent object stores.
         logDebug(s"File $path is not yet visible", e)
-        None
+        return None
+    }
+
+    // Output File Size is 0. Look to see if it has an attribute
+    // declaring a future-file-length.
+    // Failure of API call, parsing, invalid value all return the
+    // 0 byte length.
+
+    var len = 0L
+    try {
+      val attr = fs.getXAttr(path, BasicWriteJobStatsTracker.FILE_LENGTH_XATTR)
+      if (attr != null && attr.nonEmpty) {
+        val str = new String(attr, StandardCharsets.UTF_8)
+        logDebug(s"File Length statistics for $path retrieved from XAttr: $str")
+        // a non-empty header was found. parse to a long via the java class
+        val l = java.lang.Long.parseLong(str)
+        if (l > 0) {
+          len = l
+        } else {
+          logDebug("Ignoring negative value in Xattr file length")
+        }
+      }
+    } catch {
+      case e: NumberFormatException =>
+        // warn but don't dump the whole stack
+        logInfo(s"Failed to parse" +
+          s" ${BasicWriteJobStatsTracker.FILE_LENGTH_XATTR}:$e;" +
+          s" bytes written may be under-reported");
+      case e: UnsupportedOperationException =>
+        // this is not unusual; ignore
+        logDebug(s"XAttr not supported on path $path", e);
+      case e: Exception =>
+        // Something else. Log at debug and continue.
+        logDebug(s"Xattr processing failure on $path", e);

Review comment:
       `Xattr` -> `XAttr`?




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] steveloughran commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
steveloughran commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-767777521


   ok, Hadoop-side PR is in trunk; verifying backport compiles and new tests work then it'll be in branch-3.3. 
   
   This patch is ready for final review/merge


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HeartSaVioR commented on a change in pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on a change in pull request #30714:
URL: https://github.com/apache/spark/pull/30714#discussion_r575868795



##########
File path: docs/cloud-integration.md
##########
@@ -49,7 +49,6 @@ They cannot be used as a direct replacement for a cluster filesystem such as HDF
 
 Key differences are:
 
-* Changes to stored objects may not be immediately visible, both in directory listings and actual data access.

Review comment:
       Or, what about leaving this line as it is (so that the description enumerates all "possible" issues on object store considering beyond the big three), and elaborate which key differences are no longer in effect with strong consistency in consistency section?




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-780536822


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39777/
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-779441425


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135156/
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] tgravescs commented on a change in pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
tgravescs commented on a change in pull request #30714:
URL: https://github.com/apache/spark/pull/30714#discussion_r566884389



##########
File path: docs/cloud-integration.md
##########
@@ -245,17 +260,23 @@ mydataframe.write.format("parquet").save("s3a://bucket/destination")
 
 More details on these committers can be found in the latest Hadoop documentation.
 
+Note: depending upon the committer used, in-progress statistics may be
+under-reported with Hadoop versions before 3.3.1.
+Are
 ## Further Reading
 
 Here is the documentation on the standard connectors both from Apache and the cloud providers.
 
-* [OpenStack Swift](https://hadoop.apache.org/docs/current/hadoop-openstack/index.html).
+* [OpenStack Swift](https://hadoop.apache.org/docs/current/hadoop-openstack/index.html). Are

Review comment:
       extra Are




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a change in pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #30714:
URL: https://github.com/apache/spark/pull/30714#discussion_r567202719



##########
File path: docs/cloud-integration.md
##########
@@ -49,7 +49,6 @@ They cannot be used as a direct replacement for a cluster filesystem such as HDF
 
 Key differences are:
 
-* Changes to stored objects may not be immediately visible, both in directory listings and actual data access.

Review comment:
       Shall we remove [line 60](https://github.com/apache/spark/pull/30714/files#diff-a0f56e11a3171477d68df69ab5f93eb12c8cc899cceaceffdc52180c87d677c5R60) together?
   > 1. The output of work may not be immediately visible to a follow-on query.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] steveloughran commented on a change in pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #30714:
URL: https://github.com/apache/spark/pull/30714#discussion_r540868641



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/BasicWriteStatsTracker.scala
##########
@@ -170,6 +222,8 @@ object BasicWriteJobStatsTracker {
   private val NUM_OUTPUT_BYTES_KEY = "numOutputBytes"
   private val NUM_OUTPUT_ROWS_KEY = "numOutputRows"
   private val NUM_PARTS_KEY = "numParts"
+  /** XAttr key of the data length header added in HADOOP-17414. */

Review comment:
       I'm developing them in sync so they can be done end to end. Yes, it makes sense for the Hadoop one to be committed,  first -but if reviewing the spark one finds changes which need to be done there (is the header a good name?) then an initial review of this PR will make sure that they are lined up




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-765460725


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38958/
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] tgravescs commented on a change in pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
tgravescs commented on a change in pull request #30714:
URL: https://github.com/apache/spark/pull/30714#discussion_r566153947



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/BasicWriteStatsTracker.scala
##########
@@ -66,14 +67,65 @@ class BasicWriteTaskStatsTracker(hadoopConf: Configuration)
   private def getFileSize(filePath: String): Option[Long] = {
     val path = new Path(filePath)
     val fs = path.getFileSystem(hadoopConf)
+    getFileSize(fs, path)
+  }
+
+  /**
+   * Get the size of the file expected to have been written by a worker.
+   * This supports the XAttr in HADOOP-17414 when the "magic committer" adds
+   * a custom HTTP header to the a zero byte marker.
+   * If the output file as returned by getFileStatus > 0 then the length if
+   * returned. For zero-byte files, the (optional) Hadoop FS API getXAttr() is
+   * invoked. If a parseable, non-negative length can be retrieved, this
+   * is returned instead of the length.
+   * @return the file size or None if the file was not found.
+   */
+  private [datasources] def getFileSize(fs: FileSystem, path: Path): Option[Long] = {
+    // the normal file status probe.
     try {
-      Some(fs.getFileStatus(path).getLen())
+      val len = fs.getFileStatus(path).getLen
+      if (len > 0) {
+        return Some(len)
+      }
     } catch {
       case e: FileNotFoundException =>
-        // may arise against eventually consistent object stores
+        // may arise against eventually consistent object stores.
         logDebug(s"File $path is not yet visible", e)
-        None
+        return None
+    }
+
+    // Output File Size is 0. Look to see if it has an attribute
+    // declaring a future-file-length.
+    // Failure of API call, parsing, invalid value all return the
+    // 0 byte length.
+
+    var len = 0L
+    try {
+      val attr = fs.getXAttr(path, BasicWriteJobStatsTracker.FILE_LENGTH_XATTR)
+      if (attr != null && attr.nonEmpty) {
+        val str = new String(attr, StandardCharsets.UTF_8)
+        logInfo(s"File Length statistics for $path retrieved from XAttr: $str")

Review comment:
       do we want this info or just debug?  seems like I would only care if stats didn't come out but maybe its more useful...




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] steveloughran commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
steveloughran commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-781395712


   thanks!


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] asfgit closed pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #30714:
URL: https://github.com/apache/spark/pull/30714


   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-765591739


   **[Test build #134371 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134371/testReport)** for PR 30714 at commit [`c680138`](https://github.com/apache/spark/commit/c680138c299e721df3e9bc51b90c0aacaa059e24).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-780515556


   **[Test build #135196 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135196/testReport)** for PR 30714 at commit [`81c0a52`](https://github.com/apache/spark/commit/81c0a5244fa5a17184b59df9f08aed0d20868276).


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] steveloughran commented on a change in pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #30714:
URL: https://github.com/apache/spark/pull/30714#discussion_r567444807



##########
File path: docs/cloud-integration.md
##########
@@ -49,7 +49,6 @@ They cannot be used as a direct replacement for a cluster filesystem such as HDF
 
 Key differences are:
 
-* Changes to stored objects may not be immediately visible, both in directory listings and actual data access.

Review comment:
       I worry about openstack swift. The original version was woefully inconsistent. Not been near it long enough to know what's change. with swift I could consistently replicate an inconsistency in a few lines
   
   1. write a file of length, say, 1KB
   2. overwrite with a shorter dataset, e.g. 512 bytes
   3. open file
   4. read from 0-128: get the new data
   5. read from 768-1024: get the old data
   
   No S3 implementation that I know of (i.e. the open source or commercial ones) are inconsistent, nor have they ever had the 404 caching issue. But people should still be aware of the risk.
   
   oh, and I haven' t played with any of the chinese cloud stores for which connectors now exist. So I can't make statements there. All I can say is the "big three outside china" are consistent.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a change in pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #30714:
URL: https://github.com/apache/spark/pull/30714#discussion_r567202719



##########
File path: docs/cloud-integration.md
##########
@@ -49,7 +49,6 @@ They cannot be used as a direct replacement for a cluster filesystem such as HDF
 
 Key differences are:
 
-* Changes to stored objects may not be immediately visible, both in directory listings and actual data access.

Review comment:
       Shall we remove line 60 together?
   > 1. The output of work may not be immediately visible to a follow-on query.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-780556446


   Kubernetes integration test status success
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39777/
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] steveloughran commented on a change in pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #30714:
URL: https://github.com/apache/spark/pull/30714#discussion_r576208259



##########
File path: docs/cloud-integration.md
##########
@@ -49,7 +49,6 @@ They cannot be used as a direct replacement for a cluster filesystem such as HDF
 
 Key differences are:
 
-* Changes to stored objects may not be immediately visible, both in directory listings and actual data access.

Review comment:
       > (Probably you're considering more like S3 compatible implementations, but you've also mentioned you don't consider the chinese cloud stores so it can't be exhaustive.)
   
   I don't test them; I don't know their consistency




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-780698192


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135196/
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #30714: [SPARK-33739] [SQL] Jobs committed through the S3A Magic committer don't track bytes

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #30714:
URL: https://github.com/apache/spark/pull/30714#issuecomment-766402513


   Thank you for updating. I'm looking forward to using Hadoop 3.3.1!


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org