Github user patrickbrownsync commented on a diff in the pull request:
https://github.com/apache/spark/pull/22855#discussion_r231558355
--- Diff:
core/src/main/scala/org/apache/spark/serializer/KryoSerializer.scala ---
@@ -214,8 +230,14 @@ class KryoSerializer(conf: SparkConf)
kryo
}
+ override def setDefaultClassLoader(classLoader: ClassLoader): Serializer
= {
+ defaultClassLoader = Some(classLoader)
--- End diff --
defaultClassLoader is used in newKryo.
I called `getPool` after setting the defaultClassLoader to make sure we
don't accidently create a newKryo before the defaultClassLoader is updated.
Setting it on 105 was because I don't believe `setDefaultClassLoader` is
required to be called.
The issue that I unfortunately didn't notice until these tests failed is
that the tests specify that you can `setDefaultClassLoader` after serializing
an object (I'm not sure this functionality is actually used), causing an
"incorrect" Kryo instance to be in the pool. Unfortunately the pool doesn't
expose a way to clear itself out, thus the var, which clearly doesn't work.
I will work on a solution
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
