Skip to content

Commit

Permalink
[KYUUBI #985] Add logCaptureThreadReleased flag
Browse files Browse the repository at this point in the history
<!--
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/apache/incubator-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.
-->
`isInterrupted` is always false if some `InterruptedException` are throwed. So we should use a new flag to check if log capture thread is released.

### _How was this patch tested?_
- [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible

- [ ] Add screenshots for manual tests if appropriate

- [ ] [Run test](https://kyuubi.readthedocs.io/en/latest/develop_tools/testing.html#running-tests) locally before make a pull request

Closes #985 from ulysses-you/flaky-test.

Closes #985

1fcc0a9 [ulysses-you] empty
463a996 [ulysses-you] empty
d1f2ea5 [ulysses-you] eventually
f9f3734 [ulysses-you] fix

Authored-by: ulysses-you <ulyssesyou18@gmail.com>
Signed-off-by: Cheng Pan <chengpan@apache.org>
(cherry picked from commit 3afd20e)
Signed-off-by: Cheng Pan <chengpan@apache.org>
  • Loading branch information
ulysses-you authored and pan3793 committed Aug 25, 2021
1 parent e624e7b commit 3c8cce4
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ trait ProcBuilder {
@volatile private var error: Throwable = UNCAUGHT_ERROR
@volatile private var lastRowOfLog: String = "unknown"
// Visible for test
private[kyuubi] var logCaptureThread: Thread = _
@volatile private[kyuubi] var logCaptureThreadReleased: Boolean = true
private var logCaptureThread: Thread = _

private[kyuubi] lazy val engineLog: File = ProcBuilder.synchronized {
val engineLogTimeout = conf.get(KyuubiConf.ENGINE_LOG_TIMEOUT)
Expand Down Expand Up @@ -136,10 +137,12 @@ trait ProcBuilder {
case _: IOException =>
case _: InterruptedException =>
} finally {
logCaptureThreadReleased = true
reader.close()
}
}

logCaptureThreadReleased = false
logCaptureThread = PROC_BUILD_LOGGER.newThread(redirect)
logCaptureThread.start()
proc
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,13 +126,16 @@ class SparkProcessBuilderSuite extends KerberizedTestHelper {
test("log capture should release after close") {
val process = new FakeSparkProcessBuilder(KyuubiConf())
try {
assert(process.logCaptureThreadReleased)
val subProcess = process.start
assert(!process.logCaptureThread.isInterrupted)
assert(!process.logCaptureThreadReleased)
subProcess.waitFor(3, TimeUnit.SECONDS)
} finally {
process.close()
}
assert(process.logCaptureThread.isInterrupted)
eventually(timeout(3.seconds), interval(100.milliseconds)) {
assert(process.logCaptureThreadReleased)
}
}

test(s"sub process log should be overwritten") {
Expand Down

0 comments on commit 3c8cce4

Please sign in to comment.