-
Notifications
You must be signed in to change notification settings - Fork 28.8k
[STREAMING] SPARK-4986 Wait for receivers to deregister and receiver job to terminate #4338
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
Conversation
Signed-off-by: Jesper Lundgren <[email protected]>
Signed-off-by: Jesper Lundgren <[email protected]>
Signed-off-by: Jesper Lundgren <[email protected]>
Can one of the admins verify this patch? |
ok to test. |
Test build #26662 has started for PR 4338 at commit
|
override def run() { | ||
logInfo("Receiving started") | ||
for(i <- 1 to totalRecords) { | ||
Thread.sleep(recordsPerSecond * 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.
I dont get the logic here. Should the delay be inverse of recordsPerSecond? Higher the rate, lower the delay?
@cleaton Thanks for this PR. It looks reasonably good, except a few minor comments. Could you please address them very soon? I would like to merge this today for Spark 1.3. |
@tdas I will address it now. I have one question though. Don't you think there at least should be some sort of timeout while waiting for the receivers to deregister? I am afraid that if a receiver can not de-register for whatever reason the shutdown sequence will be stuck forever. edit: this might not be a problem since dead/stuck receivers will timeout and become de-registered? |
Test build #26662 has finished for PR 4338 at commit
|
Test PASSed. |
Test build #26674 has started for PR 4338 at commit
|
Test build #26674 has finished for PR 4338 at commit
|
Test PASSed. |
Alright, I like this. I am merging this. Please submit the other PR as well :) |
…job to terminate A slow receiver might not have enough time to shutdown cleanly even when graceful shutdown is used. This PR extends graceful waiting to make sure all receivers have deregistered and that the receiver job has terminated. Author: Jesper Lundgren <[email protected]> Closes #4338 from cleaton/stopreceivers and squashes the following commits: a9cf223 [Jesper Lundgren] remove cleaner.ttl config f969b6e [Jesper Lundgren] fix inversed logic in unit test 3d0bd35 [Jesper Lundgren] switch boleans to match running status instead of terminated 9a9ff88 [Jesper Lundgren] wait for receivers to shutdown and receiver job to terminate d179372 [Jesper Lundgren] Add graceful shutdown unit test covering slow receiver onStop (cherry picked from commit 1e8b539) Signed-off-by: Tathagata Das <[email protected]>
…job to terminate A slow receiver might not have enough time to shutdown cleanly even when graceful shutdown is used. This PR extends graceful waiting to make sure all receivers have deregistered and that the receiver job has terminated. Author: Jesper Lundgren <[email protected]> Closes #4338 from cleaton/stopreceivers and squashes the following commits: a9cf223 [Jesper Lundgren] remove cleaner.ttl config f969b6e [Jesper Lundgren] fix inversed logic in unit test 3d0bd35 [Jesper Lundgren] switch boleans to match running status instead of terminated 9a9ff88 [Jesper Lundgren] wait for receivers to shutdown and receiver job to terminate d179372 [Jesper Lundgren] Add graceful shutdown unit test covering slow receiver onStop (cherry picked from commit 1e8b539) Signed-off-by: Tathagata Das <[email protected]>
A slow receiver might not have enough time to shutdown cleanly even when graceful shutdown is used. This PR extends graceful waiting to make sure all receivers have deregistered and that the receiver job has terminated.