From 5e551bdd7118357b9dc42597d9b0bfdd104fb87e Mon Sep 17 00:00:00 2001 From: ulysses-you Date: Tue, 9 Mar 2021 16:29:35 +0800 Subject: [PATCH] [KYUUBI #408] Fix engine log does not be overwrite ![ulysses-you](https://badgen.net/badge/Hello/ulysses-you/green) [![Closes #408](https://badgen.net/badge/Preview/Closes%20%23408/blue)](https://github.com/yaooqinn/kyuubi/pull/408) ![40](https://badgen.net/badge/%2B/40/red) ![7](https://badgen.net/badge/-/7/green) ![3](https://badgen.net/badge/commits/3/yellow) ![Target Issue](https://badgen.net/badge/Missing/Target%20Issue/ff0000) ![Test Plan](https://badgen.net/badge/Missing/Test%20Plan/ff0000) ![Bug](https://badgen.net/badge/Label/Bug/) [❨?❩](https://pullrequestbadge.com/?utm_medium=github&utm_source=yaooqinn&utm_campaign=badge_info) ### _Why are the changes needed?_ Bug fix, otherwise the engine log will always increase size with append mode. ### _How was this patch tested?_ Add new test. Closes #408 from ulysses-you/fix-engine-log-overwrite. 7e4fd1d [ulysses-you] simply code b8d92ee [ulysses-you] config 821891c [ulysses-you] init Authored-by: ulysses-you Signed-off-by: ulysses-you (cherry picked from commit 57ed76f48d7f30fe7df317920fde57d95e92326e) Signed-off-by: ulysses-you --- .../apache/kyuubi/engine/ProcBuilder.scala | 20 +++++++++----- .../spark/SparkProcessBuilderSuite.scala | 27 ++++++++++++++++++- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/kyuubi-main/src/main/scala/org/apache/kyuubi/engine/ProcBuilder.scala b/kyuubi-main/src/main/scala/org/apache/kyuubi/engine/ProcBuilder.scala index 0b1f7bedd..c186633f6 100644 --- a/kyuubi-main/src/main/scala/org/apache/kyuubi/engine/ProcBuilder.scala +++ b/kyuubi-main/src/main/scala/org/apache/kyuubi/engine/ProcBuilder.scala @@ -65,15 +65,11 @@ trait ProcBuilder { // Visible for test private[kyuubi] var logCaptureThread: Thread = _ - private lazy val engineLog: File = ProcBuilder.synchronized { + private[kyuubi] lazy val engineLog: File = ProcBuilder.synchronized { val engineLogTimeout = conf.get(KyuubiConf.ENGINE_LOG_TIMEOUT) val currentTime = System.currentTimeMillis() val processLogPath = workingDir - val totalExistsFile = processLogPath.toFile.listFiles(new FilenameFilter() { - override def accept(dir: File, name: String): Boolean = { - name.startsWith(module) - } - }) + val totalExistsFile = processLogPath.toFile.listFiles { (_, name) => name.startsWith(module) } val sorted = totalExistsFile.sortBy(_.getName.split("\\.").last.toInt) val nextIndex = if (sorted.isEmpty) { 0 @@ -81,6 +77,18 @@ trait ProcBuilder { sorted.last.getName.split("\\.").last.toInt + 1 } val file = sorted.find(_.lastModified() < currentTime - engineLogTimeout) + .map { existsFile => + try { + // Here we want to overwrite the exists log file + existsFile.delete() + existsFile.createNewFile() + existsFile + } catch { + case e: Exception => + warn(s"failed to delete engine log file: ${existsFile.getAbsolutePath}", e) + null + } + } .getOrElse { Files.createDirectories(processLogPath) val newLogFile = new File(processLogPath.toFile, s"$module.log.$nextIndex") diff --git a/kyuubi-main/src/test/scala/org/apache/kyuubi/engine/spark/SparkProcessBuilderSuite.scala b/kyuubi-main/src/test/scala/org/apache/kyuubi/engine/spark/SparkProcessBuilderSuite.scala index 2c842b1f3..5f2424e74 100644 --- a/kyuubi-main/src/test/scala/org/apache/kyuubi/engine/spark/SparkProcessBuilderSuite.scala +++ b/kyuubi-main/src/test/scala/org/apache/kyuubi/engine/spark/SparkProcessBuilderSuite.scala @@ -18,13 +18,15 @@ package org.apache.kyuubi.engine.spark import java.io.File -import java.nio.file.{Files, Path, Paths} +import java.nio.file.{Files, Path, Paths, StandardOpenOption} +import java.time.Duration import java.util.concurrent.{Executors, TimeUnit} import org.scalatest.time.SpanSugar._ import org.apache.kyuubi.{KerberizedTestHelper, KyuubiSQLException, Utils} import org.apache.kyuubi.config.KyuubiConf +import org.apache.kyuubi.config.KyuubiConf.ENGINE_LOG_TIMEOUT import org.apache.kyuubi.service.ServiceUtils class SparkProcessBuilderSuite extends KerberizedTestHelper { @@ -179,6 +181,29 @@ class SparkProcessBuilderSuite extends KerberizedTestHelper { atomicTest() } } + + test("overwrite log file should cleanup before write") { + val fakeWorkDir = Files.createTempDirectory("fake") + val conf = KyuubiConf() + conf.set(ENGINE_LOG_TIMEOUT, Duration.ofDays(1).toMillis) + val builder1 = new FakeSparkProcessBuilder(conf) { + override val workingDir: Path = fakeWorkDir + } + val file1 = builder1.engineLog + Files.write(file1.toPath, "a".getBytes(), StandardOpenOption.APPEND) + assert(file1.length() == 1) + Files.write(file1.toPath, "a".getBytes(), StandardOpenOption.APPEND) + assert(file1.length() == 2) + file1.setLastModified(System.currentTimeMillis() - Duration.ofDays(1).toMillis - 1000) + + val builder2 = new FakeSparkProcessBuilder(conf) { + override val workingDir: Path = fakeWorkDir + } + val file2 = builder2.engineLog + assert(file1.getAbsolutePath == file2.getAbsolutePath) + Files.write(file2.toPath, "a".getBytes(), StandardOpenOption.APPEND) + assert(file2.length() == 1) + } } class FakeSparkProcessBuilder(config: KyuubiConf)