pan3793 commented on a change in pull request #1400:
URL: https://github.com/apache/incubator-kyuubi/pull/1400#discussion_r750375455



##########
File path: 
kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/hive/KyuubiConnection.java
##########
@@ -220,32 +229,71 @@ public KyuubiConnection(String uri, Properties info) 
throws SQLException {
     client = newSynchronizedClient(client);
   }
 
-  private void getLaunchEngineLog() {
+  /**
+   * Check whether launch engine operation might be producing more logs to be 
fetched.
+   * This method is a public API for usage outside of Kyuubi, although it is 
not part of the
+   * interface java.sql.Connection.
+   * @return true if launch engine operation might be producing more logs. It 
does not indicate
+   *         if last log lines have been fetched by getEngineLog.
+   */
+  public boolean hasMoreEngineLogs() {
+    return launchEngineOpHandle != null && !(launchEngineOpCompleted && 
!isEngineLogBeingGenerated);
+  }
+
+  /**
+   * Get the launch engine operation logs of current connection.
+   * This method is a public API for usage outside of Kyuubi, although it is 
not part of the
+   * interface java.sql.Connection.
+   * This method gets the incremental logs during launching engine, and uses 
fetchSize holden by
+   * KyuubiStatement object.
+   * @return a list of logs. It can be empty if there are no new logs to be 
retrieved at that time.
+   * @throws SQLException
+   * @throws ClosedConnectionException if connection has been closed
+   */
+  public List<String> getEngineLog() throws SQLException, 
ClosedConnectionException {
+    if (isClosed()) {
+      throw new ClosedConnectionException("Method getEngineLog() failed. The " 
+
+        "connection has been closed.");
+    }
+    TFetchResultsReq fetchResultsReq = new 
TFetchResultsReq(launchEngineOpHandle,
+      TFetchOrientation.FETCH_NEXT, fetchSize);
+    fetchResultsReq.setFetchType((short) 1);
+
+    List<String> logs = new ArrayList<>();
+    try {
+      TFetchResultsResp tFetchResultsResp = 
client.FetchResults(fetchResultsReq);
+      RowSet rowSet = RowSetFactory.create(tFetchResultsResp.getResults(), 
this.getProtocol());
+      for (Object[] row: rowSet) {
+        logs.add(String.valueOf(row[0]));
+      }
+    } catch (TException e) {
+      throw new SQLException("Error building result set for query log", e);
+    }
+    isEngineLogBeingGenerated = !logs.isEmpty();
+
+    TGetOperationStatusReq opStatusReq = new 
TGetOperationStatusReq(launchEngineOpHandle);
+    try {
+      launchEngineOpCompleted = 
client.GetOperationStatus(opStatusReq).getOperationCompleted() != 0;
+    } catch (TException e) {
+      launchEngineOpCompleted = true;
+    }
+
+    return Collections.unmodifiableList(logs);
+  }
+
+  private void showLaunchEngineLog() {
     if (launchEngineOpHandle != null) {
       LOG.info("Starting to get launch engine log.");
       Thread logThread = new Thread("engine-launch-log") {
-        boolean launchEngineCompleted = false;
-        long timeToEnd = Long.MAX_VALUE;
-        boolean continueToFetch = true;
 
         @Override
         public void run() {
           try {
-            while (continueToFetch && System.currentTimeMillis() < timeToEnd) {
-              List<String> logs = fetchEngineLogs();
-              if (launchEngineCompleted && logs.isEmpty()) {
-                continueToFetch = false;
-              }
-
+            while (hasMoreEngineLogs()) {

Review comment:
       the logic is much clear than before.




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to