You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kyuubi.apache.org by fe...@apache.org on 2023/01/09 07:08:25 UTC

[kyuubi] branch master updated: [KYUUBI #4119][FOLLOWUP] Refactor appSubmissionTime to appStartTime

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

feiwang 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 cd3916dc7 [KYUUBI #4119][FOLLOWUP] Refactor appSubmissionTime to appStartTime
cd3916dc7 is described below

commit cd3916dc72d8e544f625b6867aef957014335db0
Author: fwang12 <fw...@ebay.com>
AuthorDate: Mon Jan 9 15:08:17 2023 +0800

    [KYUUBI #4119][FOLLOWUP] Refactor appSubmissionTime to appStartTime
    
    ### _Why are the changes needed?_
    
    To address comment https://github.com/apache/kyuubi/pull/4119#discussion_r1064237556
    
    ### _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
    
    - [x] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request
    
    Closes #4129 from turboFei/4119_followup.
    
    Closes #4119
    
    30626e5d6 [fwang12] appSubmissionTime to appStartTime
    
    Authored-by: fwang12 <fw...@ebay.com>
    Signed-off-by: fwang12 <fw...@ebay.com>
---
 .../src/main/scala/org/apache/kyuubi/ctl/util/Render.scala |  6 +++---
 .../java/org/apache/kyuubi/client/api/v1/dto/Batch.java    | 14 +++++++-------
 .../java/org/apache/kyuubi/client/BatchRestClientTest.java |  8 ++++----
 .../org/apache/kyuubi/operation/BatchJobSubmission.scala   | 10 +++++-----
 .../org/apache/kyuubi/server/api/v1/BatchesResource.scala  |  2 +-
 .../org/apache/kyuubi/session/KyuubiBatchSessionImpl.scala |  2 +-
 .../apache/kyuubi/server/api/v1/BatchesResourceSuite.scala |  2 +-
 7 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/util/Render.scala b/kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/util/Render.scala
index 9c84feb62..aba6df35a 100644
--- a/kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/util/Render.scala
+++ b/kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/util/Render.scala
@@ -111,9 +111,9 @@ private[ctl] object Render {
 
   private def buildBatchAppInfo(batch: Batch, showDiagnostic: Boolean = true): List[String] = {
     val batchAppInfo = ListBuffer[String]()
-    if (batch.getAppSubmissionTime > 0) {
-      batchAppInfo += s"App Submission Time:" +
-        s" ${millisToDateString(batch.getAppSubmissionTime, "yyyy-MM-dd HH:mm:ss")}"
+    if (batch.getAppStartTime > 0) {
+      batchAppInfo += s"App Start Time:" +
+        s" ${millisToDateString(batch.getAppStartTime, "yyyy-MM-dd HH:mm:ss")}"
     }
     Option(batch.getAppId).foreach { _ =>
       batchAppInfo += s"App Id: ${batch.getAppId}"
diff --git a/kyuubi-rest-client/src/main/java/org/apache/kyuubi/client/api/v1/dto/Batch.java b/kyuubi-rest-client/src/main/java/org/apache/kyuubi/client/api/v1/dto/Batch.java
index f17a83823..43fbf10af 100644
--- a/kyuubi-rest-client/src/main/java/org/apache/kyuubi/client/api/v1/dto/Batch.java
+++ b/kyuubi-rest-client/src/main/java/org/apache/kyuubi/client/api/v1/dto/Batch.java
@@ -26,7 +26,7 @@ public class Batch {
   private String user;
   private String batchType;
   private String name;
-  private long appSubmissionTime;
+  private long appStartTime;
   private String appId;
   private String appUrl;
   private String appState;
@@ -43,7 +43,7 @@ public class Batch {
       String user,
       String batchType,
       String name,
-      long appSubmissionTime,
+      long appStartTime,
       String appId,
       String appUrl,
       String appState,
@@ -56,7 +56,7 @@ public class Batch {
     this.user = user;
     this.batchType = batchType;
     this.name = name;
-    this.appSubmissionTime = appSubmissionTime;
+    this.appStartTime = appStartTime;
     this.appId = appId;
     this.appUrl = appUrl;
     this.appState = appState;
@@ -155,12 +155,12 @@ public class Batch {
     this.createTime = createTime;
   }
 
-  public long getAppSubmissionTime() {
-    return appSubmissionTime;
+  public long getAppStartTime() {
+    return appStartTime;
   }
 
-  public void setAppSubmissionTime(long appSubmissionTime) {
-    this.appSubmissionTime = appSubmissionTime;
+  public void setAppStartTime(long appStartTime) {
+    this.appStartTime = appStartTime;
   }
 
   public long getEndTime() {
diff --git a/kyuubi-rest-client/src/test/java/org/apache/kyuubi/client/BatchRestClientTest.java b/kyuubi-rest-client/src/test/java/org/apache/kyuubi/client/BatchRestClientTest.java
index 304caad90..80fb1c4b9 100644
--- a/kyuubi-rest-client/src/test/java/org/apache/kyuubi/client/BatchRestClientTest.java
+++ b/kyuubi-rest-client/src/test/java/org/apache/kyuubi/client/BatchRestClientTest.java
@@ -117,7 +117,7 @@ public class BatchRestClientTest {
     assertEquals(result.getUser(), expectedBatch.getUser());
     assertEquals(result.getBatchType(), expectedBatch.getBatchType());
     assertEquals(result.getName(), expectedBatch.getName());
-    assertEquals(result.getAppSubmissionTime(), expectedBatch.getAppSubmissionTime());
+    assertEquals(result.getAppStartTime(), expectedBatch.getAppStartTime());
     assertEquals(result.getAppId(), expectedBatch.getAppId());
     assertEquals(result.getAppUrl(), expectedBatch.getAppUrl());
     assertEquals(result.getAppState(), expectedBatch.getAppState());
@@ -146,7 +146,7 @@ public class BatchRestClientTest {
     assertEquals(result.getUser(), expectedBatch.getUser());
     assertEquals(result.getBatchType(), expectedBatch.getBatchType());
     assertEquals(result.getName(), expectedBatch.getName());
-    assertEquals(result.getAppSubmissionTime(), expectedBatch.getAppSubmissionTime());
+    assertEquals(result.getAppStartTime(), expectedBatch.getAppStartTime());
     assertEquals(result.getAppId(), expectedBatch.getAppId());
     assertEquals(result.getAppUrl(), expectedBatch.getAppUrl());
     assertEquals(result.getAppState(), expectedBatch.getAppState());
@@ -178,7 +178,7 @@ public class BatchRestClientTest {
     assertEquals(result.getUser(), expectedBatch.getUser());
     assertEquals(result.getBatchType(), expectedBatch.getBatchType());
     assertEquals(result.getName(), expectedBatch.getName());
-    assertEquals(result.getAppSubmissionTime(), expectedBatch.getAppSubmissionTime());
+    assertEquals(result.getAppStartTime(), expectedBatch.getAppStartTime());
     assertEquals(result.getAppId(), expectedBatch.getAppId());
     assertEquals(result.getAppUrl(), expectedBatch.getAppUrl());
     assertEquals(result.getAppState(), expectedBatch.getAppState());
@@ -210,7 +210,7 @@ public class BatchRestClientTest {
     assertEquals(result.getUser(), expectedBatch.getUser());
     assertEquals(result.getBatchType(), expectedBatch.getBatchType());
     assertEquals(result.getName(), expectedBatch.getName());
-    assertEquals(result.getAppSubmissionTime(), expectedBatch.getAppSubmissionTime());
+    assertEquals(result.getAppStartTime(), expectedBatch.getAppStartTime());
     assertEquals(result.getAppId(), expectedBatch.getAppId());
     assertEquals(result.getAppUrl(), expectedBatch.getAppUrl());
     assertEquals(result.getAppState(), expectedBatch.getAppState());
diff --git a/kyuubi-server/src/main/scala/org/apache/kyuubi/operation/BatchJobSubmission.scala b/kyuubi-server/src/main/scala/org/apache/kyuubi/operation/BatchJobSubmission.scala
index 1485f457f..e99b3292c 100644
--- a/kyuubi-server/src/main/scala/org/apache/kyuubi/operation/BatchJobSubmission.scala
+++ b/kyuubi-server/src/main/scala/org/apache/kyuubi/operation/BatchJobSubmission.scala
@@ -74,8 +74,8 @@ class BatchJobSubmission(
   private var killMessage: KillResponse = (false, "UNKNOWN")
   def getKillMessage: KillResponse = killMessage
 
-  @volatile private var _appSubmissionTime = recoveryMetadata.map(_.engineOpenTime).getOrElse(0L)
-  def appSubmissionTime: Long = _appSubmissionTime
+  @volatile private var _appStartTime = recoveryMetadata.map(_.engineOpenTime).getOrElse(0L)
+  def appStartTime: Long = _appStartTime
 
   @VisibleForTesting
   private[kyuubi] val builder: ProcBuilder = {
@@ -102,8 +102,8 @@ class BatchJobSubmission(
     val applicationInfo =
       applicationManager.getApplicationInfo(builder.clusterManager(), batchId).filter(_.id != null)
     applicationInfo.foreach { _ =>
-      if (_appSubmissionTime <= 0) {
-        _appSubmissionTime = System.currentTimeMillis()
+      if (_appStartTime <= 0) {
+        _appStartTime = System.currentTimeMillis()
       }
     }
     applicationInfo
@@ -137,7 +137,7 @@ class BatchJobSubmission(
       val metadataToUpdate = Metadata(
         identifier = batchId,
         state = state.toString,
-        engineOpenTime = appSubmissionTime,
+        engineOpenTime = appStartTime,
         engineId = status.id,
         engineName = status.name,
         engineUrl = status.url.orNull,
diff --git a/kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/BatchesResource.scala b/kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/BatchesResource.scala
index 5ada9faf5..487362d96 100644
--- a/kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/BatchesResource.scala
+++ b/kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/BatchesResource.scala
@@ -94,7 +94,7 @@ private[v1] class BatchesResource extends ApiRequestContext with Logging {
       session.user,
       batchOp.batchType,
       name,
-      batchOp.appSubmissionTime,
+      batchOp.appStartTime,
       appId,
       appUrl,
       appState,
diff --git a/kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiBatchSessionImpl.scala b/kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiBatchSessionImpl.scala
index dcf7fa116..967397c95 100644
--- a/kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiBatchSessionImpl.scala
+++ b/kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiBatchSessionImpl.scala
@@ -161,7 +161,7 @@ class KyuubiBatchSessionImpl(
 
   private[kyuubi] def onEngineOpened(): Unit = {
     if (sessionEvent.openedTime <= 0) {
-      sessionEvent.openedTime = batchJobSubmissionOp.appSubmissionTime
+      sessionEvent.openedTime = batchJobSubmissionOp.appStartTime
       EventBus.post(sessionEvent)
     }
   }
diff --git a/kyuubi-server/src/test/scala/org/apache/kyuubi/server/api/v1/BatchesResourceSuite.scala b/kyuubi-server/src/test/scala/org/apache/kyuubi/server/api/v1/BatchesResourceSuite.scala
index 31d77deaa..83c60878a 100644
--- a/kyuubi-server/src/test/scala/org/apache/kyuubi/server/api/v1/BatchesResourceSuite.scala
+++ b/kyuubi-server/src/test/scala/org/apache/kyuubi/server/api/v1/BatchesResourceSuite.scala
@@ -101,7 +101,7 @@ class BatchesResourceSuite extends KyuubiFunSuite with RestFrontendTestHelper wi
     assert(batch.getCreateTime > 0)
     assert(batch.getEndTime === 0)
     if (batch.getAppId != null) {
-      assert(batch.getAppSubmissionTime > 0)
+      assert(batch.getAppStartTime > 0)
     }
 
     // invalid batchId