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/03/23 10:55:02 UTC

[GitHub] [incubator-kyuubi] cxzl25 opened a new pull request #2202: [KYUUBI #2201] Show ExecutionId when running status on query engine page

cxzl25 opened a new pull request #2202:
URL: https://github.com/apache/incubator-kyuubi/pull/2202


   ### _Why are the changes needed?_
   
   Now we can display the ExecutionId on the query engine page, but generally the sql has been finished.
   We can update it when SQL is running and there is an ExecutionId.
   
   close https://github.com/apache/incubator-kyuubi/issues/2201
   
   ### _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] ulysses-you commented on a change in pull request #2202: [KYUUBI #2201] Show ExecutionId when running status on query engine page

Posted by GitBox <gi...@apache.org>.
ulysses-you commented on a change in pull request #2202:
URL: https://github.com/apache/incubator-kyuubi/pull/2202#discussion_r838116589



##########
File path: kyuubi-server/src/test/scala/org/apache/kyuubi/events/KyuubiEventLoggingServiceSuite.scala
##########
@@ -82,7 +82,7 @@ class KyuubiEventLoggingServiceSuite extends WithKyuubiServer with HiveJDBCTestH
       val engineTable = engineStatementEventPath.getParent
       val resultSet2 = statement.executeQuery(s"SELECT * FROM `json`.`${engineTable}`" +
         "where statement = \"" + sql + "\"")
-      val engineStates = Array(INITIALIZED, PENDING, RUNNING, COMPILED, FINISHED)
+      val engineStates = Array(INITIALIZED, PENDING, RUNNING, COMPILED, COMPILED, FINISHED)

Review comment:
       after some thought, I think we can remove the existed `COMPILE`:
   ```SCALA
   // TODO #921: COMPILED need consider eagerly executed commands
   setState(OperationState.COMPILED)
   ```
   
   then catch the event `SparkListenerSQLExecutionStart` in `SQLOperationListener`. At this time, we can post the `COMPILED` state event which contains the execution id.
   
   This approach also fix the `TODO` about the eagerly executed commands.
   




-- 
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] cxzl25 commented on a change in pull request #2202: [KYUUBI #2201] Show ExecutionId when running status on query engine page

Posted by GitBox <gi...@apache.org>.
cxzl25 commented on a change in pull request #2202:
URL: https://github.com/apache/incubator-kyuubi/pull/2202#discussion_r835416937



##########
File path: kyuubi-server/src/test/scala/org/apache/kyuubi/events/KyuubiEventLoggingServiceSuite.scala
##########
@@ -82,7 +82,7 @@ class KyuubiEventLoggingServiceSuite extends WithKyuubiServer with HiveJDBCTestH
       val engineTable = engineStatementEventPath.getParent
       val resultSet2 = statement.executeQuery(s"SELECT * FROM `json`.`${engineTable}`" +
         "where statement = \"" + sql + "\"")
-      val engineStates = Array(INITIALIZED, PENDING, RUNNING, COMPILED, FINISHED)
+      val engineStates = Array(INITIALIZED, PENDING, RUNNING, COMPILED, COMPILED, FINISHED)

