Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-11194] [SQL] Use MutableURLClassLoader for the classLoader in IsolatedClientLoader. #9170

Closed
wants to merge 1 commit into from

Conversation

yhuai
Copy link
Contributor

@yhuai yhuai commented Oct 19, 2015

@SparkQA
Copy link

SparkQA commented Oct 19, 2015

Test build #43938 has finished for PR 9170 at commit a5d3e0a.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@yhuai
Copy link
Contributor Author

yhuai commented Oct 19, 2015

test this please

@yhuai
Copy link
Contributor Author

yhuai commented Oct 20, 2015

hmm. When we close a session in thrift server, we will close the session state, which internally closes the class loader (the code starts from https://github.com/apache/spark/blob/master/sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLSessionManager.scala#L77 and SessionState.close will call JavaUtils.closeClassLoadersTo(conf.getClassLoader(), parentLoader)). So, the mutable classloader will be closed after we close a jdbc session. Actually, this behavior may cause problems even if we do not use this mutable classloader.

@liancheng For our thrift server, do we need to use HiveSession?

@yhuai
Copy link
Contributor Author

yhuai commented Oct 20, 2015

https://docs.oracle.com/javase/8/docs/technotes/guides/net/ClassLoader.html is an article about closing a classloader. Looks like close is mainly used when we want to hot-reload a jar.

}
} else {
baseClassLoader
new MutableURLClassLoader(Array.empty, isolatedClassLoader)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One hacky way to workaround the problem is that we have a no-op close method for this classloader.

@SparkQA
Copy link

SparkQA commented Oct 23, 2015

Test build #44208 has finished for PR 9170 at commit 65f0a4c.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * case class LongEncoder(fieldName: String = \"value\", ordinal: Int = 0) extends Encoder[Long]\n * case class IntEncoder(fieldName: String = \"value\", ordinal: Int = 0) extends Encoder[Int]\n * case class StringEncoder(\n * |class Tuple$\n * class Tuple2Encoder[T1, T2](e1: Encoder[T1], e2: Encoder[T2]) extends Encoder[(T1, T2)]\n * class Tuple3Encoder[T1, T2, T3](e1: Encoder[T1], e2: Encoder[T2], e3: Encoder[T3]) extends Encoder[(T1, T2, T3)]\n * class Tuple4Encoder[T1, T2, T3, T4](e1: Encoder[T1], e2: Encoder[T2], e3: Encoder[T3], e4: Encoder[T4]) extends Encoder[(T1, T2, T3, T4)]\n * class Tuple5Encoder[T1, T2, T3, T4, T5](e1: Encoder[T1], e2: Encoder[T2], e3: Encoder[T3], e4: Encoder[T4], e5: Encoder[T5]) extends Encoder[(T1, T2, T3, T4, T5)]\n * implicit class AttributeSeq(attrs: Seq[Attribute])\n * case class MapPartitions[T, U](\n * case class AppendColumn[T, U](\n * case class MapGroups[K, T, U](\n * class TypedColumn[T](expr: Expression)(implicit val encoder: Encoder[T]) extends Column(expr)\n * abstract class SQLImplicits\n * case class MapPartitions[T, U](\n * case class AppendColumns[T, U](\n * case class MapGroups[K, T, U](\n * // to its own URLs over the parent class loader (see Executor's createClassLoader method).\n

@davies
Copy link
Contributor

davies commented Oct 23, 2015

LGTM

@yhuai
Copy link
Contributor Author

yhuai commented Oct 24, 2015

Thanks for reviewing! I am merging it to master.

@asfgit asfgit closed this in 4725cb9 Oct 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants