You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by GitBox <gi...@apache.org> on 2022/05/06 14:04:51 UTC

[GitHub] [incubator-kyuubi] jiaoqingbo opened a new pull request, #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

jiaoqingbo opened a new pull request, #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579

   <!--
   Thanks for sending a pull request!
   
   Here are some tips for you:
     1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/contributions.html
     2. If the PR is related to an issue in https://github.com/apache/incubator-kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'.
     3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'.
   -->
   
   ### _Why are the changes needed?_
   <!--
   Please clarify why the changes are needed. For instance,
     1. If you add a feature, you can talk about the use case of it.
     2. If you fix a bug, you can clarify why it is a bug.
   -->
   fix #2554 
   
   ### _How was this patch tested?_
   - [ ] 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
   


-- 
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


[GitHub] [incubator-kyuubi] zhaomin1423 commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
zhaomin1423 commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r866893392


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/flink/FlinkProcessBuilder.scala:
##########
@@ -80,13 +86,17 @@ class FlinkProcessBuilder(
     env.get("HADOOP_CONF_DIR").foreach(classpathEntries.add)
     env.get("YARN_CONF_DIR").foreach(classpathEntries.add)
     env.get("HBASE_CONF_DIR").foreach(classpathEntries.add)
-    val hadoopClasspath = env.get("HADOOP_CLASSPATH")
-    if (hadoopClasspath.isEmpty) {
-      throw KyuubiSQLException("HADOOP_CLASSPATH is not set! " +
-        "For more detail information on configuring HADOOP_CLASSPATH" +
-        "https://kyuubi.apache.org/docs/latest/deployment/settings.html#environments")
+    val hadoopCp = env.get(FLINK_HADOOP_CLASSPATH)
+    hadoopCp.foreach(classpathEntries.add)
+    val extraCp = conf.get(ENGINE_FLINK_EXTRA_CLASSPATH)
+    extraCp.foreach(classpathEntries.add)
+    if (hadoopCp.isEmpty && extraCp.isEmpty) {
+      throw new KyuubiException(s"The conf of ${FLINK_HADOOP_CLASSPATH} or " +

Review Comment:
   or -> and



##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/flink/FlinkProcessBuilder.scala:
##########
@@ -80,13 +86,17 @@ class FlinkProcessBuilder(
     env.get("HADOOP_CONF_DIR").foreach(classpathEntries.add)
     env.get("YARN_CONF_DIR").foreach(classpathEntries.add)
     env.get("HBASE_CONF_DIR").foreach(classpathEntries.add)
-    val hadoopClasspath = env.get("HADOOP_CLASSPATH")
-    if (hadoopClasspath.isEmpty) {
-      throw KyuubiSQLException("HADOOP_CLASSPATH is not set! " +
-        "For more detail information on configuring HADOOP_CLASSPATH" +
-        "https://kyuubi.apache.org/docs/latest/deployment/settings.html#environments")
+    val hadoopCp = env.get(FLINK_HADOOP_CLASSPATH)
+    hadoopCp.foreach(classpathEntries.add)
+    val extraCp = conf.get(ENGINE_FLINK_EXTRA_CLASSPATH)
+    extraCp.foreach(classpathEntries.add)
+    if (hadoopCp.isEmpty && extraCp.isEmpty) {
+      throw new KyuubiException(s"The conf of ${FLINK_HADOOP_CLASSPATH} or " +
+        s"${ENGINE_FLINK_EXTRA_CLASSPATH} is empty." +
+        s"Please set ${FLINK_HADOOP_CLASSPATH} or ${ENGINE_FLINK_EXTRA_CLASSPATH} for " +

Review Comment:
   the conf should be conf.key, for example, FLINK_HADOOP_CLASSPATH should be FLINK_HADOOP_CLASSPATH.key



##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -1437,6 +1437,28 @@ object KyuubiConf {
       .stringConf
       .createOptional
 
+  val ENGINE_FLINK_MEMORY: ConfigEntry[String] =

Review Comment:
   The new configs should be added in the setting.md. you can refer docs/develop_tools/Append descriptions of new configurations to settings.md.



-- 
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


[GitHub] [incubator-kyuubi] jiaoqingbo commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
jiaoqingbo commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867329989


##########
kyuubi-server/src/test/scala/org/apache/kyuubi/engine/flink/FlinkProcessBuilderSuite.scala:
##########
@@ -22,26 +22,33 @@ import java.io.File
 import scala.collection.JavaConverters._
 import scala.collection.immutable.ListMap
 
-import org.apache.kyuubi.{FLINK_COMPILE_VERSION, KyuubiFunSuite, KyuubiSQLException, SCALA_COMPILE_VERSION}
+import org.apache.kyuubi.{FLINK_COMPILE_VERSION, KyuubiException, KyuubiFunSuite, SCALA_COMPILE_VERSION}
 import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf.{ENGINE_FLINK_EXTRA_CLASSPATH, ENGINE_FLINK_JAVA_OPTIONS, ENGINE_FLINK_MEMORY}
 
 class FlinkProcessBuilderSuite extends KyuubiFunSuite {
   private def conf = KyuubiConf().set("kyuubi.on", "off")
+    .set(ENGINE_FLINK_MEMORY, "1g")
+    .set(
+      ENGINE_FLINK_JAVA_OPTIONS,
+      "-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=5005")
+
   private def envDefault: ListMap[String, String] = ListMap(
     "JAVA_HOME" -> s"${File.separator}jdk1.8.0_181")

Review Comment:
   This is just for testing



-- 
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


[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867331705


##########
docs/deployment/engine_on_yarn.md:
##########
@@ -175,6 +175,24 @@ If the `TopSpeedWindowing` passes, configure it in `$KYUUBI_HOME/conf/kyuubi-env
 $ echo "export HADOOP_CONF_DIR=/path/to/hadoop/conf" >> $KYUUBI_HOME/conf/kyuubi-env.sh
 ```
 
+#### Required Environment Variable
+
+The `FLINK_HADOOP_CLASSPATH` is required, too. It should contain `commons-collections-*.jar`, 
+`hadoop-client-runtime-*.jar`, `hadoop-client-api-*.jar` and `htrace-core4-*.jar`.
+All four jars are in the `HADOOP_HOME`. 

Review Comment:
   It should be. But `hadoop classpath` always ponits to vanilla jars I think, user may suffer jar hell then.



-- 
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


[GitHub] [incubator-kyuubi] jiaoqingbo commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
jiaoqingbo commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867981694


##########
conf/kyuubi-env.sh.template:
##########
@@ -59,6 +60,7 @@
 # export SPARK_HOME=/opt/spark
 # export FLINK_HOME=/opt/flink
 # export HIVE_HOME=/opt/hive
+# export FLINK_HADOOP_CLASSPATH=/path/to/hadoop-client-runtime-*.jar:/path/to/hadoop-client-api-*.jar

Review Comment:
   The version of hadoop should not be stated here, otherwise when we upgrade the hadoop version, we need to update here every time



-- 
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


[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r866880103


##########
kyuubi-server/src/test/scala/org/apache/kyuubi/engine/flink/FlinkProcessBuilderSuite.scala:
##########
@@ -22,26 +22,33 @@ import java.io.File
 import scala.collection.JavaConverters._
 import scala.collection.immutable.ListMap
 
-import org.apache.kyuubi.{FLINK_COMPILE_VERSION, KyuubiFunSuite, KyuubiSQLException, SCALA_COMPILE_VERSION}
+import org.apache.kyuubi.{FLINK_COMPILE_VERSION, KyuubiException, KyuubiFunSuite, SCALA_COMPILE_VERSION}
 import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf.{ENGINE_FLINK_EXTRA_CLASSPATH, ENGINE_FLINK_JAVA_OPTIONS, ENGINE_FLINK_MEMORY}
 
 class FlinkProcessBuilderSuite extends KyuubiFunSuite {
   private def conf = KyuubiConf().set("kyuubi.on", "off")
+    .set(ENGINE_FLINK_MEMORY, "5g")

Review Comment:
   The whole container of GitHub Action has 7G memory limitation, it's a quite large value for UT.



-- 
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


[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867331912


##########
docs/deployment/engine_on_yarn.md:
##########
@@ -175,6 +175,24 @@ If the `TopSpeedWindowing` passes, configure it in `$KYUUBI_HOME/conf/kyuubi-env
 $ echo "export HADOOP_CONF_DIR=/path/to/hadoop/conf" >> $KYUUBI_HOME/conf/kyuubi-env.sh
 ```
 
+#### Required Environment Variable
+
+The `FLINK_HADOOP_CLASSPATH` is required, too. It should contain `commons-collections-*.jar`, 
+`hadoop-client-runtime-*.jar`, `hadoop-client-api-*.jar` and `htrace-core4-*.jar`.
+All four jars are in the `HADOOP_HOME`. 

Review Comment:
   And as a downstream project, Spark always use Hadoop 3 shaded client since Spark 3.2.0.



-- 
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


[GitHub] [incubator-kyuubi] jiaoqingbo commented on pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
jiaoqingbo commented on PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#issuecomment-1121029576

   cc @pan3793 


-- 
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


[GitHub] [incubator-kyuubi] jiaoqingbo commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
jiaoqingbo commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867330023


##########
kyuubi-server/src/test/scala/org/apache/kyuubi/engine/flink/FlinkProcessBuilderSuite.scala:
##########
@@ -22,26 +22,33 @@ import java.io.File
 import scala.collection.JavaConverters._
 import scala.collection.immutable.ListMap
 
-import org.apache.kyuubi.{FLINK_COMPILE_VERSION, KyuubiFunSuite, KyuubiSQLException, SCALA_COMPILE_VERSION}
+import org.apache.kyuubi.{FLINK_COMPILE_VERSION, KyuubiException, KyuubiFunSuite, SCALA_COMPILE_VERSION}
 import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf.{ENGINE_FLINK_EXTRA_CLASSPATH, ENGINE_FLINK_JAVA_OPTIONS, ENGINE_FLINK_MEMORY}
 
 class FlinkProcessBuilderSuite extends KyuubiFunSuite {
   private def conf = KyuubiConf().set("kyuubi.on", "off")
+    .set(ENGINE_FLINK_MEMORY, "1g")

Review Comment:
   OK I will chage it



-- 
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


[GitHub] [incubator-kyuubi] jiaoqingbo commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
jiaoqingbo commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867330557


##########
docs/deployment/engine_on_yarn.md:
##########
@@ -175,6 +175,24 @@ If the `TopSpeedWindowing` passes, configure it in `$KYUUBI_HOME/conf/kyuubi-env
 $ echo "export HADOOP_CONF_DIR=/path/to/hadoop/conf" >> $KYUUBI_HOME/conf/kyuubi-env.sh
 ```
 
+#### Required Environment Variable
+
+The `FLINK_HADOOP_CLASSPATH` is required, too. It should contain `commons-collections-*.jar`, 
+`hadoop-client-runtime-*.jar`, `hadoop-client-api-*.jar` and `htrace-core4-*.jar`.
+All four jars are in the `HADOOP_HOME`. 
+
+For example, in Hadoop 3.1.0 version, the following is their location. 

Review Comment:
   OK I will change it



-- 
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


[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867329540


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/flink/FlinkProcessBuilder.scala:
##########
@@ -26,6 +26,7 @@ import scala.collection.mutable.ArrayBuffer
 
 import org.apache.kyuubi._
 import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf.{ENGINE_FLINK_EXTRA_CLASSPATH, ENGINE_FLINK_JAVA_OPTIONS, ENGINE_FLINK_MEMORY}

Review Comment:
   nit: 
   ```suggestion
   import org.apache.kyuubi.config.KyuubiConf._
   ```



-- 
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


[GitHub] [incubator-kyuubi] jiaoqingbo commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
jiaoqingbo commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867286273


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/flink/FlinkProcessBuilder.scala:
##########
@@ -80,13 +86,17 @@ class FlinkProcessBuilder(
     env.get("HADOOP_CONF_DIR").foreach(classpathEntries.add)
     env.get("YARN_CONF_DIR").foreach(classpathEntries.add)
     env.get("HBASE_CONF_DIR").foreach(classpathEntries.add)
-    val hadoopClasspath = env.get("HADOOP_CLASSPATH")
-    if (hadoopClasspath.isEmpty) {
-      throw KyuubiSQLException("HADOOP_CLASSPATH is not set! " +
-        "For more detail information on configuring HADOOP_CLASSPATH" +
-        "https://kyuubi.apache.org/docs/latest/deployment/settings.html#environments")
+    val hadoopCp = env.get(FLINK_HADOOP_CLASSPATH)
+    hadoopCp.foreach(classpathEntries.add)
+    val extraCp = conf.get(ENGINE_FLINK_EXTRA_CLASSPATH)
+    extraCp.foreach(classpathEntries.add)
+    if (hadoopCp.isEmpty && extraCp.isEmpty) {
+      throw new KyuubiException(s"The conf of ${FLINK_HADOOP_CLASSPATH} or " +
+        s"${ENGINE_FLINK_EXTRA_CLASSPATH} is empty." +
+        s"Please set ${FLINK_HADOOP_CLASSPATH} or ${ENGINE_FLINK_EXTRA_CLASSPATH} for " +

Review Comment:
   I didn't understand what you meant, this FLINK_HADOOP_CLASSPATH is a String, refer to HIVE_HADOOP_CLASSPATH in HiveProcessBuilder



-- 
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


[GitHub] [incubator-kyuubi] jiaoqingbo commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
jiaoqingbo commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867285416


##########
kyuubi-server/src/test/scala/org/apache/kyuubi/engine/flink/FlinkProcessBuilderSuite.scala:
##########
@@ -22,26 +22,33 @@ import java.io.File
 import scala.collection.JavaConverters._
 import scala.collection.immutable.ListMap
 
-import org.apache.kyuubi.{FLINK_COMPILE_VERSION, KyuubiFunSuite, KyuubiSQLException, SCALA_COMPILE_VERSION}
+import org.apache.kyuubi.{FLINK_COMPILE_VERSION, KyuubiException, KyuubiFunSuite, SCALA_COMPILE_VERSION}
 import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf.{ENGINE_FLINK_EXTRA_CLASSPATH, ENGINE_FLINK_JAVA_OPTIONS, ENGINE_FLINK_MEMORY}
 
 class FlinkProcessBuilderSuite extends KyuubiFunSuite {
   private def conf = KyuubiConf().set("kyuubi.on", "off")
+    .set(ENGINE_FLINK_MEMORY, "5g")

Review Comment:
   > 
   
   OK I will change it



-- 
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


[GitHub] [incubator-kyuubi] zhaomin1423 commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
zhaomin1423 commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r866900699


##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -1437,6 +1437,28 @@ object KyuubiConf {
       .stringConf
       .createOptional
 
+  val ENGINE_FLINK_MEMORY: ConfigEntry[String] =
+    buildConf("kyuubi.engine.flink.memory")
+      .doc("The heap memory for the flink query engine")

Review Comment:
   The flink query engine should be 'flink sql engine', we should be keep consistent in the project.



-- 
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


[GitHub] [incubator-kyuubi] zhaomin1423 commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
zhaomin1423 commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r866900699


##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -1437,6 +1437,28 @@ object KyuubiConf {
       .stringConf
       .createOptional
 
+  val ENGINE_FLINK_MEMORY: ConfigEntry[String] =
+    buildConf("kyuubi.engine.flink.memory")
+      .doc("The heap memory for the flink query engine")

Review Comment:
   The flink query engine should be 'flink sql engine', we should be keep consistent in project.



##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -1437,6 +1437,28 @@ object KyuubiConf {
       .stringConf
       .createOptional
 
+  val ENGINE_FLINK_MEMORY: ConfigEntry[String] =
+    buildConf("kyuubi.engine.flink.memory")
+      .doc("The heap memory for the flink query engine")

Review Comment:
   The flink query engine should be 'flink sql engine', we should be keep consistent in project.



-- 
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


[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867330003


##########
docs/deployment/engine_on_yarn.md:
##########
@@ -175,6 +175,24 @@ If the `TopSpeedWindowing` passes, configure it in `$KYUUBI_HOME/conf/kyuubi-env
 $ echo "export HADOOP_CONF_DIR=/path/to/hadoop/conf" >> $KYUUBI_HOME/conf/kyuubi-env.sh
 ```
 
+#### Required Environment Variable
+
+The `FLINK_HADOOP_CLASSPATH` is required, too. It should contain `commons-collections-*.jar`, 
+`hadoop-client-runtime-*.jar`, `hadoop-client-api-*.jar` and `htrace-core4-*.jar`.
+All four jars are in the `HADOOP_HOME`. 

Review Comment:
   I would say this is not accurate. The `FLINK_HADOOP_CLASSPATH` should ponit to Hadoop client localtion, generally it compose of Hadoop client configuration files, e.g. `core-site.xml`, `yarn-site.xml`, and Hadoop client jars, for Hadoop 3, Hadoop shaded client is recommended instead of vanilla jars.



-- 
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


[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867330003


##########
docs/deployment/engine_on_yarn.md:
##########
@@ -175,6 +175,24 @@ If the `TopSpeedWindowing` passes, configure it in `$KYUUBI_HOME/conf/kyuubi-env
 $ echo "export HADOOP_CONF_DIR=/path/to/hadoop/conf" >> $KYUUBI_HOME/conf/kyuubi-env.sh
 ```
 
+#### Required Environment Variable
+
+The `FLINK_HADOOP_CLASSPATH` is required, too. It should contain `commons-collections-*.jar`, 
+`hadoop-client-runtime-*.jar`, `hadoop-client-api-*.jar` and `htrace-core4-*.jar`.
+All four jars are in the `HADOOP_HOME`. 

Review Comment:
   I would say this is not accurate. The `FLINK_HADOOP_CLASSPATH` should ponit to Hadoop client localtion, generally it compose of Hadoop client configuration files, e.g. `core-site.xml`, `yarn-site.xml`, and Hadoop client jars. For Hadoop 3, Hadoop shaded client is recommended instead of vanilla jars.



-- 
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


[GitHub] [incubator-kyuubi] jiaoqingbo commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
jiaoqingbo commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867330099


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/flink/FlinkProcessBuilder.scala:
##########
@@ -26,6 +26,7 @@ import scala.collection.mutable.ArrayBuffer
 
 import org.apache.kyuubi._
 import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf.{ENGINE_FLINK_EXTRA_CLASSPATH, ENGINE_FLINK_JAVA_OPTIONS, ENGINE_FLINK_MEMORY}

Review Comment:
   OK
   



-- 
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


[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867965140


##########
kyuubi-server/src/test/scala/org/apache/kyuubi/engine/flink/FlinkProcessBuilderSuite.scala:
##########
@@ -22,26 +22,33 @@ import java.io.File
 import scala.collection.JavaConverters._
 import scala.collection.immutable.ListMap
 
-import org.apache.kyuubi.{FLINK_COMPILE_VERSION, KyuubiFunSuite, KyuubiSQLException, SCALA_COMPILE_VERSION}
+import org.apache.kyuubi.{FLINK_COMPILE_VERSION, KyuubiException, KyuubiFunSuite, SCALA_COMPILE_VERSION}
 import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf.{ENGINE_FLINK_EXTRA_CLASSPATH, ENGINE_FLINK_JAVA_OPTIONS, ENGINE_FLINK_MEMORY}
 
 class FlinkProcessBuilderSuite extends KyuubiFunSuite {
   private def conf = KyuubiConf().set("kyuubi.on", "off")
+    .set(ENGINE_FLINK_MEMORY, "512m")
+    .set(
+      ENGINE_FLINK_JAVA_OPTIONS,
+      "-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=5005")

Review Comment:
   will it cause port conflict?



-- 
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


[GitHub] [incubator-kyuubi] pan3793 commented on pull request #2579: [KYUUBI #2333][KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
pan3793 commented on PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#issuecomment-1121908389

   Thanks, merging to master


-- 
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


[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867981889


##########
docs/deployment/engine_on_yarn.md:
##########
@@ -169,12 +169,33 @@ export HADOOP_CLASSPATH=`hadoop classpath`
 echo "stop" | ./bin/yarn-session.sh -id application_XXXXX_XXX
  ```
 
-If the `TopSpeedWindowing` passes, configure it in `$KYUUBI_HOME/conf/kyuubi-env.sh` or `$FLINK_HOME/bin/config.sh`, e.g.
+If the `TopSpeedWindowing` passes, configure it in `$KYUUBI_HOME/conf/kyuubi-env.sh`
 
 ```bash
 $ echo "export HADOOP_CONF_DIR=/path/to/hadoop/conf" >> $KYUUBI_HOME/conf/kyuubi-env.sh
 ```
 
+#### Required Environment Variable
+
+The `FLINK_HADOOP_CLASSPATH` is required, too.
+
+If using Hadoop 3.x,  configured it in `$KYUUBI_HOME/conf/kyuubi-env.sh` as follows:
+
+```bash
+$ echo "export FLINK_HADOOP_CLASSPATH=/path/to/hadoop-client-runtime-*.jar:/path/to/hadoop-client-api-*.jar" >> $KYUUBI_HOME/conf/kyuubi-env.sh
+```
+`hadoop-client-api-*.jar` and `hadoop-client-runtime-*.jar` are only applicable to users whose
+ 
+hadoop version is 3.x, if users use hadoop2.x, FLINK_HADOOP_CLASSPATH is recommended to
+ 
+be set to `hadoop classpath`
+
+If using Hadoop 2.x, configured it in `$KYUUBI_HOME/conf/kyuubi-env.sh` as follows:
+
+```bash
+$ echo "export FLINK_HADOOP_CLASSPATH=`hadoop classpath`" >> $KYUUBI_HOME/conf/kyuubi-env.sh
+```

Review Comment:
   
   
   For users who are using Hadoop 3.x, Hadoop shaded client is recommaned instead of Hadoop vanilla client jars. For users who are using Hadoop 2.x, `FLINK_HADOOP_CLASSPATH` should be set to `hadoop classpath` to use Hadoop vanilla client jars. For users which does not use Hadoop services, e.g. HDFS, YARN at all, Hadoop client jars is all required, and recommand to use Hadoop shaded client as Hadoop 3.x's users.
   
   See [HADOOP-11656](https://issues.apache.org/jira/browse/HADOOP-11656) for details of Hadoop shaded client.
   
   To use Hadoop shaded client, please configure `$KYUUBI_HOME/conf/kyuubi-env.sh` as follows:
   
   ```bash
   $ echo "export FLINK_HADOOP_CLASSPATH=/path/to/hadoop-client-runtime-3.3.2.jar:/path/to/hadoop-client-api-3.3.2.jar" >> $KYUUBI_HOME/conf/kyuubi-env.sh
   ```
   
   To use Hadoop vanilla jars, please configure `$KYUUBI_HOME/conf/kyuubi-env.sh` as follows:
   
   ```bash
   $ echo "export FLINK_HADOOP_CLASSPATH=`hadoop classpath`" >> $KYUUBI_HOME/conf/kyuubi-env.sh
   ```
   



-- 
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


[GitHub] [incubator-kyuubi] zhaomin1423 commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
zhaomin1423 commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r866895637


##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -1437,6 +1437,28 @@ object KyuubiConf {
       .stringConf
       .createOptional
 
+  val ENGINE_FLINK_MEMORY: ConfigEntry[String] =

Review Comment:
   The new configs should be added in the setting.md. you can refer docs/develop_tools/developer.md/Append descriptions of new configurations to settings.md.



-- 
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


[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867331795


##########
docs/deployment/engine_on_yarn.md:
##########
@@ -175,6 +175,24 @@ If the `TopSpeedWindowing` passes, configure it in `$KYUUBI_HOME/conf/kyuubi-env
 $ echo "export HADOOP_CONF_DIR=/path/to/hadoop/conf" >> $KYUUBI_HOME/conf/kyuubi-env.sh
 ```
 
+#### Required Environment Variable
+
+The `FLINK_HADOOP_CLASSPATH` is required, too. It should contain `commons-collections-*.jar`, 
+`hadoop-client-runtime-*.jar`, `hadoop-client-api-*.jar` and `htrace-core4-*.jar`.
+All four jars are in the `HADOOP_HOME`. 

Review Comment:
   Some background https://issues.apache.org/jira/browse/HADOOP-11656



-- 
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


[GitHub] [incubator-kyuubi] jiaoqingbo commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
jiaoqingbo commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867331379


##########
docs/deployment/engine_on_yarn.md:
##########
@@ -175,6 +175,24 @@ If the `TopSpeedWindowing` passes, configure it in `$KYUUBI_HOME/conf/kyuubi-env
 $ echo "export HADOOP_CONF_DIR=/path/to/hadoop/conf" >> $KYUUBI_HOME/conf/kyuubi-env.sh
 ```
 
+#### Required Environment Variable
+
+The `FLINK_HADOOP_CLASSPATH` is required, too. It should contain `commons-collections-*.jar`, 
+`hadoop-client-runtime-*.jar`, `hadoop-client-api-*.jar` and `htrace-core4-*.jar`.
+All four jars are in the `HADOOP_HOME`. 

Review Comment:
   Is it more reasonable to set to `hadoop classpath`?



-- 
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


[GitHub] [incubator-kyuubi] jiaoqingbo commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
jiaoqingbo commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867987114


##########
docs/deployment/engine_on_yarn.md:
##########
@@ -169,12 +169,33 @@ export HADOOP_CLASSPATH=`hadoop classpath`
 echo "stop" | ./bin/yarn-session.sh -id application_XXXXX_XXX
  ```
 
-If the `TopSpeedWindowing` passes, configure it in `$KYUUBI_HOME/conf/kyuubi-env.sh` or `$FLINK_HOME/bin/config.sh`, e.g.
+If the `TopSpeedWindowing` passes, configure it in `$KYUUBI_HOME/conf/kyuubi-env.sh`
 
 ```bash
 $ echo "export HADOOP_CONF_DIR=/path/to/hadoop/conf" >> $KYUUBI_HOME/conf/kyuubi-env.sh
 ```
 
+#### Required Environment Variable
+
+The `FLINK_HADOOP_CLASSPATH` is required, too.
+
+If using Hadoop 3.x,  configured it in `$KYUUBI_HOME/conf/kyuubi-env.sh` as follows:
+
+```bash
+$ echo "export FLINK_HADOOP_CLASSPATH=/path/to/hadoop-client-runtime-*.jar:/path/to/hadoop-client-api-*.jar" >> $KYUUBI_HOME/conf/kyuubi-env.sh
+```
+`hadoop-client-api-*.jar` and `hadoop-client-runtime-*.jar` are only applicable to users whose
+ 
+hadoop version is 3.x, if users use hadoop2.x, FLINK_HADOOP_CLASSPATH is recommended to
+ 
+be set to `hadoop classpath`
+
+If using Hadoop 2.x, configured it in `$KYUUBI_HOME/conf/kyuubi-env.sh` as follows:
+
+```bash
+$ echo "export FLINK_HADOOP_CLASSPATH=`hadoop classpath`" >> $KYUUBI_HOME/conf/kyuubi-env.sh
+```

Review Comment:
   What you described is more complete, I will change it according to what you said



-- 
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


[GitHub] [incubator-kyuubi] jiaoqingbo commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
jiaoqingbo commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867985898


##########
docs/deployment/settings.md:
##########
@@ -94,6 +95,7 @@ You can configure the environment variables in `$KYUUBI_HOME/conf/kyuubi-env.sh`
 # export SPARK_HOME=/opt/spark
 # export FLINK_HOME=/opt/flink
 # export HIVE_HOME=/opt/hive
+# export FLINK_HADOOP_CLASSPATH=/path/to/hadoop-client-runtime-*.jar:/path/to/hadoop-client-api-*.jar

Review Comment:
   WDYT?



-- 
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


[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867329286


##########
kyuubi-server/src/test/scala/org/apache/kyuubi/engine/flink/FlinkProcessBuilderSuite.scala:
##########
@@ -22,26 +22,33 @@ import java.io.File
 import scala.collection.JavaConverters._
 import scala.collection.immutable.ListMap
 
-import org.apache.kyuubi.{FLINK_COMPILE_VERSION, KyuubiFunSuite, KyuubiSQLException, SCALA_COMPILE_VERSION}
+import org.apache.kyuubi.{FLINK_COMPILE_VERSION, KyuubiException, KyuubiFunSuite, SCALA_COMPILE_VERSION}
 import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf.{ENGINE_FLINK_EXTRA_CLASSPATH, ENGINE_FLINK_JAVA_OPTIONS, ENGINE_FLINK_MEMORY}
 
 class FlinkProcessBuilderSuite extends KyuubiFunSuite {
   private def conf = KyuubiConf().set("kyuubi.on", "off")
+    .set(ENGINE_FLINK_MEMORY, "1g")
+    .set(
+      ENGINE_FLINK_JAVA_OPTIONS,
+      "-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=5005")
+
   private def envDefault: ListMap[String, String] = ListMap(
     "JAVA_HOME" -> s"${File.separator}jdk1.8.0_181")

Review Comment:
   Where this magic JAVA_HOME comes from?



-- 
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


[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867981889


##########
docs/deployment/engine_on_yarn.md:
##########
@@ -169,12 +169,33 @@ export HADOOP_CLASSPATH=`hadoop classpath`
 echo "stop" | ./bin/yarn-session.sh -id application_XXXXX_XXX
  ```
 
-If the `TopSpeedWindowing` passes, configure it in `$KYUUBI_HOME/conf/kyuubi-env.sh` or `$FLINK_HOME/bin/config.sh`, e.g.
+If the `TopSpeedWindowing` passes, configure it in `$KYUUBI_HOME/conf/kyuubi-env.sh`
 
 ```bash
 $ echo "export HADOOP_CONF_DIR=/path/to/hadoop/conf" >> $KYUUBI_HOME/conf/kyuubi-env.sh
 ```
 
+#### Required Environment Variable
+
+The `FLINK_HADOOP_CLASSPATH` is required, too.
+
+If using Hadoop 3.x,  configured it in `$KYUUBI_HOME/conf/kyuubi-env.sh` as follows:
+
+```bash
+$ echo "export FLINK_HADOOP_CLASSPATH=/path/to/hadoop-client-runtime-*.jar:/path/to/hadoop-client-api-*.jar" >> $KYUUBI_HOME/conf/kyuubi-env.sh
+```
+`hadoop-client-api-*.jar` and `hadoop-client-runtime-*.jar` are only applicable to users whose
+ 
+hadoop version is 3.x, if users use hadoop2.x, FLINK_HADOOP_CLASSPATH is recommended to
+ 
+be set to `hadoop classpath`
+
+If using Hadoop 2.x, configured it in `$KYUUBI_HOME/conf/kyuubi-env.sh` as follows:
+
+```bash
+$ echo "export FLINK_HADOOP_CLASSPATH=`hadoop classpath`" >> $KYUUBI_HOME/conf/kyuubi-env.sh
+```

Review Comment:
   
   For users who are using Hadoop 3.x, Hadoop shaded client is recommaned instead of Hadoop vanilla jars. For users who are using Hadoop 2.x, `FLINK_HADOOP_CLASSPATH` should be set to `hadoop classpath` to use Hadoop vanilla jars. For users which does not use Hadoop services, e.g. HDFS, YARN at all, Hadoop client jars is also required, and recommand to use Hadoop shaded client as Hadoop 3.x's users.
   
   See [HADOOP-11656](https://issues.apache.org/jira/browse/HADOOP-11656) for details of Hadoop shaded client.
   
   To use Hadoop shaded client, please configure `$KYUUBI_HOME/conf/kyuubi-env.sh` as follows:
   
   ```bash
   $ echo "export FLINK_HADOOP_CLASSPATH=/path/to/hadoop-client-runtime-3.3.2.jar:/path/to/hadoop-client-api-3.3.2.jar" >> $KYUUBI_HOME/conf/kyuubi-env.sh
   ```
   
   To use Hadoop vanilla jars, please configure `$KYUUBI_HOME/conf/kyuubi-env.sh` as follows:
   
   ```bash
   $ echo "export FLINK_HADOOP_CLASSPATH=`hadoop classpath`" >> $KYUUBI_HOME/conf/kyuubi-env.sh
   ```
   



-- 
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


[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867331912


##########
docs/deployment/engine_on_yarn.md:
##########
@@ -175,6 +175,24 @@ If the `TopSpeedWindowing` passes, configure it in `$KYUUBI_HOME/conf/kyuubi-env
 $ echo "export HADOOP_CONF_DIR=/path/to/hadoop/conf" >> $KYUUBI_HOME/conf/kyuubi-env.sh
 ```
 
+#### Required Environment Variable
+
+The `FLINK_HADOOP_CLASSPATH` is required, too. It should contain `commons-collections-*.jar`, 
+`hadoop-client-runtime-*.jar`, `hadoop-client-api-*.jar` and `htrace-core4-*.jar`.
+All four jars are in the `HADOOP_HOME`. 

Review Comment:
   And as a downstream project, Spark always use Hadoop shaded client since Spark 3.2.0.



-- 
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


[GitHub] [incubator-kyuubi] jiaoqingbo commented on pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
jiaoqingbo commented on PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#issuecomment-1121837434

   cc @pan3793 


-- 
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


[GitHub] [incubator-kyuubi] jiaoqingbo commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
jiaoqingbo commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867980204


##########
kyuubi-server/src/test/scala/org/apache/kyuubi/engine/flink/FlinkProcessBuilderSuite.scala:
##########
@@ -22,26 +22,33 @@ import java.io.File
 import scala.collection.JavaConverters._
 import scala.collection.immutable.ListMap
 
-import org.apache.kyuubi.{FLINK_COMPILE_VERSION, KyuubiFunSuite, KyuubiSQLException, SCALA_COMPILE_VERSION}
+import org.apache.kyuubi.{FLINK_COMPILE_VERSION, KyuubiException, KyuubiFunSuite, SCALA_COMPILE_VERSION}
 import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf.{ENGINE_FLINK_EXTRA_CLASSPATH, ENGINE_FLINK_JAVA_OPTIONS, ENGINE_FLINK_MEMORY}
 
 class FlinkProcessBuilderSuite extends KyuubiFunSuite {
   private def conf = KyuubiConf().set("kyuubi.on", "off")
+    .set(ENGINE_FLINK_MEMORY, "512m")
+    .set(
+      ENGINE_FLINK_JAVA_OPTIONS,
+      "-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=5005")

Review Comment:
   These unit tests only test the string of the builder command and do not actually start the FlinkSQLEngine process



-- 
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


[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867963253


##########
docs/deployment/settings.md:
##########
@@ -94,6 +95,7 @@ You can configure the environment variables in `$KYUUBI_HOME/conf/kyuubi-env.sh`
 # export SPARK_HOME=/opt/spark
 # export FLINK_HOME=/opt/flink
 # export HIVE_HOME=/opt/hive
+# export FLINK_HADOOP_CLASSPATH=/path/to/hadoop-client-runtime-*.jar:/path/to/hadoop-client-api-*.jar

Review Comment:
   ditto



-- 
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


[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867962422


##########
conf/kyuubi-env.sh.template:
##########
@@ -59,6 +60,7 @@
 # export SPARK_HOME=/opt/spark
 # export FLINK_HOME=/opt/flink
 # export HIVE_HOME=/opt/hive
+# export FLINK_HADOOP_CLASSPATH=/path/to/hadoop-client-runtime-*.jar:/path/to/hadoop-client-api-*.jar

Review Comment:
   specific to Kyuubi bundled hadoop version here



-- 
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


[GitHub] [incubator-kyuubi] zhaomin1423 commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
zhaomin1423 commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867286631


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/flink/FlinkProcessBuilder.scala:
##########
@@ -80,13 +86,17 @@ class FlinkProcessBuilder(
     env.get("HADOOP_CONF_DIR").foreach(classpathEntries.add)
     env.get("YARN_CONF_DIR").foreach(classpathEntries.add)
     env.get("HBASE_CONF_DIR").foreach(classpathEntries.add)
-    val hadoopClasspath = env.get("HADOOP_CLASSPATH")
-    if (hadoopClasspath.isEmpty) {
-      throw KyuubiSQLException("HADOOP_CLASSPATH is not set! " +
-        "For more detail information on configuring HADOOP_CLASSPATH" +
-        "https://kyuubi.apache.org/docs/latest/deployment/settings.html#environments")
+    val hadoopCp = env.get(FLINK_HADOOP_CLASSPATH)
+    hadoopCp.foreach(classpathEntries.add)
+    val extraCp = conf.get(ENGINE_FLINK_EXTRA_CLASSPATH)
+    extraCp.foreach(classpathEntries.add)
+    if (hadoopCp.isEmpty && extraCp.isEmpty) {
+      throw new KyuubiException(s"The conf of ${FLINK_HADOOP_CLASSPATH} or " +
+        s"${ENGINE_FLINK_EXTRA_CLASSPATH} is empty." +
+        s"Please set ${FLINK_HADOOP_CLASSPATH} or ${ENGINE_FLINK_EXTRA_CLASSPATH} for " +

Review Comment:
   sorry, should be the conf of ENGINE_FLINK_EXTRA_CLASSPATH.



-- 
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


[GitHub] [incubator-kyuubi] jiaoqingbo commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
jiaoqingbo commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867285549


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/flink/FlinkProcessBuilder.scala:
##########
@@ -80,13 +86,17 @@ class FlinkProcessBuilder(
     env.get("HADOOP_CONF_DIR").foreach(classpathEntries.add)
     env.get("YARN_CONF_DIR").foreach(classpathEntries.add)
     env.get("HBASE_CONF_DIR").foreach(classpathEntries.add)
-    val hadoopClasspath = env.get("HADOOP_CLASSPATH")
-    if (hadoopClasspath.isEmpty) {
-      throw KyuubiSQLException("HADOOP_CLASSPATH is not set! " +
-        "For more detail information on configuring HADOOP_CLASSPATH" +
-        "https://kyuubi.apache.org/docs/latest/deployment/settings.html#environments")
+    val hadoopCp = env.get(FLINK_HADOOP_CLASSPATH)
+    hadoopCp.foreach(classpathEntries.add)
+    val extraCp = conf.get(ENGINE_FLINK_EXTRA_CLASSPATH)
+    extraCp.foreach(classpathEntries.add)
+    if (hadoopCp.isEmpty && extraCp.isEmpty) {
+      throw new KyuubiException(s"The conf of ${FLINK_HADOOP_CLASSPATH} or " +

Review Comment:
   I will fix it



-- 
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


[GitHub] [incubator-kyuubi] codecov-commenter commented on pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#issuecomment-1120169414

   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2579?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#2579](https://codecov.io/gh/apache/incubator-kyuubi/pull/2579?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1ca10a6) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/6e17e794952c67050a2823ed25645e933835b995?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6e17e79) will **increase** coverage by `0.14%`.
   > The diff coverage is `96.00%`.
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2579      +/-   ##
   ============================================
   + Coverage     63.74%   63.89%   +0.14%     
     Complexity       73       73              
   ============================================
     Files           377      378       +1     
     Lines         18099    18166      +67     
     Branches       2444     2452       +8     
   ============================================
   + Hits          11538    11607      +69     
     Misses         5464     5464              
   + Partials       1097     1095       -2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2579?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ache/kyuubi/engine/flink/FlinkProcessBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2579/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvZmxpbmsvRmxpbmtQcm9jZXNzQnVpbGRlci5zY2FsYQ==) | `93.87% <91.66%> (-1.25%)` | :arrow_down: |
   | [...in/scala/org/apache/kyuubi/config/KyuubiConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2579/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jb25maWcvS3l1dWJpQ29uZi5zY2FsYQ==) | `96.68% <100.00%> (+0.05%)` | :arrow_up: |
   | [...rg/apache/kyuubi/engine/trino/TrinoStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2579/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vU3RhdGVtZW50LnNjYWxh) | `68.96% <0.00%> (-2.30%)` | :arrow_down: |
   | [...rg/apache/kyuubi/engine/flink/FlinkSQLEngine.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2579/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS1mbGluay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9mbGluay9GbGlua1NRTEVuZ2luZS5zY2FsYQ==) | `20.00% <0.00%> (-1.63%)` | :arrow_down: |
   | [.../kyuubi/operation/log/SeekableBufferedReader.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2579/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vbG9nL1NlZWthYmxlQnVmZmVyZWRSZWFkZXIuc2NhbGE=) | `92.59% <0.00%> (ø)` | |
   | [...in/scala/org/apache/kyuubi/server/api/v1/dto.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2579/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvYXBpL3YxL2R0by5zY2FsYQ==) | `73.17% <0.00%> (+0.09%)` | :arrow_up: |
   | [...g/apache/kyuubi/session/KyuubiSessionManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2579/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL0t5dXViaVNlc3Npb25NYW5hZ2VyLnNjYWxh) | `87.34% <0.00%> (+0.85%)` | :arrow_up: |
   | [.../apache/kyuubi/server/api/v1/BatchesResource.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2579/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvYXBpL3YxL0JhdGNoZXNSZXNvdXJjZS5zY2FsYQ==) | `91.07% <0.00%> (+0.87%)` | :arrow_up: |
   | [.../org/apache/kyuubi/operation/KyuubiOperation.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2579/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vS3l1dWJpT3BlcmF0aW9uLnNjYWxh) | `69.33% <0.00%> (+1.33%)` | :arrow_up: |
   | ... and [3 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/2579/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2579?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2579?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6e17e79...1ca10a6](https://codecov.io/gh/apache/incubator-kyuubi/pull/2579?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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


[GitHub] [incubator-kyuubi] jiaoqingbo commented on pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
jiaoqingbo commented on PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#issuecomment-1120172440

   cc @pan3793 @zhaomin1423 


-- 
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


[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867981889


##########
docs/deployment/engine_on_yarn.md:
##########
@@ -169,12 +169,33 @@ export HADOOP_CLASSPATH=`hadoop classpath`
 echo "stop" | ./bin/yarn-session.sh -id application_XXXXX_XXX
  ```
 
-If the `TopSpeedWindowing` passes, configure it in `$KYUUBI_HOME/conf/kyuubi-env.sh` or `$FLINK_HOME/bin/config.sh`, e.g.
+If the `TopSpeedWindowing` passes, configure it in `$KYUUBI_HOME/conf/kyuubi-env.sh`
 
 ```bash
 $ echo "export HADOOP_CONF_DIR=/path/to/hadoop/conf" >> $KYUUBI_HOME/conf/kyuubi-env.sh
 ```
 
+#### Required Environment Variable
+
+The `FLINK_HADOOP_CLASSPATH` is required, too.
+
+If using Hadoop 3.x,  configured it in `$KYUUBI_HOME/conf/kyuubi-env.sh` as follows:
+
+```bash
+$ echo "export FLINK_HADOOP_CLASSPATH=/path/to/hadoop-client-runtime-*.jar:/path/to/hadoop-client-api-*.jar" >> $KYUUBI_HOME/conf/kyuubi-env.sh
+```
+`hadoop-client-api-*.jar` and `hadoop-client-runtime-*.jar` are only applicable to users whose
+ 
+hadoop version is 3.x, if users use hadoop2.x, FLINK_HADOOP_CLASSPATH is recommended to
+ 
+be set to `hadoop classpath`
+
+If using Hadoop 2.x, configured it in `$KYUUBI_HOME/conf/kyuubi-env.sh` as follows:
+
+```bash
+$ echo "export FLINK_HADOOP_CLASSPATH=`hadoop classpath`" >> $KYUUBI_HOME/conf/kyuubi-env.sh
+```

Review Comment:
   
   For users who are using Hadoop 3.x, Hadoop shaded client is recommaned instead of Hadoop vanilla jars. For users who are using Hadoop 2.x, `FLINK_HADOOP_CLASSPATH` should be set to `hadoop classpath` to use Hadoop vanilla jars. For users which does not use Hadoop services, e.g. HDFS, YARN at all, Hadoop client jars is also required, and recommend to use Hadoop shaded client as Hadoop 3.x's users do.
   
   See [HADOOP-11656](https://issues.apache.org/jira/browse/HADOOP-11656) for details of Hadoop shaded client.
   
   To use Hadoop shaded client, please configure `$KYUUBI_HOME/conf/kyuubi-env.sh` as follows:
   
   ```bash
   $ echo "export FLINK_HADOOP_CLASSPATH=/path/to/hadoop-client-runtime-3.3.2.jar:/path/to/hadoop-client-api-3.3.2.jar" >> $KYUUBI_HOME/conf/kyuubi-env.sh
   ```
   
   To use Hadoop vanilla jars, please configure `$KYUUBI_HOME/conf/kyuubi-env.sh` as follows:
   
   ```bash
   $ echo "export FLINK_HADOOP_CLASSPATH=`hadoop classpath`" >> $KYUUBI_HOME/conf/kyuubi-env.sh
   ```
   



-- 
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


[GitHub] [incubator-kyuubi] jiaoqingbo commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
jiaoqingbo commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867286960


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/flink/FlinkProcessBuilder.scala:
##########
@@ -80,13 +86,17 @@ class FlinkProcessBuilder(
     env.get("HADOOP_CONF_DIR").foreach(classpathEntries.add)
     env.get("YARN_CONF_DIR").foreach(classpathEntries.add)
     env.get("HBASE_CONF_DIR").foreach(classpathEntries.add)
-    val hadoopClasspath = env.get("HADOOP_CLASSPATH")
-    if (hadoopClasspath.isEmpty) {
-      throw KyuubiSQLException("HADOOP_CLASSPATH is not set! " +
-        "For more detail information on configuring HADOOP_CLASSPATH" +
-        "https://kyuubi.apache.org/docs/latest/deployment/settings.html#environments")
+    val hadoopCp = env.get(FLINK_HADOOP_CLASSPATH)
+    hadoopCp.foreach(classpathEntries.add)
+    val extraCp = conf.get(ENGINE_FLINK_EXTRA_CLASSPATH)
+    extraCp.foreach(classpathEntries.add)
+    if (hadoopCp.isEmpty && extraCp.isEmpty) {
+      throw new KyuubiException(s"The conf of ${FLINK_HADOOP_CLASSPATH} or " +
+        s"${ENGINE_FLINK_EXTRA_CLASSPATH} is empty." +
+        s"Please set ${FLINK_HADOOP_CLASSPATH} or ${ENGINE_FLINK_EXTRA_CLASSPATH} for " +

Review Comment:
   ok I will change it  



-- 
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


[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r868234180


##########
conf/kyuubi-env.sh.template:
##########
@@ -59,6 +60,7 @@
 # export SPARK_HOME=/opt/spark
 # export FLINK_HOME=/opt/flink
 # export HIVE_HOME=/opt/hive
+# export FLINK_HADOOP_CLASSPATH=/path/to/hadoop-client-runtime-*.jar:/path/to/hadoop-client-api-*.jar

Review Comment:
   The example should be a specific version rather than a pattern, the bundled version is selected just for consistent purpose, and it's not a big deal if we forget to update the example once we upgrade the bundled hadoop version. 
   
   Take `# export JAVA_HOME=/usr/jdk64/jdk1.8.0_152` as an example, we never update it but it's clear enough for users how to set it.



-- 
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


[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867329439


##########
kyuubi-server/src/test/scala/org/apache/kyuubi/engine/flink/FlinkProcessBuilderSuite.scala:
##########
@@ -22,26 +22,33 @@ import java.io.File
 import scala.collection.JavaConverters._
 import scala.collection.immutable.ListMap
 
-import org.apache.kyuubi.{FLINK_COMPILE_VERSION, KyuubiFunSuite, KyuubiSQLException, SCALA_COMPILE_VERSION}
+import org.apache.kyuubi.{FLINK_COMPILE_VERSION, KyuubiException, KyuubiFunSuite, SCALA_COMPILE_VERSION}
 import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf.{ENGINE_FLINK_EXTRA_CLASSPATH, ENGINE_FLINK_JAVA_OPTIONS, ENGINE_FLINK_MEMORY}
 
 class FlinkProcessBuilderSuite extends KyuubiFunSuite {
   private def conf = KyuubiConf().set("kyuubi.on", "off")
+    .set(ENGINE_FLINK_MEMORY, "1g")

Review Comment:
   Does 512m work for it? There are Job Manager, Task Manager alongside Flink engine cosumes memory too.



-- 
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


[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867330142


##########
docs/deployment/engine_on_yarn.md:
##########
@@ -175,6 +175,24 @@ If the `TopSpeedWindowing` passes, configure it in `$KYUUBI_HOME/conf/kyuubi-env
 $ echo "export HADOOP_CONF_DIR=/path/to/hadoop/conf" >> $KYUUBI_HOME/conf/kyuubi-env.sh
 ```
 
+#### Required Environment Variable
+
+The `FLINK_HADOOP_CLASSPATH` is required, too. It should contain `commons-collections-*.jar`, 
+`hadoop-client-runtime-*.jar`, `hadoop-client-api-*.jar` and `htrace-core4-*.jar`.
+All four jars are in the `HADOOP_HOME`. 
+
+For example, in Hadoop 3.1.0 version, the following is their location. 

Review Comment:
   Can we demo with the version of Hadoop which Kyuudi bundled?



-- 
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


[GitHub] [incubator-kyuubi] jiaoqingbo commented on a diff in pull request #2579: [KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
jiaoqingbo commented on code in PR #2579:
URL: https://github.com/apache/incubator-kyuubi/pull/2579#discussion_r867332888


##########
docs/deployment/engine_on_yarn.md:
##########
@@ -175,6 +175,24 @@ If the `TopSpeedWindowing` passes, configure it in `$KYUUBI_HOME/conf/kyuubi-env
 $ echo "export HADOOP_CONF_DIR=/path/to/hadoop/conf" >> $KYUUBI_HOME/conf/kyuubi-env.sh
 ```
 
+#### Required Environment Variable
+
+The `FLINK_HADOOP_CLASSPATH` is required, too. It should contain `commons-collections-*.jar`, 
+`hadoop-client-runtime-*.jar`, `hadoop-client-api-*.jar` and `htrace-core4-*.jar`.
+All four jars are in the `HADOOP_HOME`. 

Review Comment:
   ok i'll take a look and fix it



-- 
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


[GitHub] [incubator-kyuubi] pan3793 closed pull request #2579: [KYUUBI #2333][KYUUBI #2554] Configuring Flink Engine heap memory and java opts

Posted by GitBox <gi...@apache.org>.
pan3793 closed pull request #2579: [KYUUBI #2333][KYUUBI #2554] Configuring Flink Engine heap memory and java opts
URL: https://github.com/apache/incubator-kyuubi/pull/2579


-- 
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