[SPARK-21928][CORE] Set classloader on SerializerManager's private kryo #19280
Conversation
We have to make sure thatthat SerializerManager's private instance of kryo also uses the right classloader, regardless of the current thread classloader. In particular, this fixes serde during remote cache fetches, as those occur in netty threads.
|
Test build #81944 has finished for PR 19280 at commit
|
|
Test build #81949 has finished for PR 19280 at commit
|
|
reaching out to some potential reviewers: @vanzin @srowen @JoshRosen @mridulm @tgravescs |
|
Looks ok to me, assuming the "default serializer" in SerializerManager is configured correctly through other means. Title would sound better with a possessive: "SerializerManager's private kryo" |
squito
changed the title
I think that part is fine. The serializer is created here: The same instance is assigned to Which has its default classloader set in Executor.scala, right by the part I'm changing: https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/executor/Executor.scala#L131 |
|
LGTM, merging to master / 2.2 / 2.1. |
## What changes were proposed in this pull request? We have to make sure that SerializerManager's private instance of kryo also uses the right classloader, regardless of the current thread classloader. In particular, this fixes serde during remote cache fetches, as those occur in netty threads. ## How was this patch tested? Manual tests & existing suite via jenkins. I haven't been able to reproduce this is in a unit test, because when a remote RDD partition can be fetched, there is a warning message and then the partition is just recomputed locally. I manually verified the warning message is no longer present. Author: Imran Rashid <irashid@cloudera.com> Closes #19280 from squito/SPARK-21928_ser_classloader. (cherry picked from commit b75bd17) Signed-off-by: Marcelo Vanzin <vanzin@cloudera.com>
|
Didn't merge to 2.1, please open a PR against that branch if you want it there. |
We have to make sure that SerializerManager's private instance of kryo also uses the right classloader, regardless of the current thread classloader. In particular, this fixes serde during remote cache fetches, as those occur in netty threads. Manual tests & existing suite via jenkins. I haven't been able to reproduce this is in a unit test, because when a remote RDD partition can be fetched, there is a warning message and then the partition is just recomputed locally. I manually verified the warning message is no longer present. Author: Imran Rashid <irashid@cloudera.com> Closes apache#19280 from squito/SPARK-21928_ser_classloader. (cherry picked from commit b75bd17)
## What changes were proposed in this pull request? We have to make sure that SerializerManager's private instance of kryo also uses the right classloader, regardless of the current thread classloader. In particular, this fixes serde during remote cache fetches, as those occur in netty threads. ## How was this patch tested? Manual tests & existing suite via jenkins. I haven't been able to reproduce this is in a unit test, because when a remote RDD partition can be fetched, there is a warning message and then the partition is just recomputed locally. I manually verified the warning message is no longer present. Author: Imran Rashid <irashid@cloudera.com> Closes apache#19280 from squito/SPARK-21928_ser_classloader. (cherry picked from commit b75bd17) Signed-off-by: Marcelo Vanzin <vanzin@cloudera.com>
What changes were proposed in this pull request?
We have to make sure that SerializerManager's private instance of
kryo also uses the right classloader, regardless of the current thread
classloader. In particular, this fixes serde during remote cache
fetches, as those occur in netty threads.
How was this patch tested?
Manual tests & existing suite via jenkins. I haven't been able to reproduce this is in a unit test, because when a remote RDD partition can be fetched, there is a warning message and then the partition is just recomputed locally. I manually verified the warning message is no longer present.