Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19605#discussion_r147619876
--- Diff:
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala ---
@@ -89,10 +89,12 @@ private[spark] class HiveExternalCatalog(conf:
SparkConf, hadoopConf: Configurat
}
/**
- * Run some code involving `client` in a [[synchronized]] block and wrap
certain
- * exceptions thrown in the process in [[AnalysisException]].
+ * Run some code involving `client` and wrap certain exceptions thrown
in the process in
+ * [[AnalysisException]]. Thread-safety is guaranteed here because
methods in the `client`
+ * ([[org.apache.spark.sql.hive.client.HiveClientImpl]]) are already
synchronized through
+ * `clientLoader` in the `retryLocked` method.
*/
- private def withClient[T](body: => T): T = synchronized {
+ private def withClient[T](body: => T): T = {
--- End diff --
If you check the callers of `withClient`, you can find many callers conduct
multiple client-related operations in the same `body`. Removing this lock might
cause some concurrency issues.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
