You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by icexelloss <gi...@git.apache.org> on 2018/05/22 15:16:20 UTC

[GitHub] spark pull request #21397: [SPARK-24334] Fix race condition in ArrowPythonRu...

GitHub user icexelloss opened a pull request:

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

    [SPARK-24334] Fix race condition in ArrowPythonRunner causes unclean shutdown of Arrow memory allocator

    ## What changes were proposed in this pull request?
    
    There is a race condition of closing Arrow VectorSchemaRoot and Allocator in the writer thread of ArrowPythonRunner. 
    
    The race results in memory leak exception when closing the allocator. This patch removes the closing routine from the TaskCompletionListener and make the writer thread responsible for cleaning up the Arrow memory.
    
    ## How was this patch tested?
    
    Because of the race condition, the bug cannot be unit test easily. So far it has only happens on large amount of data. This is currently tested manually.
    
    Please review http://spark.apache.org/contributing.html before opening a pull request.


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

    $ git pull https://github.com/icexelloss/spark SPARK-24334-arrow-memory-leak

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

    https://github.com/apache/spark/pull/21397.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 #21397
    
----

----


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    **[Test build #91201 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91201/testReport)** for PR 21397 at commit [`756a73a`](https://github.com/apache/spark/commit/756a73aea843e8d5d90994d127c0d9d4c357c67b).
     * 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 #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    I'm guessing making a reliable test is too difficult?  is it possible to provide some steps to reproduce?


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    This seems like it should be good to me.  It's a little bit different than the ArrowConverters that also have a listener, because they are iterators and the cleanup can't be put in a finally.  I would like for @ueshin to take a look though.
    
    Also, I don't think we should include the unit test if it doesn't create the issue every time.


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    Thank you for fixing this :-)


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    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/3463/
    Test PASSed.


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    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 #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    Thanks for the quick responses. I did try to build everything from scratch and am still getting the error on large datasets. If I run on a few tens of GB, there's no problem but once it gets to a couple hundred GB, that's when I start seeing the issue. I will try to create a reproducible example and post it here shortly.


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    Only when udf raises error. In normal case, there is no race because the writer thread always closes the root and allocator before task completion listener runs.


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    **[Test build #90972 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90972/testReport)** for PR 21397 at commit [`435ccff`](https://github.com/apache/spark/commit/435ccfff44995ca5ad487e77128b2cae4ff1cfd5).


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    Merged to master and branch-2.3.


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    One more question, do you only observe this when the python udf raises an error or have you seen it in normal runtime operation?


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    **[Test build #90991 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90991/testReport)** for PR 21397 at commit [`69c9104`](https://github.com/apache/spark/commit/69c91043981056a732328b474986fe4128936c62).
     * 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 pull request #21397: [SPARK-24334] Fix race condition in ArrowPythonRu...

Posted by icexelloss <gi...@git.apache.org>.
Github user icexelloss commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21397#discussion_r189942402
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/python/ArrowPythonRunner.scala ---
    @@ -94,8 +88,18 @@ class ArrowPythonRunner(
                 writer.writeBatch()
                 arrowWriter.reset()
               }
    -        } {
               writer.end()
    --- End diff --
    
    This is moved out of the finally block because this function will throw exception if the thread is interrupted. writer.end() also doesn't do any clean up - it just writes some final bits to the output channel so we don't need to call it in clean up.


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    **[Test build #91201 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91201/testReport)** for PR 21397 at commit [`756a73a`](https://github.com/apache/spark/commit/756a73aea843e8d5d90994d127c0d9d4c357c67b).


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    Yeah this is a bit tricky because of the race. The test does fail on my machine without the fix.
    
    I have been changing the test data size until I can reproduce it, so it's not great. If you want to reproduce the memory leak, probably need to increase the data size or sth.


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    Thanks for the fix. I was having the memory leak issue described in [JIRA](https://issues.apache.org/jira/browse/SPARK-24334) when working with pandas udf's but was able to fix it after upgrading my Spark version to get the patch. However, now I'm getting an issue related with the serializer and I'm having trouble debugging and understanding the stack trace. Any ideas?
    
    ```
    INFO TaskSetManager: Lost task [...] org.apache.spark.api.python.PythonException (Traceback (most recent call last):
      File "/home/spark-current/python/lib/pyspark.zip/pyspark/worker.py", line 230, in main
        process()
      File "/home/spark-current/python/lib/pyspark.zip/pyspark/worker.py", line 225, in process
        serializer.dump_stream(func(split_index, iterator), outfile)
      File "/home/spark-current/python/lib/pyspark.zip/pyspark/serializers.py", line 260, in dump_stream
        for series in iterator:
      File "/home/spark-current/python/lib/pyspark.zip/pyspark/serializers.py", line 279, in load_stream
        for batch in reader:
      File "ipc.pxi", line 268, in __iter__
      File "ipc.pxi", line 284, in pyarrow.lib._RecordBatchReader.read_next_batch
      File "error.pxi", line 79, in pyarrow.lib.check_status
    pyarrow.lib.ArrowIOError: read length must be positive or -1
    ```


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

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


---

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


[GitHub] spark pull request #21397: [SPARK-24334] Fix race condition in ArrowPythonRu...

Posted by advancedxy <gi...@git.apache.org>.
Github user advancedxy commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21397#discussion_r189973566
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/python/ArrowPythonRunner.scala ---
    @@ -94,8 +88,18 @@ class ArrowPythonRunner(
                 writer.writeBatch()
                 arrowWriter.reset()
               }
    -        } {
               writer.end()
    --- End diff --
    
    So basically your change makes sure that `root` and `allocator` is safely closed in the final block and hence we don't need the safe belt in `TaskCompletionListener`?


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    @vamaral1 , I've seen this error too and I'm trying to remember what the cause was.. I think it can happen when there is some files get mixed up when updating/building.  If you're building your own spark with this patch, try first to clean everything and rebuild.


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    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/3619/
    Test PASSed.


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    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/3475/
    Test PASSed.


---

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


[GitHub] spark pull request #21397: [SPARK-24334] Fix race condition in ArrowPythonRu...

Posted by icexelloss <gi...@git.apache.org>.
Github user icexelloss commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21397#discussion_r189996870
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/python/ArrowPythonRunner.scala ---
    @@ -94,8 +88,18 @@ class ArrowPythonRunner(
                 writer.writeBatch()
                 arrowWriter.reset()
               }
    -        } {
               writer.end()
    --- End diff --
    
    The routine in `TaskCompletionListener` is not really a safe belt, it actually causes the race condition. But you are right, I believe putting the cleanup routine in finally block is enough.


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    Thanks all for review!


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    So far I have been using a local parquet file to test. Let me try if I can create one on the fly to reproduce this.



---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    Yea, I was wondering about it too. It should be nicer if we have some steps in the PR description.


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    **[Test build #90991 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90991/testReport)** for PR 21397 at commit [`69c9104`](https://github.com/apache/spark/commit/69c91043981056a732328b474986fe4128936c62).


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

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


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

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


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    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 #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    @icexelloss the test you added passes for me without the fix, but I did see a message of a suppressed exception in the finally block "java.lang.IllegalStateException: ArrowBuf[3] refCnt has gone negative."
    I'll try to look into it in more detail soon.


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    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 #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    Sure! Added.


---

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


[GitHub] spark pull request #21397: [SPARK-24334] Fix race condition in ArrowPythonRu...

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

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


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    cc @BryanCutler 



---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    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 #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    LGTM too.


---

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


[GitHub] spark pull request #21397: [SPARK-24334] Fix race condition in ArrowPythonRu...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21397#discussion_r191070397
  
    --- Diff: python/pyspark/sql/tests.py ---
    @@ -4931,6 +4931,30 @@ def foo3(key, pdf):
             expected4 = udf3.func((), pdf)
             self.assertPandasEqual(expected4, result4)
     
    +    # Regression test for SPARK-24334
    +    def test_memory_leak(self):
    --- End diff --
    
    Yea, I think it's good enough to have it in PR description. Let's just put this in the PR description.


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    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 #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    This seems to indicate that the arrow stream from java -> python is closed prematurely. If you have a way to reproduce I am happy to take a lok=ok.


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    @BryanCutler @HyukjinKwon I am able to have it reproduced in unit test. Please take a look thanks! :)


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    Btw, can you add a short note in PR description for the reason why the test is just in the PR description? Thanks.


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    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 pull request #21397: [SPARK-24334] Fix race condition in ArrowPythonRu...

Posted by icexelloss <gi...@git.apache.org>.
Github user icexelloss commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21397#discussion_r191082371
  
    --- Diff: python/pyspark/sql/tests.py ---
    @@ -4931,6 +4931,30 @@ def foo3(key, pdf):
             expected4 = udf3.func((), pdf)
             self.assertPandasEqual(expected4, result4)
     
    +    # Regression test for SPARK-24334
    +    def test_memory_leak(self):
    --- End diff --
    
    SGTM! Moved to PR description.


---

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


[GitHub] spark pull request #21397: [SPARK-24334] Fix race condition in ArrowPythonRu...

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21397#discussion_r191040434
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/python/ArrowPythonRunner.scala ---
    @@ -94,8 +88,18 @@ class ArrowPythonRunner(
                 writer.writeBatch()
                 arrowWriter.reset()
               }
    -        } {
               writer.end()
    --- End diff --
    
    maybe add a comment to explain the same - that this should be outside of the finally?


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    Hey @BryanCutler, any more thoughts on this?


---

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


[GitHub] spark issue #21397: [SPARK-24334] Fix race condition in ArrowPythonRunner ca...

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

    https://github.com/apache/spark/pull/21397
  
    **[Test build #90972 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90972/testReport)** for PR 21397 at commit [`435ccff`](https://github.com/apache/spark/commit/435ccfff44995ca5ad487e77128b2cae4ff1cfd5).
     * 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