You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by gatorsmile <gi...@git.apache.org> on 2018/09/07 19:08:18 UTC

[GitHub] spark pull request #22361: Revert [SPARK-10399] [SPARK-23879] [SPARK-23762]

GitHub user gatorsmile opened a pull request:

    https://github.com/apache/spark/pull/22361

    Revert [SPARK-10399] [SPARK-23879] [SPARK-23762]  

    ## What changes were proposed in this pull request?
    
    When running TPC-DS benchmarks on 2.4 release, we saw more than 10% performance regression on the following queries: q67, q24a and q24b. After we applying the PR https://github.com/apache/spark/pull/22338, the performance regression still exists. If we revert the changes in https://github.com/apache/spark/pull/19222, we found the performance regression was resolved. Thus, this PR is to revert the related changes for unblocking the 2.4 release. 
    
    In the future release, we still can continue the investigation and find out the root cause of the regression. 
    
    ## How was this patch tested?
    
    The existing test cases

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/gatorsmile/spark revertMemoryBlock

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/22361.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 #22361
    
----
commit 711963a98d4a46399f95b1969b7b952ae8866c52
Author: gatorsmile <ga...@...>
Date:   2018-09-07T17:38:52Z

    Revert "[SPARK-10399][SPARK-23879][FOLLOWUP][CORE] Free unused off-heap memory in MemoryBlockSuite"
    
    This reverts commit c3a86faa53c9e49efd595802adc38a6d412ce681.

commit 0504f08b671b114834f6fb5a879de62c2992421d
Author: gatorsmile <ga...@...>
Date:   2018-09-07T17:39:27Z

    Revert "[SPARK-10399][SPARK-23879][HOTFIX] Fix Java lint errors"
    
    This reverts commit b6935ffb4dfb1d9fdf36ba402ac07bd02978c012.

commit ca179acac2acd595a6062a86d05f9c2d9f0be6d2
Author: gatorsmile <ga...@...>
Date:   2018-09-07T18:42:33Z

    Revert "[SPARK-10399][CORE][SQL] Introduce multiple MemoryBlocks to choose several types of memory block"
    
    This reverts commit 4807d381bb113a5c61e6dad88202f23a8b6dd141.

commit cbe3a514e3bdc47a1fedb49695f65bb378fc57cb
Author: gatorsmile <ga...@...>
Date:   2018-09-07T18:42:52Z

    Revert "[SPARK-23762][SQL] UTF8StringBuffer uses MemoryBlock"
    
    This reverts commit 0b19122d434e39eb117ccc3174a0688c9c874d48.

----


---

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


[GitHub] spark issue #22361: Revert [SPARK-10399] [SPARK-23879] [SPARK-23762] [SPARK-...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:

    https://github.com/apache/spark/pull/22361
  
    Nico will give a talk in the upcoming spark summit: https://databricks.com/session/a-framework-for-evaluating-the-performance-and-the-correctness-of-the-spark-sql-engine 
    
    If possible, please join the talk! : )
    



---

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


[GitHub] spark issue #22361: Revert [SPARK-10399] [SPARK-23879] [SPARK-23762] [SPARK-...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

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


---

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


[GitHub] spark issue #22361: Revert [SPARK-10399] [SPARK-23879] [SPARK-23762]

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22361
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2933/
    Test PASSed.


---

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


[GitHub] spark issue #22361: Revert [SPARK-10399] [SPARK-23879] [SPARK-23762] [SPARK-...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:

    https://github.com/apache/spark/pull/22361
  
    LGTM, I'm merging it to unblock the 2.4 RC, thanks!


---

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


[GitHub] spark issue #22361: Revert [SPARK-10399] [SPARK-23879] [SPARK-23762] [SPARK-...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/22361
  
    It would be great if the information requested https://github.com/apache/spark/pull/22361#issuecomment-419607024 is shared so that other people can check it as well.


---

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


[GitHub] spark pull request #22361: Revert [SPARK-10399] [SPARK-23879] [SPARK-23762] ...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/22361


---

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


[GitHub] spark issue #22361: Revert [SPARK-10399] [SPARK-23879] [SPARK-23762] [SPARK-...

Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:

    https://github.com/apache/spark/pull/22361
  
    @gatorsmile Overall, I agree with revert since performance degradation is confirmed.
    
    When I run the TPC-DS in #19222, I have not seen such a performance regression as [here](https://github.com/apache/spark/pull/19222#issuecomment-378154854). For future analysis, I am very interested in 
    * execution time of each query
    * environment (machine, os, jvm, cluster or local, ...)
    * scaling factor,
    * # of repeats per query
    * others
    
    @npoggi and @winglungngai Thanks for your investigations. Would it be possible to share the avobe information?


---

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


[GitHub] spark issue #22361: Revert [SPARK-10399] [SPARK-23879] [SPARK-23762] [SPARK-...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22361
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #22361: Revert [SPARK-10399] [SPARK-23879] [SPARK-23762] [SPARK-...

Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:

    https://github.com/apache/spark/pull/22361
  
    It looks very intersting talk. Since I cannot join the SAIS unfortunally, I will watch this in live stream.


---

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


[GitHub] spark issue #22361: Revert [SPARK-10399] [SPARK-23879] [SPARK-23762] [SPARK-...

Posted by npoggi <gi...@git.apache.org>.
Github user npoggi commented on the issue:

    https://github.com/apache/spark/pull/22361
  
    Hi @kiszk and @HyukjinKwon, first sorry for the late reply, I was trying to get some detailed profiling info for this.  Basically, the regressions in TPCDS SF 1,000 are as follows when compared branch- `2.3` to `2.4` using a 10-workers cluster of 4-vcpus each :
    ```
    q67: from 323.2s to 283.6s (18.7%)
    q24a: from 139.1s to 153.9 (12.4%)
    q24b: from 128.7s to 145.7s (11.9%)
    ```
    These 3 queries in order are the easiest to root-cause to the new MemoryBlock API, as there is more time spent in UTF8String (getBytes, init, UnsafeRow.getUTF8String).  There were also other possibly affected queries: q8, q11, q54. 
    We can test future performance improvements to the proposal, just ping @winglungngai, @cloud-fan, and me in the PR.  Thanks,



---

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


[GitHub] spark issue #22361: Revert [SPARK-10399] [SPARK-23879] [SPARK-23762]

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:

    https://github.com/apache/spark/pull/22361
  
    cc @kiszk @cloud-fan 


---

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


[GitHub] spark issue #22361: Revert [SPARK-10399] [SPARK-23879] [SPARK-23762] [SPARK-...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22361
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95808/
    Test PASSed.


---

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


[GitHub] spark issue #22361: Revert [SPARK-10399] [SPARK-23879] [SPARK-23762]

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22361
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #22361: Revert [SPARK-10399] [SPARK-23879] [SPARK-23762]

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/22361
  
    **[Test build #95808 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95808/testReport)** for PR 22361 at commit [`cbe3a51`](https://github.com/apache/spark/commit/cbe3a514e3bdc47a1fedb49695f65bb378fc57cb).


---

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