[incubator-kyuubi] branch master updated: [KYUUBI #2361] [Improvement] Configuring Trino Engine heap memory and java opts

[ya...@apache.org]

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 1a68b866c [KYUUBI #2361] [Improvement] Configuring Trino Engine heap memory and java opts
1a68b866c is described below

commit 1a68b866cecc0b528e423e3d39ff8894aca9285f
Author: Min Zhao <zh...@163.com>
AuthorDate: Sat Apr 16 22:03:17 2022 +0800

    [KYUUBI #2361] [Improvement] Configuring Trino Engine heap memory and java opts
    
    ### _Why are the changes needed?_
    
    Configuring Trino Engine heap memory and java opts
    
    ### _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
    
    - [ ] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request
    
    Closes #2387 from zhaomin1423/2361.
    
    Closes #2361
    
    e8ef7f6f [Min Zhao] [KYUUBI #2361] [Improvement] Configuring Trino Engine heap memory and java opts
    
    Authored-by: Min Zhao <zh...@163.com>
    Signed-off-by: Kent Yao <ya...@apache.org>
---
 docs/deployment/settings.md                        |  3 ++
 .../org/apache/kyuubi/config/KyuubiConf.scala      | 22 ++++++++++++
 .../kyuubi/engine/trino/TrinoProcessBuilder.scala  | 12 +++++--
 .../engine/trino/TrinoProcessBuilderSuite.scala    | 41 ++++++++++++++++++++++
 4 files changed, 76 insertions(+), 2 deletions(-)

diff --git a/docs/deployment/settings.md b/docs/deployment/settings.md
index 6962dc0bb..17b07ced3 100644
--- a/docs/deployment/settings.md
+++ b/docs/deployment/settings.md
@@ -213,6 +213,9 @@ Key | Default | Meaning | Type | Since
 <code>kyuubi.engine.share.level.sub.domain</code>|<div style='width: 65pt;word-wrap: break-word;white-space: normal'>&lt;undefined&gt;</div>|<div style='width: 170pt;word-wrap: break-word;white-space: normal'>(deprecated) - Using kyuubi.engine.share.level.subdomain instead</div>|<div style='width: 30pt'>string</div>|<div style='width: 20pt'>1.2.0</div>
 <code>kyuubi.engine.share.level.subdomain</code>|<div style='width: 65pt;word-wrap: break-word;white-space: normal'>&lt;undefined&gt;</div>|<div style='width: 170pt;word-wrap: break-word;white-space: normal'>Allow end-users to create a subdomain for the share level of an engine. A subdomain is a case-insensitive string values that must be a valid zookeeper sub path. For example, for `USER` share level, an end-user can share a certain engine within a subdomain, not for all of its clients. [...]
 <code>kyuubi.engine.single.spark.session</code>|<div style='width: 65pt;word-wrap: break-word;white-space: normal'>false</div>|<div style='width: 170pt;word-wrap: break-word;white-space: normal'>When set to true, this engine is running in a single session mode. All the JDBC/ODBC connections share the temporary views, function registries, SQL configuration and the current database.</div>|<div style='width: 30pt'>boolean</div>|<div style='width: 20pt'>1.3.0</div>
+<code>kyuubi.engine.trino.extra.classpath</code>|<div style='width: 65pt;word-wrap: break-word;white-space: normal'>&lt;undefined&gt;</div>|<div style='width: 170pt;word-wrap: break-word;white-space: normal'>The extra classpath for the trino query engine, for configuring other libs which may need by the trino engine </div>|<div style='width: 30pt'>string</div>|<div style='width: 20pt'>1.6.0</div>
+<code>kyuubi.engine.trino.java.options</code>|<div style='width: 65pt;word-wrap: break-word;white-space: normal'>&lt;undefined&gt;</div>|<div style='width: 170pt;word-wrap: break-word;white-space: normal'>The extra java options for the trino query engine</div>|<div style='width: 30pt'>string</div>|<div style='width: 20pt'>1.6.0</div>
+<code>kyuubi.engine.trino.memory</code>|<div style='width: 65pt;word-wrap: break-word;white-space: normal'>1g</div>|<div style='width: 170pt;word-wrap: break-word;white-space: normal'>The heap memory for the trino query engine</div>|<div style='width: 30pt'>string</div>|<div style='width: 20pt'>1.6.0</div>
 <code>kyuubi.engine.type</code>|<div style='width: 65pt;word-wrap: break-word;white-space: normal'>SPARK_SQL</div>|<div style='width: 170pt;word-wrap: break-word;white-space: normal'>Specify the detailed engine that supported by the Kyuubi. The engine type bindings to SESSION scope. This configuration is experimental. Currently, available configs are: <ul> <li>SPARK_SQL: specify this engine type will launch a Spark engine which can provide all the capacity of the Apache Spark. Note, it's [...]
 <code>kyuubi.engine.ui.retainedSessions</code>|<div style='width: 65pt;word-wrap: break-word;white-space: normal'>200</div>|<div style='width: 170pt;word-wrap: break-word;white-space: normal'>The number of SQL client sessions kept in the Kyuubi Query Engine web UI.</div>|<div style='width: 30pt'>int</div>|<div style='width: 20pt'>1.4.0</div>
 <code>kyuubi.engine.ui.retainedStatements</code>|<div style='width: 65pt;word-wrap: break-word;white-space: normal'>200</div>|<div style='width: 170pt;word-wrap: break-word;white-space: normal'>The number of statements kept in the Kyuubi Query Engine web UI.</div>|<div style='width: 30pt'>int</div>|<div style='width: 20pt'>1.4.0</div>
diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
index 81bc3259b..273831b06 100644
--- a/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
+++ b/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
@@ -1346,4 +1346,26 @@ object KyuubiConf {
       .version("1.6.0")
       .stringConf
       .createWithDefault("yyyy-MM-dd HH:mm:ss.SSS")
+
+  val ENGINE_TRINO_MEMORY: ConfigEntry[String] =
+    buildConf("kyuubi.engine.trino.memory")
+      .doc("The heap memory for the trino query engine")
+      .version("1.6.0")
+      .stringConf
+      .createWithDefault("1g")
+
+  val ENGINE_TRINO_JAVA_OPTIONS: OptionalConfigEntry[String] =
+    buildConf("kyuubi.engine.trino.java.options")
+      .doc("The extra java options for the trino query engine")
+      .version("1.6.0")
+      .stringConf
+      .createOptional
+
+  val ENGINE_TRINO_EXTRA_CLASSPATH: OptionalConfigEntry[String] =
+    buildConf("kyuubi.engine.trino.extra.classpath")
+      .doc("The extra classpath for the trino query engine, " +
+        "for configuring other libs which may need by the trino engine ")
+      .version("1.6.0")
+      .stringConf
+      .createOptional
 }
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 510cc7864..cb33f8f40 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
@@ -26,7 +26,7 @@ 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, ENGINE_TRINO_CONNECTION_URL}
+import org.apache.kyuubi.config.KyuubiConf.{ENGINE_TRINO_CONNECTION_CATALOG, ENGINE_TRINO_CONNECTION_URL, ENGINE_TRINO_EXTRA_CLASSPATH, ENGINE_TRINO_JAVA_OPTIONS, ENGINE_TRINO_MEMORY}
 import org.apache.kyuubi.config.KyuubiReservedKeys.KYUUBI_SESSION_USER_KEY
 import org.apache.kyuubi.engine.ProcBuilder
 import org.apache.kyuubi.operation.log.OperationLog
@@ -56,7 +56,12 @@ class TrinoProcessBuilder(
     // or just leave it, because we can handle it at operation layer
     buffer += s"-D$KYUUBI_SESSION_USER_KEY=$proxyUser"
 
-    // TODO: add Kyuubi.engineEnv.TRINO_ENGINE_MEMORY or kyuubi.engine.trino.memory to configure
+    val memory = conf.get(ENGINE_TRINO_MEMORY)
+    buffer += s"-Xmx$memory"
+    val javaOptions = conf.get(ENGINE_TRINO_JAVA_OPTIONS)
+    if (javaOptions.isDefined) {
+      buffer += javaOptions.get
+    }
     // -Xmx5g
     // java options
     for ((k, v) <- conf.getAll) {
@@ -82,6 +87,9 @@ class TrinoProcessBuilder(
       }
     }
 
+    val extraCp = conf.get(ENGINE_TRINO_EXTRA_CLASSPATH)
+    extraCp.foreach(classpathEntries.add)
+
     buffer += classpathEntries.asScala.mkString(File.pathSeparator)
     buffer += mainClass
     buffer.toArray
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 64fc46acb..7afc1ed06 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
@@ -39,4 +39,45 @@ class TrinoProcessBuilderSuite extends KyuubiFunSuite {
     assert(e1.getMessage contains
       s"Trino server url can not be null! Please set ${ENGINE_TRINO_CONNECTION_URL.key}")
   }
+
+  test("default engine memory") {
+    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("\n")
+    assert(commands.contains("-Xmx1g"))
+  }
+
+  test("set engine memory") {
+    val conf = KyuubiConf()
+      .set(ENGINE_TRINO_CONNECTION_URL, "dummy_url")
+      .set(ENGINE_TRINO_CONNECTION_CATALOG, "dummy_catalog")
+      .set(ENGINE_TRINO_MEMORY, "5g")
+    val builder = new TrinoProcessBuilder("kyuubi", conf)
+    val commands = builder.toString.split("\n")
+    assert(commands.contains("-Xmx5g"))
+  }
+
+  test("set engine java options") {
+    val conf = KyuubiConf()
+      .set(ENGINE_TRINO_CONNECTION_URL, "dummy_url")
+      .set(ENGINE_TRINO_CONNECTION_CATALOG, "dummy_catalog")
+      .set(
+        ENGINE_TRINO_JAVA_OPTIONS,
+        "-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=5005")
+    val builder = new TrinoProcessBuilder("kyuubi", conf)
+    val commands = builder.toString.split("\n")
+    assert(commands.contains("-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=5005"))
+  }
+
+  test("set extra classpath") {
+    val conf = KyuubiConf()
+      .set(ENGINE_TRINO_CONNECTION_URL, "dummy_url")
+      .set(ENGINE_TRINO_CONNECTION_CATALOG, "dummy_catalog")
+      .set(ENGINE_TRINO_EXTRA_CLASSPATH, "/dummy_classpath/*")
+    val builder = new TrinoProcessBuilder("kyuubi", conf)
+    val commands = builder.toString
+    assert(commands.contains("/dummy_classpath/*"))
+  }
 }