You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by "turboFei (via GitHub)" <gi...@apache.org> on 2024/11/01 00:26:24 UTC

[PR] do not trigger job [kyuubi]

turboFei opened a new pull request, #6789:
URL: https://github.com/apache/kyuubi/pull/6789

   # :mag: Description
   ## Issue References ๐Ÿ”—
   <!-- Append the issue number after #. If there is no issue for you to link create one or -->
   <!-- If there are no issues to link, please provide details here. -->
   
   This pull request fixes #
   
   ## Describe Your Solution ๐Ÿ”ง
   
   Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
   
   
   ## Types of changes :bookmark:
   <!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
   - [ ] Bugfix (non-breaking change which fixes an issue)
   - [ ] New feature (non-breaking change which adds functionality)
   - [ ] Breaking change (fix or feature that would cause existing functionality to change)
   
   ## Test Plan ๐Ÿงช
   
   #### Behavior Without This Pull Request :coffin:
   
   
   #### Behavior With This Pull Request :tada:
   
   
   #### Related Unit Tests
   
   
   ---
   
   # Checklist ๐Ÿ“
   <!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
   <!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
   
   - [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)
   
   **Be nice. Be informative.**
   


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

Re: [PR] Prevent triggering spark job when executing the initialization sql [kyuubi]

Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6789:
URL: https://github.com/apache/kyuubi/pull/6789#discussion_r1825325721


