This is an automated email from the ASF dual-hosted git repository.

chengpan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kyuubi.git


The following commit(s) were added to refs/heads/master by this push:
     new d1bcb4feb [KYUUBI #5036] Fix Operation.close not update complete 
timestamp
d1bcb4feb is described below

commit d1bcb4feb6b8617d772b1a333685655248b667c5
Author: zhangliang <[email protected]>
AuthorDate: Thu Jul 13 13:47:49 2023 +0800

    [KYUUBI #5036] Fix Operation.close not update complete timestamp
    
    ### _Why are the changes needed?_
    To fix #5036, Operation.close() (maybe caused by query timeout) does not 
update operation complete time.
    
    User may find it confusing and think the query is still running because 
each refresh on UI will show a different timestamp
    
    ### _How was this patch tested?_
    - [x] Add some test cases that check the changes thoroughly including 
negative and positive cases if possible
    
    - [x] Add screenshots for manual tests if appropriate
    
    - [x] [Run 
test](https://kyuubi.readthedocs.io/en/master/contributing/code/testing.html#running-tests)
 locally before make a pull request
    
    Closes #5042 from CavemanIV/kyuubi-5036.
    
    Closes #5036
    
    2e68c0b54 [zhangliang] [KYUUBI-5036] fix Operation.close not update 
complete timestamp
    
    Authored-by: zhangliang <[email protected]>
    Signed-off-by: Cheng Pan <[email protected]>
---
 .../kyuubi/operation/AbstractOperation.scala       |  2 +-
 .../kyuubi/operation/OperationStateSuite.scala     | 27 +++++++++++++++++++++-
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git 
a/kyuubi-common/src/main/scala/org/apache/kyuubi/operation/AbstractOperation.scala
 
b/kyuubi-common/src/main/scala/org/apache/kyuubi/operation/AbstractOperation.scala
index 5264ddebd..94f53111d 100644
--- 
a/kyuubi-common/src/main/scala/org/apache/kyuubi/operation/AbstractOperation.scala
+++ 
b/kyuubi-common/src/main/scala/org/apache/kyuubi/operation/AbstractOperation.scala
@@ -115,7 +115,7 @@ abstract class AbstractOperation(session: Session) extends 
Operation with Loggin
         info(s"Processing ${session.user}'s query[$statementId]: " +
           s"${state.name} -> ${newState.name}, statement:\n$redactedStatement")
         startTime = System.currentTimeMillis()
-      case ERROR | FINISHED | CANCELED | TIMEOUT =>
+      case ERROR | FINISHED | CANCELED | TIMEOUT | CLOSED =>
         completedTime = System.currentTimeMillis()
         val timeCost = s", time taken: ${(completedTime - startTime) / 1000.0} 
seconds"
         info(s"Processing ${session.user}'s query[$statementId]: " +
diff --git 
a/kyuubi-common/src/test/scala/org/apache/kyuubi/operation/OperationStateSuite.scala
 
b/kyuubi-common/src/test/scala/org/apache/kyuubi/operation/OperationStateSuite.scala
index d35ea246f..86c7e5e80 100644
--- 
a/kyuubi-common/src/test/scala/org/apache/kyuubi/operation/OperationStateSuite.scala
+++ 
b/kyuubi-common/src/test/scala/org/apache/kyuubi/operation/OperationStateSuite.scala
@@ -17,11 +17,13 @@
 
 package org.apache.kyuubi.operation
 
-import org.apache.hive.service.rpc.thrift.TOperationState
+import org.apache.hive.service.rpc.thrift.{TOperationState, TProtocolVersion}
 import org.apache.hive.service.rpc.thrift.TOperationState._
 
 import org.apache.kyuubi.{KyuubiFunSuite, KyuubiSQLException}
+import org.apache.kyuubi.config.KyuubiConf
 import org.apache.kyuubi.operation.OperationState._
+import org.apache.kyuubi.session.NoopSessionManager
 
 class OperationStateSuite extends KyuubiFunSuite {
   test("toTOperationState") {
@@ -79,4 +81,27 @@ class OperationStateSuite extends KyuubiFunSuite {
       assert(!OperationState.isTerminal(state))
     }
   }
+
+  test("kyuubi-5036 operation close should set completeTime") {
+    val sessionManager = new NoopSessionManager
+    sessionManager.initialize(KyuubiConf())
+    val sHandle = sessionManager.openSession(
+      TProtocolVersion.HIVE_CLI_SERVICE_PROTOCOL_V11,
+      "kyuubi",
+      "passwd",
+      "localhost",
+      Map.empty)
+    val session = sessionManager.getSession(sHandle)
+
+    val operation = new NoopOperation(session)
+    assert(operation.getStatus.completed == 0)
+
+    operation.close()
+    val afterClose1 = operation.getStatus
+    assert(afterClose1.state == OperationState.CLOSED)
+    assert(afterClose1.completed != 0)
+    Thread.sleep(1000)
+    val afterClose2 = operation.getStatus
+    assert(afterClose1.completed == afterClose2.completed)
+  }
 }

Reply via email to