Review comment:
       There are indeed a lot of event logs. I read the spark engine. There are two events for starting the engine, two events for session opening and closing, and a new event for each statement execution state change, which is about 5 indivual.
   
   My changes here will add an event to each statement.
   
   Because the user's sql may not be executed quickly sometimes, the session page can better track the execution of a specific user's entire session.
   
   I also planned to do one thing, like the spark thrift server, to list a list of jobids that are continuously generated by the statement, so that there are more events.
   
   ![image](https://user-images.githubusercontent.com/3898450/160158887-0c351ffe-b992-437a-ae6c-2ee2848d9992.png)
   
   
   




-- 
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 #2202: [KYUUBI #2201] Show ExecutionId when running status on query engine page

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


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2202?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 [#2202](https://codecov.io/gh/apache/incubator-kyuubi/pull/2202?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bb35823) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/6fe697536f41198f6b9117bdfa5d72156750d925?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6fe6975) will **increase** coverage by `0.32%`.
   > The diff coverage is `78.57%`.
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2202      +/-   ##
   ============================================
   + Coverage     61.94%   62.27%   +0.32%     
     Complexity       69       69              
   ============================================
     Files           333      349      +16     
     Lines         16107    16735     +628     
     Branches       2052     2264     +212     
   ============================================
   + Hits           9977    10421     +444     
   - Misses         5288     5355      +67     
   - Partials        842      959     +117     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...org/apache/spark/kyuubi/SQLOperationListener.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2202/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-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvc3Bhcmsva3l1dWJpL1NRTE9wZXJhdGlvbkxpc3RlbmVyLnNjYWxh) | `84.61% <50.00%> (-1.88%)` | :arrow_down: |
   | [...uubi/engine/spark/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2202/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-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9vcGVyYXRpb24vRXhlY3V0ZVN0YXRlbWVudC5zY2FsYQ==) | `82.89% <90.00%> (+0.54%)` | :arrow_up: |
   | [.../org/apache/kyuubi/engine/hive/HiveSQLEngine.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2202/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-ZXh0ZXJuYWxzL2t5dXViaS1oaXZlLXNxbC1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL2hpdmUvSGl2ZVNRTEVuZ2luZS5zY2FsYQ==) | `71.66% <0.00%> (-3.34%)` | :arrow_down: |
   | [...ache/kyuubi/operation/KyuubiOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2202/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vS3l1dWJpT3BlcmF0aW9uTWFuYWdlci5zY2FsYQ==) | `94.11% <0.00%> (-1.97%)` | :arrow_down: |
   | [...e/kyuubi/engine/hive/session/HiveSessionImpl.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2202/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-ZXh0ZXJuYWxzL2t5dXViaS1oaXZlLXNxbC1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL2hpdmUvc2Vzc2lvbi9IaXZlU2Vzc2lvbkltcGwuc2NhbGE=) | `100.00% <0.00%> (ø)` | |
   | [...yuubi/engine/hive/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2202/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-ZXh0ZXJuYWxzL2t5dXViaS1oaXZlLXNxbC1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL2hpdmUvb3BlcmF0aW9uL0V4ZWN1dGVTdGF0ZW1lbnQuc2NhbGE=) | `100.00% <0.00%> (ø)` | |
   | [.../kyuubi/plugin/spark/authz/PrivilegesBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2202/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-ZXh0ZW5zaW9ucy9zcGFyay9reXV1Ymktc3BhcmstYXV0aHovc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvcGx1Z2luL3NwYXJrL2F1dGh6L1ByaXZpbGVnZXNCdWlsZGVyLnNjYWxh) | `69.48% <0.00%> (ø)` | |
   | [...yuubi/plugin/spark/authz/PrivilegeObjectType.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2202/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-ZXh0ZW5zaW9ucy9zcGFyay9reXV1Ymktc3BhcmstYXV0aHovc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvcGx1Z2luL3NwYXJrL2F1dGh6L1ByaXZpbGVnZU9iamVjdFR5cGUuc2NhbGE=) | `100.00% <0.00%> (ø)` | |
   | [...uubi/plugin/spark/authz/ranger/AccessRequest.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2202/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-ZXh0ZW5zaW9ucy9zcGFyay9reXV1Ymktc3BhcmstYXV0aHovc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvcGx1Z2luL3NwYXJrL2F1dGh6L3Jhbmdlci9BY2Nlc3NSZXF1ZXN0LnNjYWxh) | `93.54% <0.00%> (ø)` | |
   | [...he/kyuubi/plugin/spark/authz/PrivilegeObject.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2202/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-ZXh0ZW5zaW9ucy9zcGFyay9reXV1Ymktc3BhcmstYXV0aHovc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvcGx1Z2luL3NwYXJrL2F1dGh6L1ByaXZpbGVnZU9iamVjdC5zY2FsYQ==) | `83.33% <0.00%> (ø)` | |
   | ... and [13 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/2202/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/2202?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/2202?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 [6fe6975...bb35823](https://codecov.io/gh/apache/incubator-kyuubi/pull/2202?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] cxzl25 commented on a change in pull request #2202: [KYUUBI #2201] Show ExecutionId when running status on query engine page

Posted by GitBox <gi...@apache.org>.
cxzl25 commented on a change in pull request #2202:
URL: https://github.com/apache/incubator-kyuubi/pull/2202#discussion_r839155091



##########
File path: kyuubi-server/src/test/scala/org/apache/kyuubi/events/KyuubiEventLoggingServiceSuite.scala
##########
@@ -82,7 +82,7 @@ class KyuubiEventLoggingServiceSuite extends WithKyuubiServer with HiveJDBCTestH
       val engineTable = engineStatementEventPath.getParent
       val resultSet2 = statement.executeQuery(s"SELECT * FROM `json`.`${engineTable}`" +
         "where statement = \"" + sql + "\"")
-      val engineStates = Array(INITIALIZED, PENDING, RUNNING, COMPILED, FINISHED)
+      val engineStates = Array(INITIALIZED, PENDING, RUNNING, COMPILED, COMPILED, FINISHED)

Review comment:
       Sorry, after reading the Spark code, each SQL does have an executionId.
   The DDL I watched directly from the kyuubi session page yesterday did not show the executionId.
   That would also fix two issues by the way.
   1. ddl shows executionId.
   2. We can also remove listener when no job is generated to prevent memory leaks.




-- 
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] ulysses-you commented on a change in pull request #2202: [KYUUBI #2201] Show ExecutionId when running status on query engine page

Posted by GitBox <gi...@apache.org>.
ulysses-you commented on a change in pull request #2202:
URL: https://github.com/apache/incubator-kyuubi/pull/2202#discussion_r838116589



##########
File path: kyuubi-server/src/test/scala/org/apache/kyuubi/events/KyuubiEventLoggingServiceSuite.scala
##########
@@ -82,7 +82,7 @@ class KyuubiEventLoggingServiceSuite extends WithKyuubiServer with HiveJDBCTestH
       val engineTable = engineStatementEventPath.getParent
       val resultSet2 = statement.executeQuery(s"SELECT * FROM `json`.`${engineTable}`" +
         "where statement = \"" + sql + "\"")
-      val engineStates = Array(INITIALIZED, PENDING, RUNNING, COMPILED, FINISHED)
+      val engineStates = Array(INITIALIZED, PENDING, RUNNING, COMPILED, COMPILED, FINISHED)

Review comment:
       after some thought, I think we can remove the existed `COMPILED` (just remove the event code):
   ```SCALA
   // TODO #921: COMPILED need consider eagerly executed commands
   setState(OperationState.COMPILED)
   ```
   
   then catch the event `SparkListenerSQLExecutionStart` in `SQLOperationListener`. At this time, we can post the `COMPILED` state event which contains the execution id.
   
   This approach also fix the `TODO` about the eagerly executed commands.
   




-- 
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] ulysses-you commented on a change in pull request #2202: [KYUUBI #2201] Show ExecutionId when running status on query engine page

Posted by GitBox <gi...@apache.org>.
ulysses-you commented on a change in pull request #2202:
URL: https://github.com/apache/incubator-kyuubi/pull/2202#discussion_r833857770



##########
File path: kyuubi-server/src/test/scala/org/apache/kyuubi/events/KyuubiEventLoggingServiceSuite.scala
##########
@@ -82,7 +82,7 @@ class KyuubiEventLoggingServiceSuite extends WithKyuubiServer with HiveJDBCTestH
       val engineTable = engineStatementEventPath.getParent
       val resultSet2 = statement.executeQuery(s"SELECT * FROM `json`.`${engineTable}`" +
         "where statement = \"" + sql + "\"")
-      val engineStates = Array(INITIALIZED, PENDING, RUNNING, COMPILED, FINISHED)
+      val engineStates = Array(INITIALIZED, PENDING, RUNNING, COMPILED, COMPILED, FINISHED)

Review comment:
       I'm afraid the extra event. There is many redundant event info now ..




-- 
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] cxzl25 commented on a change in pull request #2202: [KYUUBI #2201] Show ExecutionId when running status on query engine page

