You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by ypcat <gi...@git.apache.org> on 2015/03/16 10:13:44 UTC
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
GitHub user ypcat opened a pull request:
https://github.com/apache/spark/pull/5042
[SPARK-6352] [SQL] Add DirectParquetOutputCommitter
Add a DirectParquetOutputCommitter class that skips _temporary directory when saving to s3. Add new config value "spark.sql.parquet.useDirectParquetOutputCommitter" (default false) to choose between the default output committer.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ypcat/spark spark-6352
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/5042.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #5042
----
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-82226815
[Test build #28712 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28712/consoleFull) for PR 5042 at commit [`0d540b9`](https://github.com/apache/spark/commit/0d540b9cfc03fc71d228616123f8cad4602e8f14).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/5042#discussion_r28238198
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/parquet/ParquetTableOperations.scala ---
@@ -403,6 +405,26 @@ private[parquet] class AppendingParquetOutputFormat(offset: Int)
private def getTaskAttemptID(context: TaskAttemptContext): TaskAttemptID = {
context.getClass.getMethod("getTaskAttemptID").invoke(context).asInstanceOf[TaskAttemptID]
}
+
+ // override to create output committer from configuration
+ override def getOutputCommitter(context: TaskAttemptContext): OutputCommitter = {
+ if (committer == null) {
+ val output = getOutputPath(context)
+ val cls = context.getConfiguration.getClass("spark.sql.parquet.output.committer.class",
+ classOf[ParquetOutputCommitter], classOf[ParquetOutputCommitter])
+ val ctor = cls.getDeclaredConstructor(classOf[Path], classOf[TaskAttemptContext])
+ committer = ctor.newInstance(output, context).asInstanceOf[ParquetOutputCommitter]
+ }
+ committer
+ }
+
+ // FileOutputFormat.getOutputPath takes JobConf in hadoop-1 but JobContext in hadoop-2
+ private def getOutputPath(context: TaskAttemptContext): Path = {
+ context.getConfiguration().get("mapred.output.dir") match {
--- End diff --
Oh I see, this is for Hadoop 1 compatibility.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-82183129
[Test build #28709 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28709/consoleFull) for PR 5042 at commit [`c42468c`](https://github.com/apache/spark/commit/c42468c9b207edd995524afc2ebb1f723e375d20).
* This patch **fails RAT tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-82183087
[Test build #28709 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28709/consoleFull) for PR 5042 at commit [`c42468c`](https://github.com/apache/spark/commit/c42468c9b207edd995524afc2ebb1f723e375d20).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-82191285
[Test build #28712 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28712/consoleFull) for PR 5042 at commit [`0d540b9`](https://github.com/apache/spark/commit/0d540b9cfc03fc71d228616123f8cad4602e8f14).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-84939649
[Test build #28992 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28992/consoleFull) for PR 5042 at commit [`9ae7545`](https://github.com/apache/spark/commit/9ae7545701f522702f2d0240367fc6fba06b7c26).
* This patch **does not merge cleanly**.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-81794422
[Test build #28653 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28653/consoleFull) for PR 5042 at commit [`769bd67`](https://github.com/apache/spark/commit/769bd6737acc3f13a8678688e842e902fc38e802).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-81523383
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-82226852
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28712/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by ypcat <gi...@git.apache.org>.
Github user ypcat commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-93313595
New PR is https://github.com/apache/spark/pull/5525
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by ash211 <gi...@git.apache.org>.
Github user ash211 commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-93224863
I think it's best to open a new PR
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by ypcat <gi...@git.apache.org>.
Github user ypcat commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-92183165
@liancheng this DirectParquetOutputCommitter is based on DirectOutputCommitter in this [thread](https://mail-archives.apache.org/mod_mbox/spark-user/201502.mbox/%3CCANGvG8qfmNPkb2vhL0-t9bi3d39b3ROhs6DhO+PODRRbzh-9Tg@mail.gmail.com%3E) and was intended to use on s3. I was not aware of the problem of speculation. I think we can either:
1. exclude DirectParquetOutputCommitter and let user implement their own like DirectOutputCommitter.
2. write down the consequences in documentation as @JoshRosen described.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-84940461
[Test build #28993 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28993/consoleFull) for PR 5042 at commit [`e17bf47`](https://github.com/apache/spark/commit/e17bf474ab3db112958cf67318d13305bde60788).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/5042
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-81786293
[Test build #28650 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28650/consoleFull) for PR 5042 at commit [`769bd67`](https://github.com/apache/spark/commit/769bd6737acc3f13a8678688e842e902fc38e802).
* This patch **fails RAT tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class DirectParquetOutputCommitter(outputPath: Path, context: TaskAttemptContext)`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-81786112
[Test build #28650 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28650/consoleFull) for PR 5042 at commit [`769bd67`](https://github.com/apache/spark/commit/769bd6737acc3f13a8678688e842e902fc38e802).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by ash211 <gi...@git.apache.org>.
Github user ash211 commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-81768782
Jenkins this is ok to test
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-92441217
We can't change this now, but note that the commit message / PR description here is slightly out of date w.r.t. the actual changes being committed, since we now use the `spark.sql.parquet.output.committer.class` configuration instead of `spark.sql.parquet.useDirectParquetOutputCommitter`.
We may want to document this configuration at some point; if we do, I think the right place is the Parquet Configuration section in the SQL programming guide: https://spark.apache.org/docs/latest/sql-programming-guide.html#configuration
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-81794652
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28653/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-84971396
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28992/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-91870687
Hey @ypcat, sorry for the late review. If I understand this correctly, with `DirectParquetOutputCommitter`, all tasks writes to the final output directory directly. Then what if speculative tasks are scheduled? This may end up with two tasks trying to write to the same file simultaneously, right?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-81786321
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28650/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-81794613
[Test build #28653 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28653/consoleFull) for PR 5042 at commit [`769bd67`](https://github.com/apache/spark/commit/769bd6737acc3f13a8678688e842e902fc38e802).
* This patch **fails RAT tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class DirectParquetOutputCommitter(outputPath: Path, context: TaskAttemptContext)`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-91905439
@liancheng, if this is intended only for writing directly to S3, then I think we purposely want to avoid writing to `_temporary` because the move / rename of the temporary file to the final file is expensive and because S3 PUT operations are atomic. If you have two tasks racing to write to the same file, then the last writer should win and we shouldn't have to worry about partially-written files. As with DirectOutputCommitter, this is _not_ safe for use on filesystems where file creation / overwrite isn't atomic, such as HDFS, but that's not a use-case that we have to worry about here (I think this is only for use by advanced users who know that they're writing directly to S3).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-84965366
[Test build #28993 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28993/consoleFull) for PR 5042 at commit [`e17bf47`](https://github.com/apache/spark/commit/e17bf474ab3db112958cf67318d13305bde60788).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-84971389
[Test build #28992 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28992/consoleFull) for PR 5042 at commit [`9ae7545`](https://github.com/apache/spark/commit/9ae7545701f522702f2d0240367fc6fba06b7c26).
* This patch **passes all tests**.
* This patch **does not merge cleanly**.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by ypcat <gi...@git.apache.org>.
Github user ypcat commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-92657249
@JoshRosen I updated PR description. It looks like the parquet code base I used was too old that it did not include the ParquetOutputFormat.ENABLE_JOB_SUMMARY thing. I will add support to this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-92357505
@JoshRosen @ypcat Thanks for the detailed explanation, I didn't realize S3 PUT operations are atomic at first. This LGTM. Merging to master.
@ypcat Thanks again for working on this!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-91874354
At least we should state explicitly that speculative tasks should be disabled when using the direct output committer.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-93061589
It looks like this patch broke the Hadoop 1.x build because the test code uses `Configuration.unset`, which isn't present in Hadoop 1.x:
```
[info] Compiling 1 Scala source to /home/jenkins/workspace/Spark-Master-SBT/AMPLAB_JENKINS_BUILD_PROFILE/hadoop1.0/label/centos/examples/target/scala-2.10/classes...
[error] /home/jenkins/workspace/Spark-Master-SBT/AMPLAB_JENKINS_BUILD_PROFILE/hadoop1.0/label/centos/sql/core/src/test/scala/org/apache/spark/sql/parquet/ParquetIOSuite.scala:402: value unset is not a member of org.apache.hadoop.conf.Configuration
[error] configuration.unset("spark.sql.parquet.output.committer.class")
[error] ^
[error] one error found
```
To unbreak the build, I'm going to revert this patch and re-open the JIRA. Let's open a new PR to fix this. At the same time, we can also address the ENABLE_JOB_SUMMARY flag.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/5042#discussion_r28237317
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/parquet/ParquetTableOperations.scala ---
@@ -403,6 +405,26 @@ private[parquet] class AppendingParquetOutputFormat(offset: Int)
private def getTaskAttemptID(context: TaskAttemptContext): TaskAttemptID = {
context.getClass.getMethod("getTaskAttemptID").invoke(context).asInstanceOf[TaskAttemptID]
}
+
+ // override to create output committer from configuration
+ override def getOutputCommitter(context: TaskAttemptContext): OutputCommitter = {
+ if (committer == null) {
+ val output = getOutputPath(context)
+ val cls = context.getConfiguration.getClass("spark.sql.parquet.output.committer.class",
+ classOf[ParquetOutputCommitter], classOf[ParquetOutputCommitter])
+ val ctor = cls.getDeclaredConstructor(classOf[Path], classOf[TaskAttemptContext])
+ committer = ctor.newInstance(output, context).asInstanceOf[ParquetOutputCommitter]
+ }
+ committer
+ }
+
+ // FileOutputFormat.getOutputPath takes JobConf in hadoop-1 but JobContext in hadoop-2
+ private def getOutputPath(context: TaskAttemptContext): Path = {
+ context.getConfiguration().get("mapred.output.dir") match {
--- End diff --
Should we check for `mapreduce.output.fileoutputformat.outputdir` instead?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-92442370
Actually, on giving this a closer look I'm not sure whether this faithfully respects all of the possible Parquet configurations for controlling OutputCommitter behavior. For instance, it looks like there's a `ParquetOutputFormat.ENABLE_JOB_SUMMARY` configuration that can be used to suppress the writing of the summary file (https://github.com/Parquet/parquet-mr/blame/fa8957d7939b59e8d391fa17000b34e865de015d/parquet-hadoop/src/main/java/parquet/hadoop/ParquetOutputCommitter.java#L48). I don't really know much about Parquet, so I can't really speak to whether this is a configuration that would be commonly used, but we should probably try to support it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by ypcat <gi...@git.apache.org>.
Github user ypcat commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-84941735
I change the design to allow more general usage. User can set "spark.sql.parquet.output.committer.class" to a class extending ParquetOutputFormat.
I still include the implementation of DirectParquetOutputCommitter, if it is unwanted, we can move it to be only used in test case.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-84965374
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28993/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by ypcat <gi...@git.apache.org>.
Github user ypcat commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-93214702
I cannot find a way to unset a config value in hadoop 1.x API. The closest thing is to set it to a default value, which I think should be fine in test code.
And I found I cannot add more commits to this PR since it is closed. Should we reopen it or use a new PR?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by shaneknapp <gi...@git.apache.org>.
Github user shaneknapp commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-81790432
jenkins, test this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6352] [SQL] Add DirectParquetOutputComm...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5042#issuecomment-82183137
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28709/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org