[SPARK-20935][STREAMING] Always close WriteAheadLog and make it idempotent #18224
Conversation
HyukjinKwon
changed the title
|
Test build #77792 has finished for PR 18224 at commit
|
|
Test build #77793 has finished for PR 18224 at commit
|
There was a problem hiding this comment.
"It must be idempotent" or "It is required to be idempotent"
There was a problem hiding this comment.
active might be simpler as an AtomicBoolean. Then I think you can just write if (!active.getAndSet(false)) return as the very first line.
There was a problem hiding this comment.
Does this need to be thread-safe? it seems like the impl just above tries to be. I don't know if it's needed.
There was a problem hiding this comment.
@srowen, maybe I missed your point. However, wouldn't there be a chance to try shutdown multiple times if isShutdown is not synchronised together? Probably, shutdown is idempotent itself but just wonder if it is okay to keep this as a safeguard.
HyukjinKwon
changed the title
HyukjinKwon
force-pushed
the
streaming-closing
branch
from
9ef6dba to
76546c3
Compare
HyukjinKwon
changed the title
|
Test build #77875 has finished for PR 18224 at commit
|
|
Merged to master |
…otent ## What changes were proposed in this pull request? This PR proposes to stop `ReceiverTracker` to close `WriteAheadLog` whenever it is and make `WriteAheadLog` and its implementations idempotent. ## How was this patch tested? Added a test in `WriteAheadLogSuite`. Note that the added test looks passing even if it closes twice (namely even without the changes in `FileBasedWriteAheadLog` and `BatchedWriteAheadLog`. It looks both are already idempotent but this is a rather sanity check. Author: hyukjinkwon <[email protected]> Closes apache#18224 from HyukjinKwon/streaming-closing.
3 participants
What changes were proposed in this pull request?
This PR proposes to stop
ReceiverTrackerto closeWriteAheadLogwhenever it is and makeWriteAheadLogand its implementations idempotent.How was this patch tested?
Added a test in
WriteAheadLogSuite. Note that the added test looks passing even if it closes twice (namely even without the changes inFileBasedWriteAheadLogandBatchedWriteAheadLog. It looks both are already idempotent but this is a rather sanity check.