[GitHub] [incubator-kyuubi] zhaomin1423 commented on a diff in pull request #2290: [KYUUBI #2024][FOLLOWUP] Hive Backend Engine - ProcBuilder for HiveEngine

[GitBox <gi...@apache.org>]

zhaomin1423 commented on code in PR #2290:
URL: https://github.com/apache/incubator-kyuubi/pull/2290#discussion_r848455275


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/hive/HiveProcessBuilder.scala:
##########
@@ -80,21 +74,53 @@ class HiveProcessBuilder(
     }
   }
 
-  override protected def childProcEnv: Map[String, String] = conf.getEnvs +
-    ("HIVE_ENGINE_JAR" -> mainResource.get) +
-    ("HIVE_ENGINE_DYNAMIC_ARGS" ->
-      conf.getAll.map { case (k, v) => s"-D$k=$v" }.mkString(" "))
-
   override protected def module: String = "kyuubi-hive-sql-engine"
 
   override protected def mainClass: String = "org.apache.kyuubi.engine.hive.HiveSQLEngine"
 
-  override protected def commands: Array[String] = Array(executable)
+  override protected def commands: Array[String] = {
+    val buffer = new ArrayBuffer[String]()
+    buffer += executable
 
-}
+    // TODO: How shall we deal with proxyUser,
+    // user.name
+    // kyuubi.session.user
+    // or just leave it, because we can handle it at operation layer
+    buffer += s"-D$KYUUBI_SESSION_USER_KEY=$proxyUser"
 
-object HiveProcessBuilder {
+    // TODO: add Kyuubi.engineEnv.HIVE_ENGINE_MEMORY or kyuubi.engine.hive.memory to configure
+    // -Xmx5g
+    // java options
+    for ((k, v) <- conf.getAll) {
+      buffer += s"-D$k=$v"
+    }
+
+    buffer += "-cp"
+    val classpathEntries = new LinkedHashSet[String]
+    // hive engine runtime jar
+    mainResource.foreach(classpathEntries.add)
+    // classpath contains hive configurations, default to hive.home/conf
+    classpathEntries.add(env.getOrElse("HIVE_CONF_DIR", s"$hiveHome${File.separator}conf"))
+    // classpath contains hadoop configurations
+    env.get("HADOOP_CONF_DIR").foreach(classpathEntries.add)
+    env.get("YARN_CONF_DIR").foreach(classpathEntries.add)
+    // jars from hive distribution
+    classpathEntries.add(s"$hiveHome${File.separator}lib${File.separator}*")
+    val hadoopCp = env.get("HIVE_HADOOP_CLASSPATH").orElse(env.get("HADOOP_CLASSPATH"))

Review Comment:
   The HIVE_HADOOP_CLASSPATH or HADOOP_CLASSPATH is neccessary when using in production, but without validation. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org