You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kyuubi.apache.org by ya...@apache.org on 2022/04/13 04:32:16 UTC
[incubator-kyuubi] branch master updated: [KYUUBI #2336] Simplify TrinoProcessBuilder with java executable
This is an automated email from the ASF dual-hosted git repository.
yao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-kyuubi.git
The following commit(s) were added to refs/heads/master by this push:
new 9e486080f [KYUUBI #2336] Simplify TrinoProcessBuilder with java executable
9e486080f is described below
commit 9e486080ffad43245c84cda9674196f15a226bd8
Author: Kent Yao <ya...@apache.org>
AuthorDate: Wed Apr 13 12:32:08 2022 +0800
[KYUUBI #2336] Simplify TrinoProcessBuilder with java executable
### _Why are the changes needed?_
Build the java command programlly instead of a static shell script
### _How was this patch tested?_
- [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible
- [ ] Add screenshots for manual tests if appropriate
- [x] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request
Closes #2336 from yaooqinn/trino.
Closes #2336
c906fcc4 [Kent Yao] add comments
7048f971 [Kent Yao] fix tests
49e582fc [Kent Yao] fix tests
444de58e [Kent Yao] Simplify TrinoProcessBuilder with java executable
9f2d1467 [Kent Yao] Simplify TrinoProcessBuilder with java executable
d4fdd467 [Kent Yao] Simplify TrinoProcessBuilder with java executable
48da4e0a [Kent Yao] Simplify TrinoProcessBuilder with java executable
de4884c0 [Kent Yao] Simplify TrinoProcessBuilder with java executable
6ac315a2 [Kent Yao] Simplify TrinoProcessBuilder with java executable
Authored-by: Kent Yao <ya...@apache.org>
Signed-off-by: Kent Yao <ya...@apache.org>
---
build/dist | 9 +-
externals/kyuubi-trino-engine/bin/trino-engine.sh | 42 -------
externals/kyuubi-trino-engine/pom.xml | 7 +-
.../engine/trino/util/PreconditionsWrapper.java | 31 -----
.../apache/kyuubi/engine/trino/schema/RowSet.scala | 7 +-
.../engine/trino/session/TrinoSessionImpl.scala | 5 +-
.../org/apache/kyuubi/engine/ProcBuilder.scala | 54 ++++++++-
.../kyuubi/engine/flink/FlinkProcessBuilder.scala | 31 +----
.../kyuubi/engine/hive/HiveProcessBuilder.scala | 40 +------
.../kyuubi/engine/spark/SparkProcessBuilder.scala | 32 +-----
.../kyuubi/engine/trino/TrinoProcessBuilder.scala | 128 +++++++++------------
.../engine/trino/TrinoProcessBuilderSuite.scala | 17 +--
12 files changed, 134 insertions(+), 269 deletions(-)
diff --git a/build/dist b/build/dist
index ef949d177..b6103a090 100755
--- a/build/dist
+++ b/build/dist
@@ -218,7 +218,6 @@ mkdir -p "$DISTDIR/externals/engines/flink/lib"
mkdir -p "$DISTDIR/externals/engines/spark"
mkdir -p "$DISTDIR/externals/engines/trino"
mkdir -p "$DISTDIR/externals/engines/hive"
-mkdir -p "$DISTDIR/externals/engines/hive/jars"
mkdir -p "$DISTDIR/beeline-jars"
echo "Kyuubi $VERSION $GITREVSTRING built for" > "$DISTDIR/RELEASE"
echo "Java $JAVA_VERSION" >> "$DISTDIR/RELEASE"
@@ -235,9 +234,6 @@ cp -r "$KYUUBI_HOME/kyuubi-assembly/target/scala-$SCALA_VERSION/jars/" "$DISTDIR
# Copy kyuubi beeline jars
cp "$KYUUBI_HOME"/kyuubi-hive-beeline/target/*.jar "$DISTDIR/beeline-jars/"
-# Copy kyuubi trino client jars
-cp -r "$KYUUBI_HOME/externals/kyuubi-trino-engine/target/scala-$SCALA_VERSION/jars/" "$DISTDIR/externals/engines/trino/jars"
-
# Share the jars between server and beeline to reduce binary size
cd $DISTDIR/beeline-jars
for jar in $(ls "$DISTDIR/jars/"); do
@@ -257,9 +253,8 @@ cp "$KYUUBI_HOME/externals/kyuubi-flink-sql-engine/target/kyuubi-flink-sql-engin
cp "$KYUUBI_HOME/externals/kyuubi-spark-sql-engine/target/kyuubi-spark-sql-engine_${SCALA_VERSION}-${VERSION}.jar" "$DISTDIR/externals/engines/spark"
# Copy trino engines
-cp -r "$KYUUBI_HOME/externals/kyuubi-trino-engine/bin/" "$DISTDIR/externals/engines/trino/bin/"
-chmod a+x "$DISTDIR/externals/engines/trino/bin/trino-engine.sh"
-cp "$KYUUBI_HOME/externals/kyuubi-trino-engine/target/kyuubi-trino-engine_${SCALA_VERSION}-${VERSION}.jar" "$DISTDIR/externals/engines/trino/jars"
+cp "$KYUUBI_HOME/externals/kyuubi-trino-engine/target/kyuubi-trino-engine_${SCALA_VERSION}-${VERSION}.jar" "$DISTDIR/externals/engines/trino"
+cp -r "$KYUUBI_HOME/externals/kyuubi-trino-engine/target/scala-$SCALA_VERSION/jars/" "$DISTDIR/externals/engines/trino"
# Copy hive engines
cp "$KYUUBI_HOME/externals/kyuubi-hive-sql-engine/target/kyuubi-hive-sql-engine_${SCALA_VERSION}-${VERSION}.jar" "$DISTDIR/externals/engines/hive"
diff --git a/externals/kyuubi-trino-engine/bin/trino-engine.sh b/externals/kyuubi-trino-engine/bin/trino-engine.sh
deleted file mode 100755
index be957bc50..000000000
--- a/externals/kyuubi-trino-engine/bin/trino-engine.sh
+++ /dev/null
@@ -1,42 +0,0 @@
-#!/usr/bin/env bash
-#
-# Licensed to the Apache Software Foundation (ASF) under one or more
-# contributor license agreements. See the NOTICE file distributed with
-# this work for additional information regarding copyright ownership.
-# The ASF licenses this file to You under the Apache License, Version 2.0
-# (the "License"); you may not use this file except in compliance with
-# the License. You may obtain a copy of the License at
-#
-# http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-#
-if [[ -z ${JAVA_HOME} ]]; then
- echo "[ERROR] JAVA_HOME IS NOT SET! CANNOT PROCEED."
- exit 1
-fi
-
-RUNNER="${JAVA_HOME}/bin/java"
-
-if [[ "$TRINO_ENGINE_HOME" == "$KYUUBI_HOME/externals/engines/trino" ]]; then
- TRINO_CLIENT_JAR="$TRINO_ENGINE_JAR"
- TRINO_CLIENT_JARS_DIR="$TRINO_ENGINE_HOME/jars"
-else
- echo "\nTRINO_ENGINE_HOME $TRINO_ENGINE_HOME doesn't match production directory, assuming in development environment..."
- TRINO_CLIENT_JAR=$(find $TRINO_ENGINE_HOME/target -regex '.*/kyuubi-trino-engine_.*.jar$' | grep -v '\-sources.jar$' | grep -v '\-javadoc.jar$' | grep -v '\-tests.jar$')
- TRINO_CLIENT_JARS_DIR=$(find $TRINO_ENGINE_HOME/target -regex '.*/jars')
-fi
-
-TRINO_CLIENT_CLASSPATH="$TRINO_CLIENT_JARS_DIR/*"
-FULL_CLASSPATH="$TRINO_CLIENT_CLASSPATH:$TRINO_CLIENT_JAR"
-
-if [ -n "$TRINO_CLIENT_JAR" ]; then
- exec $RUNNER ${TRINO_ENGINE_DYNAMIC_ARGS} -cp ${FULL_CLASSPATH} org.apache.kyuubi.engine.trino.TrinoSqlEngine "$@"
-else
- (>&2 echo "[ERROR] TRINO Engine JAR file 'kyuubi-trino-engine*.jar' should be located in $TRINO_ENGINE_HOME/jars.")
- exit 1
-fi
diff --git a/externals/kyuubi-trino-engine/pom.xml b/externals/kyuubi-trino-engine/pom.xml
index 5fa0d9ca9..b5ad5267f 100644
--- a/externals/kyuubi-trino-engine/pom.xml
+++ b/externals/kyuubi-trino-engine/pom.xml
@@ -44,6 +44,12 @@
<groupId>org.apache.kyuubi</groupId>
<artifactId>kyuubi-ha_${scala.binary.version}</artifactId>
<version>${project.version}</version>
+ <exclusions>
+ <exclusion>
+ <groupId>com.google.guava</groupId>
+ <artifactId>guava</artifactId>
+ </exclusion>
+ </exclusions>
</dependency>
<dependency>
@@ -111,5 +117,4 @@
</plugin>
</plugins>
</build>
-
</project>
diff --git a/externals/kyuubi-trino-engine/src/main/java/org/apache/kyuubi/engine/trino/util/PreconditionsWrapper.java b/externals/kyuubi-trino-engine/src/main/java/org/apache/kyuubi/engine/trino/util/PreconditionsWrapper.java
deleted file mode 100644
index 5f0a642cd..000000000
--- a/externals/kyuubi-trino-engine/src/main/java/org/apache/kyuubi/engine/trino/util/PreconditionsWrapper.java
+++ /dev/null
@@ -1,31 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.kyuubi.engine.trino.util;
-
-import com.google.common.base.Preconditions;
-
-public class PreconditionsWrapper {
- /**
- * To avoid ambiguous reference to overloaded definition in scala. {@link
- * Preconditions#checkArgument(boolean, Object)} {@link Preconditions#checkArgument(boolean,
- * String, Object...)}
- */
- public static void checkArgument(boolean expression, Object errorMessage) {
- Preconditions.checkArgument(expression, errorMessage);
- }
-}
diff --git a/externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/schema/RowSet.scala b/externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/schema/RowSet.scala
index 144d476a9..6e23a3e1f 100644
--- a/externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/schema/RowSet.scala
+++ b/externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/schema/RowSet.scala
@@ -48,7 +48,6 @@ import org.apache.hive.service.rpc.thrift.TRowSet
import org.apache.hive.service.rpc.thrift.TStringColumn
import org.apache.hive.service.rpc.thrift.TStringValue
-import org.apache.kyuubi.engine.trino.util.PreconditionsWrapper._
import org.apache.kyuubi.util.RowSetUtils.bitSetToBuffer
object RowSet {
@@ -262,7 +261,7 @@ object RowSet {
"\"" + s + "\""
case (list: java.util.List[_], ARRAY) =>
- checkArgument(
+ require(
typ.getArgumentsAsTypeSignatures.asScala.nonEmpty,
"Missing ARRAY argument type")
val listType = typ.getArgumentsAsTypeSignatures.get(0)
@@ -271,7 +270,7 @@ object RowSet {
.mkString("[", ",", "]")
case (m: java.util.Map[_, _], MAP) =>
- checkArgument(
+ require(
typ.getArgumentsAsTypeSignatures.size() == 2,
"Mismatched number of MAP argument types")
val keyType = typ.getArgumentsAsTypeSignatures.get(0)
@@ -281,7 +280,7 @@ object RowSet {
}.toSeq.sorted.mkString("{", ",", "}")
case (row: Row, ROW) =>
- checkArgument(
+ require(
row.getFields.size() == typ.getArguments.size(),
"Mismatched data values and ROW type")
row.getFields.asScala.zipWithIndex.map { case (r, index) =>
diff --git a/externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/session/TrinoSessionImpl.scala b/externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/session/TrinoSessionImpl.scala
index 9325f4386..e53ecec7b 100644
--- a/externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/session/TrinoSessionImpl.scala
+++ b/externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/session/TrinoSessionImpl.scala
@@ -31,7 +31,7 @@ import org.apache.hive.service.rpc.thrift.TProtocolVersion
import org.apache.kyuubi.KyuubiSQLException
import org.apache.kyuubi.Utils.currentUser
-import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.{KyuubiConf, KyuubiReservedKeys}
import org.apache.kyuubi.engine.trino.TrinoConf
import org.apache.kyuubi.engine.trino.TrinoContext
import org.apache.kyuubi.session.AbstractSession
@@ -71,7 +71,8 @@ class TrinoSessionImpl(
throw KyuubiSQLException("Trino server url can not be null!"))
val catalog = sessionConf.get(KyuubiConf.ENGINE_TRINO_CONNECTION_CATALOG).getOrElse(
throw KyuubiSQLException("Trino default catalog can not be null!"))
- val user = sessionConf.getOption("kyuubi.trino.user").getOrElse(currentUser)
+ val user = sessionConf
+ .getOption(KyuubiReservedKeys.KYUUBI_SESSION_USER_KEY).getOrElse(currentUser)
val clientRequestTimeout = sessionConf.get(TrinoConf.CLIENT_REQUEST_TIMEOUT)
new ClientSession(
diff --git a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/ProcBuilder.scala b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/ProcBuilder.scala
index 2fd795178..d6cf9426e 100644
--- a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/ProcBuilder.scala
+++ b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/ProcBuilder.scala
@@ -18,6 +18,7 @@
package org.apache.kyuubi.engine
import java.io.{File, FilenameFilter, IOException}
+import java.net.URI
import java.nio.charset.StandardCharsets
import java.nio.file.{Files, Path, Paths}
@@ -26,8 +27,9 @@ import scala.collection.JavaConverters._
import com.google.common.collect.EvictingQueue
import org.apache.commons.lang3.StringUtils.containsIgnoreCase
-import org.apache.kyuubi.{KyuubiSQLException, Logging, Utils}
+import org.apache.kyuubi._
import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf.KYUUBI_HOME
import org.apache.kyuubi.operation.log.OperationLog
import org.apache.kyuubi.util.NamedThreadFactory
@@ -35,12 +37,58 @@ trait ProcBuilder {
import ProcBuilder._
- protected def executable: String
+ /**
+ * The short name of the engine process builder, we use this for form the engine jar paths now
+ * see `mainResource`
+ */
+ protected def shortName: String
- protected def mainResource: Option[String]
+ /**
+ * executable, it is `JAVA_HOME/bin/java` by default
+ */
+ protected def executable: String = {
+ val javaHome = env.get("JAVA_HOME")
+ if (javaHome.isEmpty) {
+ throw validateEnv("JAVA_HOME")
+ } else {
+ Paths.get(javaHome.get, "bin", "java").toString
+ }
+ }
+
+ /**
+ * The engine jar or other runnable jar containing the main method
+ */
+ def mainResource: Option[String] = {
+ // 1. get the main resource jar for user specified config first
+ // TODO use SPARK_SCALA_VERSION instead of SCALA_COMPILE_VERSION
+ val jarName = s"${module}_$SCALA_COMPILE_VERSION-$KYUUBI_VERSION.jar"
+ conf.getOption(s"kyuubi.session.engine.$shortName.main.resource").filter { userSpecified =>
+ // skip check exist if not local file.
+ val uri = new URI(userSpecified)
+ val schema = if (uri.getScheme != null) uri.getScheme else "file"
+ schema match {
+ case "file" => Files.exists(Paths.get(userSpecified))
+ case _ => true
+ }
+ }.orElse {
+ // 2. get the main resource jar from system build default
+ env.get(KYUUBI_HOME)
+ .map { Paths.get(_, "externals", "engines", shortName, jarName) }
+ .filter(Files.exists(_)).map(_.toAbsolutePath.toFile.getCanonicalPath)
+ }.orElse {
+ // 3. get the main resource from dev environment
+ val cwd = Utils.getCodeSourceLocation(getClass).split("kyuubi-server")
+ assert(cwd.length > 1)
+ Option(Paths.get(cwd.head, "externals", module, "target", jarName))
+ .map(_.toAbsolutePath.toFile.getCanonicalPath)
+ }
+ }
protected def module: String
+ /**
+ * The class containing the main method
+ */
protected def mainClass: String
protected def proxyUser: String
diff --git a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/flink/FlinkProcessBuilder.scala b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/flink/FlinkProcessBuilder.scala
index 992ce4c7a..8e53d9e6b 100644
--- a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/flink/FlinkProcessBuilder.scala
+++ b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/flink/FlinkProcessBuilder.scala
@@ -19,8 +19,7 @@ package org.apache.kyuubi.engine.flink
import java.io.{File, FilenameFilter}
import java.lang.ProcessBuilder.Redirect
-import java.net.URI
-import java.nio.file.{Files, Paths}
+import java.nio.file.Paths
import scala.collection.JavaConverters._
@@ -28,7 +27,6 @@ import com.google.common.annotations.VisibleForTesting
import org.apache.kyuubi._
import org.apache.kyuubi.config.KyuubiConf
-import org.apache.kyuubi.config.KyuubiConf.ENGINE_FLINK_MAIN_RESOURCE
import org.apache.kyuubi.engine.ProcBuilder
import org.apache.kyuubi.engine.flink.FlinkProcessBuilder.FLINK_ENGINE_BINARY_FILE
import org.apache.kyuubi.operation.log.OperationLog
@@ -64,31 +62,6 @@ class FlinkProcessBuilder(
}
}
- override protected def mainResource: Option[String] = {
- val jarName = s"${module}_$SCALA_COMPILE_VERSION-$KYUUBI_VERSION.jar"
- // 1. get the main resource jar for user specified config first
- conf.get(ENGINE_FLINK_MAIN_RESOURCE).filter { userSpecified =>
- // skip check exist if not local file.
- val uri = new URI(userSpecified)
- val schema = if (uri.getScheme != null) uri.getScheme else "file"
- schema match {
- case "file" => Files.exists(Paths.get(userSpecified))
- case _ => true
- }
- }.orElse {
- // 2. get the main resource jar from system build default
- env.get(KyuubiConf.KYUUBI_HOME)
- .map { Paths.get(_, "externals", "engines", "flink", jarName) }
- .filter(Files.exists(_)).map(_.toAbsolutePath.toFile.getCanonicalPath)
- }.orElse {
- // 3. get the main resource from dev environment
- Option(Paths.get("externals", module, "target", jarName))
- .filter(Files.exists(_)).orElse {
- Some(Paths.get("..", "externals", module, "target", jarName))
- }.map(_.toAbsolutePath.toFile.getCanonicalPath)
- }
- }
-
override protected def module: String = "kyuubi-flink-sql-engine"
override protected def mainClass: String = "org.apache.kyuubi.engine.flink.FlinkSQLEngine"
@@ -159,7 +132,7 @@ class FlinkProcessBuilder(
}
}
- private def useKeytab(): Boolean = false
+ override protected def shortName: String = "flink"
}
object FlinkProcessBuilder {
diff --git a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/hive/HiveProcessBuilder.scala b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/hive/HiveProcessBuilder.scala
index 4ee0e5f43..6dd2c5069 100644
--- a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/hive/HiveProcessBuilder.scala
+++ b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/hive/HiveProcessBuilder.scala
@@ -18,8 +18,7 @@
package org.apache.kyuubi.engine.hive
import java.io.File
-import java.net.URI
-import java.nio.file.{Files, Paths}
+import java.nio.file.Paths
import java.util.LinkedHashSet
import scala.collection.JavaConverters._
@@ -27,7 +26,6 @@ import scala.collection.mutable.ArrayBuffer
import org.apache.kyuubi._
import org.apache.kyuubi.config.KyuubiConf
-import org.apache.kyuubi.config.KyuubiConf.ENGINE_HIVE_MAIN_RESOURCE
import org.apache.kyuubi.config.KyuubiReservedKeys.KYUUBI_SESSION_USER_KEY
import org.apache.kyuubi.engine.ProcBuilder
import org.apache.kyuubi.operation.log.OperationLog
@@ -40,40 +38,6 @@ class HiveProcessBuilder(
private val hiveHome: String = getEngineHome("hive")
- override protected def executable: String = {
- val javaHome = env.get("JAVA_HOME")
- if (javaHome.isEmpty) {
- throw validateEnv("JAVA_HOME")
- } else {
- Paths.get(javaHome.get, "bin", "java").toString
- }
- }
-
- override protected def mainResource: Option[String] = {
- val jarName = s"${module}_$SCALA_COMPILE_VERSION-$KYUUBI_VERSION.jar"
- // 1. get the main resource jar for user specified config first
- conf.get(ENGINE_HIVE_MAIN_RESOURCE).filter { userSpecified =>
- // skip check exist if not local file.
- val uri = new URI(userSpecified)
- val schema = if (uri.getScheme != null) uri.getScheme else "file"
- schema match {
- case "file" => Files.exists(Paths.get(userSpecified))
- case _ => true
- }
- }.orElse {
- // 2. get the main resource jar from system build default
- env.get(KyuubiConf.KYUUBI_HOME)
- .map { Paths.get(_, "externals", "engines", "hive", jarName) }
- .filter(Files.exists(_)).map(_.toAbsolutePath.toFile.getCanonicalPath)
- }.orElse {
- // 3. get the main resource from dev environment
- Option(Paths.get("externals", module, "target", jarName))
- .filter(Files.exists(_)).orElse {
- Some(Paths.get("..", "externals", module, "target", jarName))
- }.map(_.toAbsolutePath.toFile.getCanonicalPath)
- }
- }
-
override protected def module: String = "kyuubi-hive-sql-engine"
override protected def mainClass: String = "org.apache.kyuubi.engine.hive.HiveSQLEngine"
@@ -123,4 +87,6 @@ class HiveProcessBuilder(
}
override def toString: String = commands.mkString("\n")
+
+ override protected def shortName: String = "hive"
}
diff --git a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/spark/SparkProcessBuilder.scala b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/spark/SparkProcessBuilder.scala
index b4ed17155..18870a611 100644
--- a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/spark/SparkProcessBuilder.scala
+++ b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/spark/SparkProcessBuilder.scala
@@ -18,8 +18,7 @@
package org.apache.kyuubi.engine.spark
import java.io.IOException
-import java.net.URI
-import java.nio.file.{Files, Paths}
+import java.nio.file.Paths
import scala.collection.mutable.ArrayBuffer
import scala.util.matching.Regex
@@ -31,7 +30,6 @@ import org.apache.hadoop.yarn.conf.YarnConfiguration
import org.apache.kyuubi._
import org.apache.kyuubi.config.KyuubiConf
-import org.apache.kyuubi.config.KyuubiConf.ENGINE_SPARK_MAIN_RESOURCE
import org.apache.kyuubi.engine.ProcBuilder
import org.apache.kyuubi.ha.HighAvailabilityConf
import org.apache.kyuubi.ha.client.AuthTypes
@@ -57,33 +55,6 @@ class SparkProcessBuilder(
override def mainClass: String = "org.apache.kyuubi.engine.spark.SparkSQLEngine"
- override def mainResource: Option[String] = {
- // 1. get the main resource jar for user specified config first
- // TODO use SPARK_SCALA_VERSION instead of SCALA_COMPILE_VERSION
- val jarName = s"${module}_$SCALA_COMPILE_VERSION-$KYUUBI_VERSION.jar"
- conf.get(ENGINE_SPARK_MAIN_RESOURCE).filter { userSpecified =>
- // skip check exist if not local file.
- val uri = new URI(userSpecified)
- val schema = if (uri.getScheme != null) uri.getScheme else "file"
- schema match {
- case "file" => Files.exists(Paths.get(userSpecified))
- case _ => true
- }
- }.orElse {
- // 2. get the main resource jar from system build default
- env.get(KyuubiConf.KYUUBI_HOME)
- .map { Paths.get(_, "externals", "engines", "spark", jarName) }
- .filter(Files.exists(_)).map(_.toAbsolutePath.toFile.getCanonicalPath)
- }.orElse {
- // 3. get the main resource from dev environment
- val cwd = Utils.getCodeSourceLocation(getClass)
- .split("kyuubi-server")
- assert(cwd.length > 1)
- Option(Paths.get(cwd.head, "externals", module, "target", jarName))
- .map(_.toAbsolutePath.toFile.getCanonicalPath)
- }
- }
-
override protected def commands: Array[String] = {
val buffer = new ArrayBuffer[String]()
buffer += executable
@@ -192,6 +163,7 @@ class SparkProcessBuilder(
case None => ""
}
+ override protected def shortName: String = "spark"
}
object SparkProcessBuilder {
diff --git a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/trino/TrinoProcessBuilder.scala b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/trino/TrinoProcessBuilder.scala
index 75f39082a..510cc7864 100644
--- a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/trino/TrinoProcessBuilder.scala
+++ b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/trino/TrinoProcessBuilder.scala
@@ -17,17 +17,18 @@
package org.apache.kyuubi.engine.trino
-import java.net.URI
-import java.nio.file.Files
+import java.io.File
import java.nio.file.Paths
+import java.util.LinkedHashSet
-import org.apache.kyuubi.{KYUUBI_VERSION, KyuubiSQLException, Logging, SCALA_COMPILE_VERSION, Utils}
+import scala.collection.JavaConverters._
+import scala.collection.mutable.ArrayBuffer
+
+import org.apache.kyuubi.{Logging, SCALA_COMPILE_VERSION, Utils}
import org.apache.kyuubi.config.KyuubiConf
-import org.apache.kyuubi.config.KyuubiConf.ENGINE_TRINO_CONNECTION_CATALOG
-import org.apache.kyuubi.config.KyuubiConf.ENGINE_TRINO_CONNECTION_URL
-import org.apache.kyuubi.config.KyuubiConf.ENGINE_TRINO_MAIN_RESOURCE
+import org.apache.kyuubi.config.KyuubiConf.{ENGINE_TRINO_CONNECTION_CATALOG, ENGINE_TRINO_CONNECTION_URL}
+import org.apache.kyuubi.config.KyuubiReservedKeys.KYUUBI_SESSION_USER_KEY
import org.apache.kyuubi.engine.ProcBuilder
-import org.apache.kyuubi.engine.trino.TrinoProcessBuilder._
import org.apache.kyuubi.operation.log.OperationLog
class TrinoProcessBuilder(
@@ -35,83 +36,58 @@ class TrinoProcessBuilder(
override val conf: KyuubiConf,
val extraEngineLog: Option[OperationLog] = None) extends ProcBuilder with Logging {
- private[trino] lazy val trinoConf: Map[String, String] = {
- assert(
- conf.get(ENGINE_TRINO_CONNECTION_URL).isDefined,
- throw KyuubiSQLException(
- s"Trino server url can not be null! Please set ${ENGINE_TRINO_CONNECTION_URL.key}"))
- assert(
- conf.get(ENGINE_TRINO_CONNECTION_CATALOG).isDefined,
- throw KyuubiSQLException(
- s"Trino default catalog can not be null!" +
- s" Please set ${ENGINE_TRINO_CONNECTION_CATALOG.key}"))
+ override protected def module: String = "kyuubi-trino-engine"
- conf.getAll.filter { case (k, v) =>
- !k.startsWith("spark.") && !k.startsWith("hadoop.")
- } + (USER -> proxyUser)
- }
+ override protected def mainClass: String = "org.apache.kyuubi.engine.trino.TrinoSqlEngine"
- override protected val executable: String = {
- val trinoHomeOpt = env.get("TRINO_ENGINE_HOME").orElse {
- val cwd = Utils.getCodeSourceLocation(getClass)
- .split("kyuubi-server")
- assert(cwd.length > 1)
- Option(
- Paths.get(cwd.head)
- .resolve("externals")
- .resolve(module)
- .toFile)
- .map(_.getAbsolutePath)
- }
+ override protected def commands: Array[String] = {
+ require(
+ conf.get(ENGINE_TRINO_CONNECTION_URL).nonEmpty,
+ s"Trino server url can not be null! Please set ${ENGINE_TRINO_CONNECTION_URL.key}")
+ require(
+ conf.get(ENGINE_TRINO_CONNECTION_CATALOG).nonEmpty,
+ s"Trino default catalog can not be null! Please set ${ENGINE_TRINO_CONNECTION_CATALOG.key}")
+ val buffer = new ArrayBuffer[String]()
+ buffer += executable
- trinoHomeOpt.map { dir =>
- Paths.get(dir, "bin", TRINO_ENGINE_BINARY_FILE).toAbsolutePath.toFile.getCanonicalPath
- }.getOrElse {
- throw KyuubiSQLException("TRINO_ENGINE_HOME is not set! " +
- "For more detail information on installing and configuring Trino, please visit " +
- "https://kyuubi.apache.org/docs/latest/deployment/settings.html#environments")
- }
- }
+ // 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"
- override protected def mainResource: Option[String] = {
- val jarName = s"${module}_$SCALA_COMPILE_VERSION-$KYUUBI_VERSION.jar"
- // 1. get the main resource jar for user specified config first
- conf.get(ENGINE_TRINO_MAIN_RESOURCE).filter { userSpecified =>
- // skip check exist if not local file.
- val uri = new URI(userSpecified)
- val schema = if (uri.getScheme != null) uri.getScheme else "file"
- schema match {
- case "file" => Files.exists(Paths.get(userSpecified))
- case _ => true
- }
- }.orElse {
- // 2. get the main resource jar from system build default
- env.get(KyuubiConf.KYUUBI_HOME)
- .map { Paths.get(_, "externals", "engines", "trino", "jars", jarName) }
- .filter(Files.exists(_)).map(_.toAbsolutePath.toFile.getCanonicalPath)
- }.orElse {
- // 3. get the main resource from dev environment
- Option(Paths.get("externals", module, "target", jarName))
- .filter(Files.exists(_)).orElse {
- Some(Paths.get("..", "externals", module, "target", jarName))
- }.map(_.toAbsolutePath.toFile.getCanonicalPath)
+ // TODO: add Kyuubi.engineEnv.TRINO_ENGINE_MEMORY or kyuubi.engine.trino.memory to configure
+ // -Xmx5g
+ // java options
+ for ((k, v) <- conf.getAll) {
+ buffer += s"-D$k=$v"
}
- }
-
- override protected def module: String = "kyuubi-trino-engine"
- override protected def mainClass: String = "org.apache.kyuubi.engine.trino.TrinoSqlEngine"
+ buffer += "-cp"
+ val classpathEntries = new LinkedHashSet[String]
+ // trino engine runtime jar
+ mainResource.foreach(classpathEntries.add)
- override protected def childProcEnv: Map[String, String] = conf.getEnvs +
- ("TRINO_ENGINE_JAR" -> mainResource.get) +
- ("TRINO_ENGINE_DYNAMIC_ARGS" ->
- trinoConf.map { case (k, v) => s"-D$k=$v" }.mkString(" "))
+ mainResource.foreach { path =>
+ val parent = Paths.get(path).getParent
+ if (Utils.isTesting) {
+ // add dev classpath
+ val trinoDeps = parent
+ .resolve(s"scala-$SCALA_COMPILE_VERSION")
+ .resolve("jars")
+ classpathEntries.add(s"$trinoDeps${File.separator}*")
+ } else {
+ // add prod classpath
+ classpathEntries.add(s"$parent${File.separator}*")
+ }
+ }
- override protected def commands: Array[String] = Array(executable)
-}
+ buffer += classpathEntries.asScala.mkString(File.pathSeparator)
+ buffer += mainClass
+ buffer.toArray
+ }
-object TrinoProcessBuilder {
- final private val USER = "kyuubi.trino.user"
+ override protected def shortName: String = "trino"
- final private val TRINO_ENGINE_BINARY_FILE = "trino-engine.sh"
+ override def toString: String = commands.mkString("\n")
}
diff --git a/kyuubi-server/src/test/scala/org/apache/kyuubi/engine/trino/TrinoProcessBuilderSuite.scala b/kyuubi-server/src/test/scala/org/apache/kyuubi/engine/trino/TrinoProcessBuilderSuite.scala
index f0b5bd443..64fc46acb 100644
--- a/kyuubi-server/src/test/scala/org/apache/kyuubi/engine/trino/TrinoProcessBuilderSuite.scala
+++ b/kyuubi-server/src/test/scala/org/apache/kyuubi/engine/trino/TrinoProcessBuilderSuite.scala
@@ -18,22 +18,25 @@
package org.apache.kyuubi.engine.trino
import org.apache.kyuubi.KyuubiFunSuite
-import org.apache.kyuubi.KyuubiSQLException
import org.apache.kyuubi.config.KyuubiConf
-import org.apache.kyuubi.config.KyuubiConf.ENGINE_TRINO_CONNECTION_URL
+import org.apache.kyuubi.config.KyuubiConf._
class TrinoProcessBuilderSuite extends KyuubiFunSuite {
- private def conf = KyuubiConf().set("kyuubi.on", "off")
test("trino process builder") {
+ val conf = KyuubiConf()
+ .set(ENGINE_TRINO_CONNECTION_URL, "dummy_url")
+ .set(ENGINE_TRINO_CONNECTION_CATALOG, "dummy_catalog")
val builder = new TrinoProcessBuilder("kyuubi", conf)
- val commands = builder.toString.split(' ')
- assert(commands.exists(_.endsWith("trino-engine.sh")))
+ val commands = builder.toString.split("\n")
+ assert(commands.head.endsWith("java"))
+ assert(commands.contains(s"-D${ENGINE_TRINO_CONNECTION_URL.key}=dummy_url"))
}
test("capture error from trino process builder") {
- val e1 = intercept[KyuubiSQLException](new TrinoProcessBuilder("kyuubi", conf).trinoConf)
- assert(e1.getMessage ===
+ val e1 = intercept[IllegalArgumentException](
+ new TrinoProcessBuilder("kyuubi", KyuubiConf()).processBuilder)
+ assert(e1.getMessage contains
s"Trino server url can not be null! Please set ${ENGINE_TRINO_CONNECTION_URL.key}")
}
}