-
Notifications
You must be signed in to change notification settings - Fork 896
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix engine log does not be overwrite #408
Conversation
Codecov Report
@@ Coverage Diff @@
## master #408 +/- ##
==========================================
+ Coverage 80.57% 80.59% +0.02%
==========================================
Files 101 101
Lines 3701 3706 +5
Branches 453 452 -1
==========================================
+ Hits 2982 2987 +5
- Misses 484 485 +1
+ Partials 235 234 -1
Continue to review full report at Codecov.
|
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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we explicitly set ENGINE_LOG_TIMEOUT rather than depends on default value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, use the config exsplicitly.
@@ -65,7 +65,7 @@ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scala support SAM since 2.12, we can simplify below code as
val totalExistsFile = processLogPath.toFile.listFiles { (_, name) => name.startsWith(module) }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
magic !
![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)<!-- PR-BADGE: PLEASE DO NOT REMOVE THIS COMMENT --> <!-- Thanks for sending a pull request! Here are some tips for you: 1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/contributions.html 2. If the PR is related to an issue in https://github.com/yaooqinn/kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'. 3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'. --> ### _Why are the changes needed?_ <!-- Please clarify why the changes are needed. For instance, 1. If you add a feature, you can talk about the use case of it. 2. If you fix a bug, you can clarify why it is a bug. --> 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 <ulyssesyou18@gmail.com> Signed-off-by: ulysses-you <ulyssesyou18@gmail.com> (cherry picked from commit 57ed76f) Signed-off-by: ulysses-you <ulyssesyou18@gmail.com>
thanks merging to branch-1.1 and master. |
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.