thanks for reviewing

yaooqinn · yaooqinn · commit c50a32f0476d · 2017-08-09T23:45:05.000+08:00
diff --git a/core/src/main/scala/org/apache/spark/deploy/SparkHadoopUtil.scala b/core/src/main/scala/org/apache/spark/deploy/SparkHadoopUtil.scala
@@ -138,8 +138,7 @@ class SparkHadoopUtil extends Logging {
 
   def loginUserFromKeytab(principalName: String, keytabFilename: String): Unit = {
     if (!new File(keytabFilename).exists()) {
-      throw new SparkException(s"Keytab file: ${keytabFilename}" +
-        " specified in spark.yarn.keytab does not exist")
+      throw new SparkException(s"Keytab file: ${keytabFilename} does not exist")
     } else {
       logInfo("Attempting to login to Kerberos" +
         s" using principal: ${principalName} and keytab: ${keytabFilename}")
diff --git a/core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala b/core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala
@@ -28,7 +28,7 @@ import javax.net.ssl._
 
 import scala.annotation.tailrec
 import scala.collection.mutable.{ArrayBuffer, HashMap, Map}
-import scala.util.{Failure, Properties, Success, Try}
+import scala.util.Properties
 
 import com.google.common.io.ByteStreams
 import org.apache.commons.io.FileUtils
diff --git a/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala b/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala
@@ -230,6 +230,11 @@ private[spark] object HiveUtils extends Logging {
     }.toMap
   }
 
+  /**
+   * Check current Thread's SessionState type
+   * @return true when SessionState.get returns an instance of CliSessionState,
+   *         false when it gets non-CliSessionState instance or null
+   */
   def isCliSessionState(): Boolean = {
     val state = SessionState.get
     var temp: Class[_] = if (state != null) state.getClass else null
diff --git a/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala b/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala
@@ -108,8 +108,7 @@ private[hive] class HiveClientImpl(
     if (clientLoader.isolationOn) {
       // Switch to the initClassLoader.
       Thread.currentThread().setContextClassLoader(initClassLoader)
-      // Set up kerberos credentials for UserGroupInformation.loginUser within
-      // current class loader
+      // Set up kerberos credentials for UserGroupInformation.loginUser within current class loader
       if (sparkConf.contains("spark.yarn.principal") && sparkConf.contains("spark.yarn.keytab")) {
         val principal = sparkConf.get("spark.yarn.principal")
         val keytab = sparkConf.get("spark.yarn.keytab")
@@ -121,7 +120,16 @@ private[hive] class HiveClientImpl(
         Thread.currentThread().setContextClassLoader(original)
       }
     } else {
-      Option(SessionState.get()).getOrElse(newState())
+      // Isolation off means we detect a CliSessionState instance in current thread. 1: Inside the
+      // spark project, we have already started a CliSessionState  in `SparkSQLCLIDriver`, which
+      // contains information like configurations from command lines. Later, we call
+      // `SparkSQLEnv.init()` there, which would new a hive client again into this part again. so we
+      // should keep those configurations and reuse the existing instance of `CliSessionState`. In
+      // this case, SessionState.get will always return a CliSessionState.
+      // In another case, a user app may start a CliSessionState outside spark project with built in
+      // hive jars, which will turn off isolation, after that, SessionSate.detachSession is called
+      // to remove the current state, hive client created later will init its state by newState()
+      Option(SessionState.get).getOrElse(newState())
     }
   }
 
@@ -151,11 +159,11 @@ private[hive] class HiveClientImpl(
     // 3: we set all entries in config to this hiveConf.
     (hadoopConf.iterator().asScala.map(kv => kv.getKey -> kv.getValue)
       ++ sparkConf.getAll.toMap ++ extraConfig).foreach { case (k, v) =>
-      if (k.toLowerCase(Locale.ROOT).contains("password")) {
-        logDebug(s"Applying Spark config to Hive Conf: $k=xxx")
-      } else {
-        logDebug(s"Applying Spark config to Hive Conf: $k=$v")
-      }
+      logDebug(
+        s"""
+           |Applying Hadoop/Hive/Spark and extra properties to Hive Conf:
+           |$k=${if (k.toLowerCase(Locale.ROOT).contains("password")) "xxx" else v}
+         """.stripMargin)
       hiveConf.set(k, v)
     }
     val state = new SessionState(hiveConf)