Posted by GitBox <gi...@apache.org>.
cxzl25 commented on a change in pull request #2202:
URL: https://github.com/apache/incubator-kyuubi/pull/2202#discussion_r839587230



##########
File path: kyuubi-server/src/test/scala/org/apache/kyuubi/events/KyuubiEventLoggingServiceSuite.scala
##########
@@ -82,7 +82,7 @@ class KyuubiEventLoggingServiceSuite extends WithKyuubiServer with HiveJDBCTestH
       val engineTable = engineStatementEventPath.getParent
       val resultSet2 = statement.executeQuery(s"SELECT * FROM `json`.`${engineTable}`" +
         "where statement = \"" + sql + "\"")
-      val engineStates = Array(INITIALIZED, PENDING, RUNNING, COMPILED, FINISHED)
+      val engineStates = Array(INITIALIZED, PENDING, RUNNING, COMPILED, COMPILED, FINISHED)

Review comment:
       After offline discussion, using `SparkListenerSQLExecutionStart` to get the `executionId`, it is difficult to map the current statement.
   So ddl still can't display the `executionId`.
   If there is no `executionId`, `SparkOperation#close` will remove the `SQLOperationListener` at the end, so there is no memory leak.




-- 
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] cxzl25 commented on a change in pull request #2202: [KYUUBI #2201] Show ExecutionId when running status on query engine page

