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