You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by Udbhav30 <gi...@git.apache.org> on 2018/10/31 13:34:57 UTC
[GitHub] spark pull request #22906: [SPARK-25895][Core]Adding testcase to compare Lz4...
GitHub user Udbhav30 opened a pull request:
https://github.com/apache/spark/pull/22906
[SPARK-25895][Core]Adding testcase to compare Lz4 and Zstd compression
## What changes were proposed in this pull request?
Added a UT to verify Zstd compression algorithm has better compression ratio than lz4 in shuffle read/write and spill
## How was this patch tested?
Added UT
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Udbhav30/spark perfTest
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/22906.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 #22906
----
commit c0e598dacf1e3237bffb4c3a801fab89cc8eb37e
Author: Udbhav Agrawal <u....@...>
Date: 2018-10-31T13:29:57Z
Adding testcase to compare Lz4 and Zstd compression
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22906: [SPARK-25895][Core]Adding testcase to compare Lz4 and Zs...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22906
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22906: [SPARK-25895][Core]Adding testcase to compare Lz4 and Zs...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/22906
Ping @Udbhav30
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22906: [SPARK-25895][Core]Adding testcase to compare Lz4 and Zs...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/22906
Benchmarks aren't run with tests right? That's fine. I am still not sure Spark is the best place for this.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22906: [SPARK-25895][Core]Adding testcase to compare Lz4...
Posted by Udbhav30 <gi...@git.apache.org>.
Github user Udbhav30 commented on a diff in the pull request:
https://github.com/apache/spark/pull/22906#discussion_r230269652
--- Diff: core/src/test/scala/org/apache/spark/io/CompressionCodecSuite.scala ---
@@ -128,6 +130,69 @@ class CompressionCodecSuite extends SparkFunSuite {
}
}
+ test("SPARK-25895 Zstd shuffle Read/Write/spill comparison w.r.t lz4") {
--- End diff --
Thanks for your suggestion, i will go through the benchmarks and update the PR
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22906: [SPARK-25895][Core]Adding testcase to compare Lz4 and Zs...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/22906
I don't think this tests something about the correctness of Spark though. I am not sure this is worth it.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22906: [SPARK-25895][Core]Adding testcase to compare Lz4 and Zs...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22906
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22906: [SPARK-25895][Core]Adding testcase to compare Lz4...
Posted by Udbhav30 <gi...@git.apache.org>.
Github user Udbhav30 closed the pull request at:
https://github.com/apache/spark/pull/22906
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22906: [SPARK-25895][Core]Adding testcase to compare Lz4 and Zs...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22906
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22906: [SPARK-25895][Core]Adding testcase to compare Lz4 and Zs...
Posted by Udbhav30 <gi...@git.apache.org>.
Github user Udbhav30 commented on the issue:
https://github.com/apache/spark/pull/22906
Okay I'll close this PR
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22906: [SPARK-25895][Core]Adding testcase to compare Lz4...
Posted by wangyum <gi...@git.apache.org>.
Github user wangyum commented on a diff in the pull request:
https://github.com/apache/spark/pull/22906#discussion_r229723135
--- Diff: core/src/test/scala/org/apache/spark/io/CompressionCodecSuite.scala ---
@@ -128,6 +130,69 @@ class CompressionCodecSuite extends SparkFunSuite {
}
}
+ test("SPARK-25895 Zstd shuffle Read/Write/spill comparison w.r.t lz4") {
--- End diff --
How about add a benchmark? You can check our all benchmarks here:: https://issues.apache.org/jira/browse/SPARK-25475
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org