Posted by GitBox <gi...@apache.org>.
cxzl25 commented on a change in pull request #2202:
URL: https://github.com/apache/incubator-kyuubi/pull/2202#discussion_r838651032



##########
File path: kyuubi-server/src/test/scala/org/apache/kyuubi/events/KyuubiEventLoggingServiceSuite.scala
##########
@@ -82,7 +82,7 @@ class KyuubiEventLoggingServiceSuite extends WithKyuubiServer with HiveJDBCTestH
       val engineTable = engineStatementEventPath.getParent
       val resultSet2 = statement.executeQuery(s"SELECT * FROM `json`.`${engineTable}`" +
         "where statement = \"" + sql + "\"")
-      val engineStates = Array(INITIALIZED, PENDING, RUNNING, COMPILED, FINISHED)
+      val engineStates = Array(INITIALIZED, PENDING, RUNNING, COMPILED, COMPILED, FINISHED)

Review comment:
       Now I think of a way, if there is no `COMPILED` state before setting the `FINISHED` state, then set the `COMPILED` state first.




-- 
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] cxzl25 commented on pull request #2202: [KYUUBI #2201] Show ExecutionId when running status on query engine page

Posted by GitBox <gi...@apache.org>.
cxzl25 commented on pull request #2202:
URL: https://github.com/apache/incubator-kyuubi/pull/2202#issuecomment-1076236000


   # Current
   ![image](https://user-images.githubusercontent.com/3898450/159685099-87bd7fa9-635d-4365-9a4e-ba6230dab059.png)
   # Fix
   ![image](https://user-images.githubusercontent.com/3898450/159684172-0144c123-7707-485c-88c9-b1ca31f8e29c.png)
   


-- 
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] ulysses-you commented on a change in pull request #2202: [KYUUBI #2201] Show ExecutionId when running status on query engine page

Posted by GitBox <gi...@apache.org>.
ulysses-you commented on a change in pull request #2202:
URL: https://github.com/apache/incubator-kyuubi/pull/2202#discussion_r839127902



##########
File path: kyuubi-server/src/test/scala/org/apache/kyuubi/events/KyuubiEventLoggingServiceSuite.scala
##########
@@ -82,7 +82,7 @@ class KyuubiEventLoggingServiceSuite extends WithKyuubiServer with HiveJDBCTestH
       val engineTable = engineStatementEventPath.getParent
       val resultSet2 = statement.executeQuery(s"SELECT * FROM `json`.`${engineTable}`" +
         "where statement = \"" + sql + "\"")
-      val engineStates = Array(INITIALIZED, PENDING, RUNNING, COMPILED, FINISHED)
+      val engineStates = Array(INITIALIZED, PENDING, RUNNING, COMPILED, COMPILED, FINISHED)

Review comment:
       > However, some DDLs do not generate jobs, so there is no executionId
   
   It's not right, query must have the executionId whether it generates jobs or not. So I said, we can catch  `SparkListenerSQLExecutionStart` rather than `SparkListenerJobStart`




-- 
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 edited a comment on pull request #2202: [KYUUBI #2201] Show ExecutionId when running status on query engine page

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #2202:
URL: https://github.com/apache/incubator-kyuubi/pull/2202#issuecomment-1084572760


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2202?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 [#2202](https://codecov.io/gh/apache/incubator-kyuubi/pull/2202?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e1273e5) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/0fd47381468c387d7febeccc817ee1bfad35b16a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0fd4738) will **increase** coverage by `0.01%`.
   > The diff coverage is `78.57%`.
   
   > :exclamation: Current head e1273e5 differs from pull request most recent head abb0cc0. Consider uploading reports for the commit abb0cc0 to get more accurate results
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2202      +/-   ##
   ============================================
   + Coverage     62.26%   62.27%   +0.01%     
     Complexity       69       69              
   ============================================
     Files           349      349              
     Lines         16723    16735      +12     
     Branches       2261     2264       +3     
   ============================================
   + Hits          10412    10422      +10     
   - Misses         5354     5355       +1     
   - Partials        957      958       +1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...org/apache/spark/kyuubi/SQLOperationListener.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2202/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-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvc3Bhcmsva3l1dWJpL1NRTE9wZXJhdGlvbkxpc3RlbmVyLnNjYWxh) | `84.61% <50.00%> (-1.88%)` | :arrow_down: |
   | [...uubi/engine/spark/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2202/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-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9vcGVyYXRpb24vRXhlY3V0ZVN0YXRlbWVudC5zY2FsYQ==) | `82.89% <90.00%> (+0.54%)` | :arrow_up: |
   | [...ache/kyuubi/operation/KyuubiOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2202/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vS3l1dWJpT3BlcmF0aW9uTWFuYWdlci5zY2FsYQ==) | `96.07% <0.00%> (+1.96%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2202?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/2202?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 [0fd4738...abb0cc0](https://codecov.io/gh/apache/incubator-kyuubi/pull/2202?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] cxzl25 commented on a change in pull request #2202: [KYUUBI #2201] Show ExecutionId when running status on query engine page

Posted by GitBox <gi...@apache.org>.
cxzl25 commented on a change in pull request #2202:
URL: https://github.com/apache/incubator-kyuubi/pull/2202#discussion_r838612608



##########
File path: kyuubi-server/src/test/scala/org/apache/kyuubi/events/KyuubiEventLoggingServiceSuite.scala
##########
@@ -82,7 +82,7 @@ class KyuubiEventLoggingServiceSuite extends WithKyuubiServer with HiveJDBCTestH
       val engineTable = engineStatementEventPath.getParent
       val resultSet2 = statement.executeQuery(s"SELECT * FROM `json`.`${engineTable}`" +
         "where statement = \"" + sql + "\"")
-      val engineStates = Array(INITIALIZED, PENDING, RUNNING, COMPILED, FINISHED)
+      val engineStates = Array(INITIALIZED, PENDING, RUNNING, COMPILED, COMPILED, FINISHED)

Review comment:
       However, some DDLs do not generate jobs, so there is no executionId
   e.g. use db / drop table.
   
   But this TODO, I guess it is because the command such as insert select or ctas will be executed directly, so TODO wants to set this state in advance.
   
   We can do this if we allow some DDL to have no `COMPILED` status.
   




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