Github user wzhfy commented on a diff in the pull request: https://github.com/apache/spark/pull/19605#discussion_r147632813 --- 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 -- Then can we remove the synchronization of `clientLoader` in `HiveClientImpl`? If we synchronize `withClient`, then there's no need to sync each operation in `body`.
--- --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org