You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by viirya <gi...@git.apache.org> on 2018/10/04 23:51:08 UTC

[GitHub] spark pull request #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting des...

GitHub user viirya opened a pull request:

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

    [SPARK-25591][PySpark][SQL] Avoid overwriting deserialized accumulator

    ## What changes were proposed in this pull request?
    
    If we use accumulators in more than one UDFs, it is possible to overwrite deserialized accumulators and its values. We should check if an accumulator was deserialized before overwriting it in accumulator registry.
    
    ## How was this patch tested?
    
    Added test.

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

    $ git pull https://github.com/viirya/spark-1 SPARK-25591

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

    https://github.com/apache/spark/pull/22635.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 #22635
    
----
commit db0a58350482545996b171c7ccc6603bade938f6
Author: Liang-Chi Hsieh <vi...@...>
Date:   2018-10-04T23:40:45Z

    Avoid overwriting deserialized accumulator.

----


---

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


[GitHub] spark issue #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

    https://github.com/apache/spark/pull/22635
  
    Please review https://issues.apache.org/jira/browse/SPARK-26019 
    "pyspark/accumulators.py: "TypeError: object of type 'NoneType' has no len()" in authenticate_and_accum_updates()"
    
    I suspect this change might have introduced SPARK-26019 regression.


---

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


[GitHub] spark pull request #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting des...

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

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


---

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


[GitHub] spark issue #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

    https://github.com/apache/spark/pull/22635
  
    How does it related with the JIRA? looks not quite related from a cursory look. Please leave some analysis next time or at least testing it before/after the specific commit. Let me take a look anyway.


---

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


[GitHub] spark pull request #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting des...

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

    https://github.com/apache/spark/pull/22635#discussion_r223251196
  
    --- Diff: python/pyspark/accumulators.py ---
    @@ -109,10 +109,14 @@
     
     def _deserialize_accumulator(aid, zero_value, accum_param):
         from pyspark.accumulators import _accumulatorRegistry
    -    accum = Accumulator(aid, zero_value, accum_param)
    -    accum._deserialized = True
    -    _accumulatorRegistry[aid] = accum
    -    return accum
    +    # If this certain accumulator was deserialized, don't overwrite it.
    +    if aid in _accumulatorRegistry:
    --- End diff --
    
    Yes.


---

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


[GitHub] spark issue #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

    https://github.com/apache/spark/pull/22635
  
    @cloud-fan @gatorsmile @HyukjinKwon Thanks. Yes. As Pandas UDF has the same issue and it is fixed by this PR.


---

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


[GitHub] spark issue #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

    https://github.com/apache/spark/pull/22635
  
    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 #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

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


---

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