##########
externals/kyuubi-spark-sql-engine/src/test/scala/org/apache/kyuubi/engine/spark/WithSparkSQLEngine.scala:
##########
@@ -35,7 +35,9 @@ trait WithSparkSQLEngine extends KyuubiFunSuite {
   // Behavior is affected by the initialization SQL: 'SHOW DATABASES'
   // SPARK-35378 (3.2.0) makes it triggers job
   // SPARK-43124 (4.0.0) makes it avoid triggering job
-  protected val initJobId: Int = if (SPARK_ENGINE_RUNTIME_VERSION >= "4.0") 0 else 1
+  // KYUUBI #6789 makes it avoid triggering job
+  // protected val initJobId: Int = if (SPARK_ENGINE_RUNTIME_VERSION >= "4.0") 0 else 1
+  protected val initJobId: Int = 0

Review Comment:
   In our use case, I set initial/mini executors to 0 for notebook connections.
   
   If job triggered by initialization SQL, it will need at least 1 executor before the connection ready.
   
   



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

Re: [PR] Prevent triggering spark job when executing the initialization sql [kyuubi]

Posted by "ulysses-you (via GitHub)" <gi...@apache.org>.
ulysses-you commented on code in PR #6789:
URL: https://github.com/apache/kyuubi/pull/6789#discussion_r1825328760


##########
externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/KyuubiSparkUtil.scala:
##########
@@ -48,7 +48,7 @@ object KyuubiSparkUtil extends Logging {
         interruptOnCancel = true)
       debug(s"Execute initialization sql: $sql")
       try {
-        spark.sql(sql).isEmpty
+        spark.sql(sql).take(1).isEmpty

Review Comment:
   `spark.sql(sql).take(1)` has two issues:
   - it will do an row conversion from `InternalRow` to `Row`
   - it does not do column pruning
   
   how about using the same way of the pr https://github.com/apache/spark/pull/45373 to improve 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

Re: [PR] Prevent triggering spark job when executing the initialization sql [kyuubi]

Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei closed pull request #6789: Prevent triggering spark job when executing the initialization sql
URL: https://github.com/apache/kyuubi/pull/6789


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

Re: [PR] Prevent triggering spark job when executing the initialization sql [kyuubi]

Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6789:
URL: https://github.com/apache/kyuubi/pull/6789#discussion_r1825353254


##########
externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/spark/sql/kyuubi/SparkDatasetHelper.scala:
##########
@@ -290,4 +290,13 @@ object SparkDatasetHelper extends Logging {
     nodeName == "org.apache.spark.sql.execution.command.ExecutedCommandExec" ||
     nodeName == "org.apache.spark.sql.execution.CommandResultExec"
   }
+
+  /** SPARK-47270: Returns a optimized plan for CommandResult, convert to `LocalRelation`. */
+  def commandResultOptimized[T](dataset: Dataset[T]): Dataset[T] = {

Review Comment:
   Ok. I will cherry-pick the pr into our managed spark 3.5.
   



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

Re: [PR] do not trigger job [kyuubi]

Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei closed pull request #6789: do not trigger job
URL: https://github.com/apache/kyuubi/pull/6789


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

Re: [PR] Prevent triggering spark job when executing the initialization sql [kyuubi]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #6789:
URL: https://github.com/apache/kyuubi/pull/6789#discussion_r1825349910


##########
externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/spark/sql/kyuubi/SparkDatasetHelper.scala:
##########
@@ -290,4 +290,13 @@ object SparkDatasetHelper extends Logging {
     nodeName == "org.apache.spark.sql.execution.command.ExecutedCommandExec" ||
     nodeName == "org.apache.spark.sql.execution.CommandResultExec"
   }
+
+  /** SPARK-47270: Returns a optimized plan for CommandResult, convert to `LocalRelation`. */
+  def commandResultOptimized[T](dataset: Dataset[T]): Dataset[T] = {

Review Comment:
   I'm -0 on this change, I would consider it a Spark side issue. Sparkโ€™s master branch is undergoing a major refactoring, I'm worried about accessing Spark's non-public API in the engine's "core code path".
   
   For users who want to avoid triggering executor launch, they can either patch their spark or set init SQL as something like: `SET spark.app.id`



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

Re: [PR] Prevent triggering spark job when executing the initialization sql [kyuubi]

Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6789:
URL: https://github.com/apache/kyuubi/pull/6789#discussion_r1825345628


##########
externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/KyuubiSparkUtil.scala:
##########
@@ -48,7 +48,7 @@ object KyuubiSparkUtil extends Logging {
         interruptOnCancel = true)
       debug(s"Execute initialization sql: $sql")
       try {
-        spark.sql(sql).isEmpty
+        spark.sql(sql).take(1).isEmpty

Review Comment:
   Not sure whether it is easy to maintain this workaround in kyuubi end.



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

Re: [PR] Prevent triggering spark job when executing the initialization sql [kyuubi]

Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #6789:
URL: https://github.com/apache/kyuubi/pull/6789#issuecomment-2456129394

   Prefer to backport the spark PR to managed spark


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

Re: [PR] Prevent triggering spark job when executing the initialization sql [kyuubi]

Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6789:
URL: https://github.com/apache/kyuubi/pull/6789#discussion_r1825325225


##########
externals/kyuubi-spark-sql-engine/src/test/scala/org/apache/kyuubi/engine/spark/WithSparkSQLEngine.scala:
##########
@@ -35,7 +35,9 @@ trait WithSparkSQLEngine extends KyuubiFunSuite {
   // Behavior is affected by the initialization SQL: 'SHOW DATABASES'
   // SPARK-35378 (3.2.0) makes it triggers job
   // SPARK-43124 (4.0.0) makes it avoid triggering job
-  protected val initJobId: Int = if (SPARK_ENGINE_RUNTIME_VERSION >= "4.0") 0 else 1
+  // KYUUBI #6789 makes it avoid triggering job
+  // protected val initJobId: Int = if (SPARK_ENGINE_RUNTIME_VERSION >= "4.0") 0 else 1
+  protected val initJobId: Int = 0

Review Comment:
   Fixed by @wForget  https://github.com/apache/spark/pull/45397



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

Re: [PR] Prevent triggering spark job when executing the initialization sql [kyuubi]

Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6789:
URL: https://github.com/apache/kyuubi/pull/6789#discussion_r1825326380


##########
externals/kyuubi-spark-sql-engine/src/test/scala/org/apache/kyuubi/engine/spark/WithSparkSQLEngine.scala:
##########
@@ -35,7 +35,9 @@ trait WithSparkSQLEngine extends KyuubiFunSuite {
   // Behavior is affected by the initialization SQL: 'SHOW DATABASES'
   // SPARK-35378 (3.2.0) makes it triggers job
   // SPARK-43124 (4.0.0) makes it avoid triggering job
-  protected val initJobId: Int = if (SPARK_ENGINE_RUNTIME_VERSION >= "4.0") 0 else 1
+  // KYUUBI #6789 makes it avoid triggering job
+  // protected val initJobId: Int = if (SPARK_ENGINE_RUNTIME_VERSION >= "4.0") 0 else 1
+  protected val initJobId: Int = 0

Review Comment:
   And for notebook connections, we will init the spark driver in a temporary queue, and then move it to the user queue eventually, so it should be fast to make the connection ready even the user queue has no available resources.
   
   But if job triggered during initialization, it might make it slow if the  user queue has no available resources.



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

Re: [PR] Prevent triggering spark job when executing the initialization sql [kyuubi]

Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6789:
URL: https://github.com/apache/kyuubi/pull/6789#discussion_r1825326380


##########
externals/kyuubi-spark-sql-engine/src/test/scala/org/apache/kyuubi/engine/spark/WithSparkSQLEngine.scala:
##########
@@ -35,7 +35,9 @@ trait WithSparkSQLEngine extends KyuubiFunSuite {
   // Behavior is affected by the initialization SQL: 'SHOW DATABASES'
   // SPARK-35378 (3.2.0) makes it triggers job
   // SPARK-43124 (4.0.0) makes it avoid triggering job
-  protected val initJobId: Int = if (SPARK_ENGINE_RUNTIME_VERSION >= "4.0") 0 else 1
+  // KYUUBI #6789 makes it avoid triggering job
+  // protected val initJobId: Int = if (SPARK_ENGINE_RUNTIME_VERSION >= "4.0") 0 else 1
+  protected val initJobId: Int = 0

Review Comment:
   And for notebook connections, we will init the spark driver in a temporary queue, and then move it to the final user queue, so it should be fast to make the connection ready even the user queue has no available resources.
   
   But if job triggered during initialization, it will make it slow if the  user queue has no available resources.



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

Re: [PR] Prevent triggering spark job when executing the initialization sql [kyuubi]

Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6789:
URL: https://github.com/apache/kyuubi/pull/6789#discussion_r1825326380


##########
externals/kyuubi-spark-sql-engine/src/test/scala/org/apache/kyuubi/engine/spark/WithSparkSQLEngine.scala:
##########
@@ -35,7 +35,9 @@ trait WithSparkSQLEngine extends KyuubiFunSuite {
   // Behavior is affected by the initialization SQL: 'SHOW DATABASES'
   // SPARK-35378 (3.2.0) makes it triggers job
   // SPARK-43124 (4.0.0) makes it avoid triggering job
-  protected val initJobId: Int = if (SPARK_ENGINE_RUNTIME_VERSION >= "4.0") 0 else 1
+  // KYUUBI #6789 makes it avoid triggering job
+  // protected val initJobId: Int = if (SPARK_ENGINE_RUNTIME_VERSION >= "4.0") 0 else 1
+  protected val initJobId: Int = 0

Review Comment:
   And for notebook connections, we will init the spark driver in a temporary queue, and then move it to the final user queues, so it should be fast to make the connection ready even the user queue has no available resources.
   
   But if job triggered during initialization, it will make it slow if the  user queue has no available resources.



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

Re: [PR] Prevent triggering spark job when executing the initialization sql [kyuubi]

Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6789:
URL: https://github.com/apache/kyuubi/pull/6789#discussion_r1825326380


##########
externals/kyuubi-spark-sql-engine/src/test/scala/org/apache/kyuubi/engine/spark/WithSparkSQLEngine.scala:
##########
@@ -35,7 +35,9 @@ trait WithSparkSQLEngine extends KyuubiFunSuite {
   // Behavior is affected by the initialization SQL: 'SHOW DATABASES'
   // SPARK-35378 (3.2.0) makes it triggers job
   // SPARK-43124 (4.0.0) makes it avoid triggering job
-  protected val initJobId: Int = if (SPARK_ENGINE_RUNTIME_VERSION >= "4.0") 0 else 1
+  // KYUUBI #6789 makes it avoid triggering job
+  // protected val initJobId: Int = if (SPARK_ENGINE_RUNTIME_VERSION >= "4.0") 0 else 1
+  protected val initJobId: Int = 0

Review Comment:
   And for notebook connections, we will init the spark driver in a temporary queue, and then move it to the final user queue, so it should be fast to make the connection ready even the user queue has no available resources.
   
   But if job triggered during initialization, it might make it slow if the  user queue has no available resources.



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

Re: [PR] Prevent triggering spark job when executing the initialization sql [kyuubi]

Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6789:
URL: https://github.com/apache/kyuubi/pull/6789#discussion_r1825324130


##########
externals/kyuubi-spark-sql-engine/src/test/scala/org/apache/kyuubi/engine/spark/WithSparkSQLEngine.scala:
##########
@@ -35,7 +35,9 @@ trait WithSparkSQLEngine extends KyuubiFunSuite {
   // Behavior is affected by the initialization SQL: 'SHOW DATABASES'
   // SPARK-35378 (3.2.0) makes it triggers job
   // SPARK-43124 (4.0.0) makes it avoid triggering job
-  protected val initJobId: Int = if (SPARK_ENGINE_RUNTIME_VERSION >= "4.0") 0 else 1
+  // KYUUBI #6789 makes it avoid triggering job
+  // protected val initJobId: Int = if (SPARK_ENGINE_RUNTIME_VERSION >= "4.0") 0 else 1
+  protected val initJobId: Int = 0

Review Comment:
   how do you think about? @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