You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kyuubi.apache.org by ch...@apache.org on 2023/05/30 07:43:48 UTC

[kyuubi] branch branch-1.7 updated: [KYUUBI #4898] fix logOperation multiple read with missing line

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

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


The following commit(s) were added to refs/heads/branch-1.7 by this push:
     new 9d2a9cde3 [KYUUBI #4898] fix  logOperation multiple read with missing line
9d2a9cde3 is described below

commit 9d2a9cde370a6b1a3e67387cde04f23395726b67
Author: huangzhir <30...@qq.com>
AuthorDate: Tue May 30 15:43:08 2023 +0800

    [KYUUBI #4898] fix  logOperation multiple read with missing line
    
    ### _Why are the changes needed?_
    
    to close https://github.com/apache/kyuubi/issues/4897
    
    ### _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.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request
    
    Closes #4898 from huangzhir/operation_log_bug.
    
    Closes #4898
    
    5d630dd31 [huangzhir] use do while style
    87fc2d23d [huangzhir] fix
    6cb0e5561 [huangzhir] fix  logOperation multiple read with missing line
    
    Authored-by: huangzhir <30...@qq.com>
    Signed-off-by: Cheng Pan <ch...@apache.org>
    (cherry picked from commit 4c3c36e77f3b5cd0bb75339b7743fb1f2a7aa788)
    Signed-off-by: Cheng Pan <ch...@apache.org>
---
 .../apache/kyuubi/operation/log/OperationLog.scala | 12 ++++++----
 .../kyuubi/operation/log/OperationLogSuite.scala   | 28 ++++++++++++++++++++++
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/operation/log/OperationLog.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/operation/log/OperationLog.scala
index 3791c08d2..c25874b20 100644
--- a/kyuubi-common/src/main/scala/org/apache/kyuubi/operation/log/OperationLog.scala
+++ b/kyuubi-common/src/main/scala/org/apache/kyuubi/operation/log/OperationLog.scala
@@ -130,12 +130,14 @@ class OperationLog(path: Path) {
     val logs = new JArrayList[String]
     var i = 0
     try {
-      var line: String = reader.readLine()
-      while ((i < lastRows || maxRows <= 0) && line != null) {
-        logs.add(line)
+      var line: String = null
+      do {
         line = reader.readLine()
-        i += 1
-      }
+        if (line != null) {
+          logs.add(line)
+          i += 1
+        }
+      } while ((i < lastRows || maxRows <= 0) && line != null)
       (logs, i)
     } catch {
       case e: IOException =>
diff --git a/kyuubi-common/src/test/scala/org/apache/kyuubi/operation/log/OperationLogSuite.scala b/kyuubi-common/src/test/scala/org/apache/kyuubi/operation/log/OperationLogSuite.scala
index b333b59fd..edc6b3375 100644
--- a/kyuubi-common/src/test/scala/org/apache/kyuubi/operation/log/OperationLogSuite.scala
+++ b/kyuubi-common/src/test/scala/org/apache/kyuubi/operation/log/OperationLogSuite.scala
@@ -318,4 +318,32 @@ class OperationLogSuite extends KyuubiFunSuite {
     log.close()
     session.close()
   }
+
+  test("test operationLog multiple read with missing line ") {
+    val file = Utils.createTempDir().resolve("f")
+    val writer = Files.newBufferedWriter(file, StandardCharsets.UTF_8)
+    try {
+      0.until(10).foreach(x => writer.write(s"$x\n"))
+      writer.flush()
+      writer.close()
+
+      val log = new OperationLog(file)
+      // The operation log file is created externally and should be initialized actively.
+      log.initOperationLogIfNecessary()
+
+      def compareResult(rows: TRowSet, expected: Seq[String]): Unit = {
+        val res = rows.getColumns.get(0).getStringVal.getValues.asScala
+        assert(res.size == expected.size)
+        res.zip(expected).foreach { case (l, r) =>
+          assert(l == r)
+        }
+      }
+      compareResult(log.read(2), Seq("0", "1"))
+      compareResult(log.read(3), Seq("2", "3", "4"))
+      compareResult(log.read(10), Seq("5", "6", "7", "8", "9"))
+    } finally {
+      Utils.deleteDirectoryRecursively(file.toFile)
+    }
+  }
+
 }