You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/07/15 09:52:15 UTC
[GitHub] [spark] LantaoJin opened a new pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
LantaoJin opened a new pull request #29123:
URL: https://github.com/apache/spark/pull/29123
### What changes were proposed in this pull request?
`spark.kryo.registrator` in 3.0 has a regression problem. From [SPARK-12080](https://issues.apache.org/jira/browse/SPARK-12080), it supports multiple user registrators by
```scala
private val userRegistrators = conf.get("spark.kryo.registrator", "")
.split(',').map(_.trim)
.filter(!_.isEmpty)
```
But it donsn't work in 3.0. Fix it by `toSequence` in `Kryo.scala`
### Why are the changes needed?
In previous Spark version (2.x), it supported multiple user registrators by
```scala
private val userRegistrators = conf.get("spark.kryo.registrator", "")
.split(',').map(_.trim)
.filter(!_.isEmpty)
```
But it doesn't work in 3.0. It's should be a regression.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Existed unit tests.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29123:
URL: https://github.com/apache/spark/pull/29123#issuecomment-659105655
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #29123:
URL: https://github.com/apache/spark/pull/29123#issuecomment-665438747
+1, late LGTM. Thank you, @LantaoJin and @cloud-fan .
cc @HeartSaVioR and @srowen
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LantaoJin commented on pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
LantaoJin commented on pull request #29123:
URL: https://github.com/apache/spark/pull/29123#issuecomment-659374776
cc @RotemShaul @vanzin @dongjoon-hyun
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LantaoJin commented on pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
LantaoJin commented on pull request #29123:
URL: https://github.com/apache/spark/pull/29123#issuecomment-659073795
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29123:
URL: https://github.com/apache/spark/pull/29123#issuecomment-659105655
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29123:
URL: https://github.com/apache/spark/pull/29123#issuecomment-659074553
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #29123:
URL: https://github.com/apache/spark/pull/29123#issuecomment-665416270
thanks, merging to master/3.0!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29123:
URL: https://github.com/apache/spark/pull/29123#issuecomment-658984370
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29123:
URL: https://github.com/apache/spark/pull/29123#issuecomment-658857296
**[Test build #125880 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125880/testReport)** for PR 29123 at commit [`45d1e43`](https://github.com/apache/spark/commit/45d1e4341ecab8d5271e17f9ae13072c71c46e32).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29123:
URL: https://github.com/apache/spark/pull/29123#discussion_r461495173
##########
File path: core/src/main/scala/org/apache/spark/internal/config/Kryo.scala
##########
@@ -29,7 +29,8 @@ private[spark] object Kryo {
val KRYO_USER_REGISTRATORS = ConfigBuilder("spark.kryo.registrator")
.version("0.5.0")
.stringConf
- .createOptional
Review comment:
do you know which commit broke it?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29123:
URL: https://github.com/apache/spark/pull/29123#issuecomment-659185091
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29123:
URL: https://github.com/apache/spark/pull/29123#issuecomment-658983163
**[Test build #125880 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125880/testReport)** for PR 29123 at commit [`45d1e43`](https://github.com/apache/spark/commit/45d1e4341ecab8d5271e17f9ae13072c71c46e32).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29123:
URL: https://github.com/apache/spark/pull/29123#issuecomment-658984381
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/125880/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29123:
URL: https://github.com/apache/spark/pull/29123#issuecomment-658859532
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LantaoJin commented on pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
LantaoJin commented on pull request #29123:
URL: https://github.com/apache/spark/pull/29123#issuecomment-659104173
Retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29123:
URL: https://github.com/apache/spark/pull/29123#issuecomment-659074553
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LantaoJin commented on pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
LantaoJin commented on pull request #29123:
URL: https://github.com/apache/spark/pull/29123#issuecomment-661684754
cc @cloud-fan
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan closed pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #29123:
URL: https://github.com/apache/spark/pull/29123
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29123:
URL: https://github.com/apache/spark/pull/29123#issuecomment-658857296
**[Test build #125880 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125880/testReport)** for PR 29123 at commit [`45d1e43`](https://github.com/apache/spark/commit/45d1e4341ecab8d5271e17f9ae13072c71c46e32).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HeartSaVioR commented on pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on pull request #29123:
URL: https://github.com/apache/spark/pull/29123#issuecomment-665441547
Nice find and thanks for fixing this! I have no idea why I took different way to deal with KRYO_USER_REGISTRATORS and KRYO_CLASSES_TO_REGISTER... I guess it was just a mistake. My bad.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29123:
URL: https://github.com/apache/spark/pull/29123#issuecomment-659131918
**[Test build #125928 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125928/testReport)** for PR 29123 at commit [`45d1e43`](https://github.com/apache/spark/commit/45d1e4341ecab8d5271e17f9ae13072c71c46e32).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LantaoJin commented on a change in pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
LantaoJin commented on a change in pull request #29123:
URL: https://github.com/apache/spark/pull/29123#discussion_r462005092
##########
File path: core/src/main/scala/org/apache/spark/internal/config/Kryo.scala
##########
@@ -29,7 +29,8 @@ private[spark] object Kryo {
val KRYO_USER_REGISTRATORS = ConfigBuilder("spark.kryo.registrator")
.version("0.5.0")
.stringConf
- .createOptional
Review comment:
#23532 / SPARK-26466
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29123:
URL: https://github.com/apache/spark/pull/29123#issuecomment-658984370
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29123:
URL: https://github.com/apache/spark/pull/29123#issuecomment-658859532
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29123:
URL: https://github.com/apache/spark/pull/29123#issuecomment-659131918
**[Test build #125928 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125928/testReport)** for PR 29123 at commit [`45d1e43`](https://github.com/apache/spark/commit/45d1e4341ecab8d5271e17f9ae13072c71c46e32).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29123:
URL: https://github.com/apache/spark/pull/29123#issuecomment-659184552
**[Test build #125928 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125928/testReport)** for PR 29123 at commit [`45d1e43`](https://github.com/apache/spark/commit/45d1e4341ecab8d5271e17f9ae13072c71c46e32).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29123: [SPARK-32283][CORE] Kryo should support multiple user registrators
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29123:
URL: https://github.com/apache/spark/pull/29123#issuecomment-659185091
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org