[GitHub] spark issue #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

    https://github.com/apache/spark/pull/22635
  
    **[Test build #97100 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97100/testReport)** for PR 22635 at commit [`08c7223`](https://github.com/apache/spark/commit/08c7223c57d6c2b9536ba311ea4f81b20f37d973).
     * 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 #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting des...

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

    https://github.com/apache/spark/pull/22635#discussion_r223218361
  
    --- Diff: python/pyspark/sql/tests.py ---
    @@ -3603,6 +3603,31 @@ def test_repr_behaviors(self):
                         self.assertEquals(None, df._repr_html_())
                         self.assertEquals(expected, df.__repr__())
     
    +    # SPARK-25591
    +    def test_same_accumulator_in_udfs(self):
    +        from pyspark.sql.functions import udf
    +
    +        data_schema = StructType([StructField("a", DoubleType(), True),
    +                                  StructField("b", DoubleType(), True)])
    +        data = self.spark.createDataFrame([[1.0, 2.0]], schema=data_schema)
    +
    +        test_accum = self.sc.accumulator(0.0)
    +
    +        def first_udf(x):
    +            test_accum.add(1.0)
    +            return x
    +
    +        def second_udf(x):
    +            test_accum.add(100.0)
    +            return x
    +
    +        func_udf = udf(first_udf, DoubleType())
    +        func_udf2 = udf(second_udf, DoubleType())
    +        data = data.withColumn("out1", func_udf(data["a"]))
    +        data = data.withColumn("out2", func_udf2(data["b"]))
    +        data.collect()
    +        self.assertEqual(test_accum.value, 101)
    --- End diff --
    
    @viirya, can we just use int for data and accumulator as well in this test case?


---

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


[GitHub] spark pull request #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting des...

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

    https://github.com/apache/spark/pull/22635#discussion_r222890103
  
    --- Diff: python/pyspark/accumulators.py ---
    @@ -109,10 +109,14 @@
     
     def _deserialize_accumulator(aid, zero_value, accum_param):
         from pyspark.accumulators import _accumulatorRegistry
    -    accum = Accumulator(aid, zero_value, accum_param)
    -    accum._deserialized = True
    -    _accumulatorRegistry[aid] = accum
    -    return accum
    +    # If this certain accumulator was deserialized, don't overwrite it.
    +    if aid in _accumulatorRegistry:
    --- End diff --
    
    Should it be `if aid in _accumulatorRegistry and _accumulatorRegistry[aid]._deserialized is True`
    or:
    ```
    if aid in _accumulatorRegistry:
        _accumulatorRegistry[aid]._deserialize = True
        return _accumulatorRegistry[aid]
    ```
    To make double sure that this function always returns a deserialize version of the accum ?


---

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


[GitHub] spark issue #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

    https://github.com/apache/spark/pull/22635
  
    Yea, same issue exists in Pandas UDFs too (quickly double checked). This PR fixes it. That code path is rather one same place FYI.


---

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


[GitHub] spark pull request #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting des...

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

    https://github.com/apache/spark/pull/22635#discussion_r223251175
  
    --- Diff: python/pyspark/sql/tests.py ---
    @@ -3603,6 +3603,31 @@ def test_repr_behaviors(self):
                         self.assertEquals(None, df._repr_html_())
                         self.assertEquals(expected, df.__repr__())
     
    +    # SPARK-25591
    +    def test_same_accumulator_in_udfs(self):
    +        from pyspark.sql.functions import udf
    +
    +        data_schema = StructType([StructField("a", DoubleType(), True),
    +                                  StructField("b", DoubleType(), True)])
    +        data = self.spark.createDataFrame([[1.0, 2.0]], schema=data_schema)
    +
    +        test_accum = self.sc.accumulator(0.0)
    +
    +        def first_udf(x):
    +            test_accum.add(1.0)
    +            return x
    +
    +        def second_udf(x):
    +            test_accum.add(100.0)
    +            return x
    +
    +        func_udf = udf(first_udf, DoubleType())
    +        func_udf2 = udf(second_udf, DoubleType())
    +        data = data.withColumn("out1", func_udf(data["a"]))
    +        data = data.withColumn("out2", func_udf2(data["b"]))
    +        data.collect()
    +        self.assertEqual(test_accum.value, 101)
    --- End diff --
    
    Ok. 


---

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


[GitHub] spark issue #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

    https://github.com/apache/spark/pull/22635
  
    Since this is for correctness, I think we should include this into 2.4 if it can catch up. cc @cloud-fan 


---

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


[GitHub] spark pull request #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting des...

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

    https://github.com/apache/spark/pull/22635#discussion_r222899988
  
    --- Diff: python/pyspark/accumulators.py ---
    @@ -109,10 +109,14 @@
     
     def _deserialize_accumulator(aid, zero_value, accum_param):
         from pyspark.accumulators import _accumulatorRegistry
    -    accum = Accumulator(aid, zero_value, accum_param)
    -    accum._deserialized = True
    -    _accumulatorRegistry[aid] = accum
    -    return accum
    +    # If this certain accumulator was deserialized, don't overwrite it.
    +    if aid in _accumulatorRegistry:
    --- End diff --
    
    That doesnt seem right because the constructor for `Accumulator` has:
    ```
            ...
            self._deserialized = False
            _accumulatorRegistry[aid] = self
    ```


---

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


[GitHub] spark pull request #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting des...

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

    https://github.com/apache/spark/pull/22635#discussion_r222954381
  
    --- Diff: python/pyspark/accumulators.py ---
    @@ -109,10 +109,14 @@
     
     def _deserialize_accumulator(aid, zero_value, accum_param):
         from pyspark.accumulators import _accumulatorRegistry
    -    accum = Accumulator(aid, zero_value, accum_param)
    -    accum._deserialized = True
    -    _accumulatorRegistry[aid] = accum
    -    return accum
    +    # If this certain accumulator was deserialized, don't overwrite it.
    +    if aid in _accumulatorRegistry:
    --- End diff --
    
    I see - got it :+1:


---

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


[GitHub] spark issue #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

    https://github.com/apache/spark/pull/22635
  
    Thank you @viirya 
    



---

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


[GitHub] spark issue #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

    https://github.com/apache/spark/pull/22635
  
    cc @HyukjinKwon 



---

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


[GitHub] spark issue #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

    https://github.com/apache/spark/pull/22635
  
    @Tagar I will look into it. Thanks.


---

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


[GitHub] spark issue #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

    https://github.com/apache/spark/pull/22635
  
    @viirya I appologize, as I mentioned in my [comment in SPARK-26019](https://issues.apache.org/jira/browse/SPARK-26019?focusedCommentId=16692233&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16692233), it's due to another change 
    https://github.com/apache/spark/commit/15fc2372269159ea2556b028d4eb8860c4108650#diff-c3339bbf2b850b79445b41e9eecf57c4R249 - error happens in `authenticate_and_accum_updates()` and that's a new code that was brought by that code. Thanks for looking at that anyway! 


---

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


[GitHub] spark issue #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

    https://github.com/apache/spark/pull/22635
  
    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 #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

    https://github.com/apache/spark/pull/22635
  
    **[Test build #96960 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96960/testReport)** for PR 22635 at commit [`db0a583`](https://github.com/apache/spark/commit/db0a58350482545996b171c7ccc6603bade938f6).
     * 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 #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

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


---

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


[GitHub] spark issue #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

    https://github.com/apache/spark/pull/22635
  
    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/3690/
    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 #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting des...

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

    https://github.com/apache/spark/pull/22635#discussion_r223218449
  
    --- Diff: python/pyspark/accumulators.py ---
    @@ -109,10 +109,14 @@
     
     def _deserialize_accumulator(aid, zero_value, accum_param):
         from pyspark.accumulators import _accumulatorRegistry
    -    accum = Accumulator(aid, zero_value, accum_param)
    -    accum._deserialized = True
    -    _accumulatorRegistry[aid] = accum
    -    return accum
    +    # If this certain accumulator was deserialized, don't overwrite it.
    +    if aid in _accumulatorRegistry:
    --- End diff --
    
    Ah, so the problem is this accumulator is de/serialized multiple times and `_deserialize_accumulator` modifies the global status multiple times. I see. LGTM.


---

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


[GitHub] spark issue #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

    https://github.com/apache/spark/pull/22635
  
    Yeah, thanks @HyukjinKwon. I have an initial look, looks like it is not quite related.


---

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


[GitHub] spark issue #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

    https://github.com/apache/spark/pull/22635
  
    Nice catch @viirya LGTM.


---

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


[GitHub] spark issue #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

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


---

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


[GitHub] spark issue #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

    https://github.com/apache/spark/pull/22635
  
    This is fixed in 2.4.0 and your issue is when 2.3.1 -> 2.3.2. It's not related.


---

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


[GitHub] spark pull request #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting des...

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

    https://github.com/apache/spark/pull/22635#discussion_r222892890
  
    --- Diff: python/pyspark/accumulators.py ---
    @@ -109,10 +109,14 @@
     
     def _deserialize_accumulator(aid, zero_value, accum_param):
         from pyspark.accumulators import _accumulatorRegistry
    -    accum = Accumulator(aid, zero_value, accum_param)
    -    accum._deserialized = True
    -    _accumulatorRegistry[aid] = accum
    -    return accum
    +    # If this certain accumulator was deserialized, don't overwrite it.
    +    if aid in _accumulatorRegistry:
    --- End diff --
    
    We only save deserialized accumulators (`_deserialized` is `True`) into this dict.


---

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


[GitHub] spark issue #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

    https://github.com/apache/spark/pull/22635
  
    Thanks for cc'ing me. Will take a look this week.


---

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


[GitHub] spark pull request #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting des...

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

    https://github.com/apache/spark/pull/22635#discussion_r222905059
  
    --- Diff: python/pyspark/accumulators.py ---
    @@ -109,10 +109,14 @@
     
     def _deserialize_accumulator(aid, zero_value, accum_param):
         from pyspark.accumulators import _accumulatorRegistry
    -    accum = Accumulator(aid, zero_value, accum_param)
    -    accum._deserialized = True
    -    _accumulatorRegistry[aid] = accum
    -    return accum
    +    # If this certain accumulator was deserialized, don't overwrite it.
    +    if aid in _accumulatorRegistry:
    --- End diff --
    
    Yeah, but `_deserialize_accumulator` is only called when doing deserialzation at executors. The constructor saves accumulators in `_accumulatorRegistry` at driver.


---

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


[GitHub] spark issue #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

    https://github.com/apache/spark/pull/22635
  
    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 #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

    https://github.com/apache/spark/pull/22635
  
    Thanks @HyukjinKwon 


---

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


[GitHub] spark issue #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

    https://github.com/apache/spark/pull/22635
  
    **[Test build #97100 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97100/testReport)** for PR 22635 at commit [`08c7223`](https://github.com/apache/spark/commit/08c7223c57d6c2b9536ba311ea4f81b20f37d973).


---

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


[GitHub] spark issue #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

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


---

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


[GitHub] spark issue #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

    https://github.com/apache/spark/pull/22635
  
    Merged to master and branch-2.4.


---

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


[GitHub] spark issue #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

    https://github.com/apache/spark/pull/22635
  
    @cloud-fan @viirya Any chance of this making it into 2.4 ?


---

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


[GitHub] spark issue #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

    https://github.com/apache/spark/pull/22635
  
    How about pandas UDF? How about using RDD APIs? Do we face the same issues?


---

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


[GitHub] spark issue #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

    https://github.com/apache/spark/pull/22635
  
    @AbdealiJK since RC3 is not cut, this will be in 2.4.


---

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


[GitHub] spark issue #22635: [SPARK-25591][PySpark][SQL] Avoid overwriting deserializ...

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

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


---

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