[spark] branch master updated: [SPARK-37017][SQL] Reduce the scope of synchronized to prevent potential deadlock

[we...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

wenchen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/master by this push:
     new 875963a  [SPARK-37017][SQL] Reduce the scope of synchronized to prevent potential deadlock
875963a is described below

commit 875963a28a75532871010fcdcdb916bf093dab34
Author: chenzhx <ch...@apache.org>
AuthorDate: Tue Oct 19 14:48:32 2021 +0800

    [SPARK-37017][SQL] Reduce the scope of synchronized to prevent potential deadlock
    
    ### What changes were proposed in this pull request?
    
    There is a `synchronize` block in `CatalogManager.currentNamespace` function.
    This PR pulls `SessionCatalog.getCurrentDatabase` out from this `synchronize` block to prevent potential deadlock.
    
    ### Why are the changes needed?
    
    In our case, we have implemented an external catalog, and there is a thread that directly calls SessionCatalog.getTempViewOrPermanentTableMetadata and holds the lock of SessionCatalog. It eventually goes into our external catalog, unfortunately, we then call some functions of SparkSession, e.g. sql. When it calls CatalogManager.currentNamespace, it tries to hold the lock of CatalogManager.
    
    In the meantime, there are some query threads that execute sqls via DataFrame interface.
    
    This is how deadlock occurs.
    
    ### Does this PR introduce _any_ user-facing change?
    
    No
    
    ### How was this patch tested?
    
    use existed test.
    
    Closes #34292 from chenzhx/bug-fix.
    
    Authored-by: chenzhx <ch...@apache.org>
    Signed-off-by: Wenchen Fan <we...@databricks.com>
---
 .../spark/sql/connector/catalog/CatalogManager.scala     | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogManager.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogManager.scala
index 7d8bc4f..0380621 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogManager.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogManager.scala
@@ -89,12 +89,16 @@ class CatalogManager(
 
   private var _currentNamespace: Option[Array[String]] = None
 
-  def currentNamespace: Array[String] = synchronized {
-    _currentNamespace.getOrElse {
-      if (currentCatalog.name() == SESSION_CATALOG_NAME) {
-        Array(v1SessionCatalog.getCurrentDatabase)
-      } else {
-        currentCatalog.defaultNamespace()
+  def currentNamespace: Array[String] = {
+    val defaultNamespace = if (currentCatalog.name() == SESSION_CATALOG_NAME) {
+      Array(v1SessionCatalog.getCurrentDatabase)
+    } else {
+      currentCatalog.defaultNamespace()
+    }
+
+    this.synchronized {
+      _currentNamespace.getOrElse {
+        defaultNamespace
       }
     }
   }

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org