Skip to content
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

[SPARK-10137][Streaming]Avoid to restart receivers if scheduleReceivers returns balanced results #8340

Closed
wants to merge 9 commits into from
Closed

Conversation

zsxwing
Copy link
Member

@zsxwing zsxwing commented Aug 20, 2015

This PR fixes the following cases for ReceiverSchedulingPolicy.

  1. Assume there are 4 executors: host1, host2, host3, host4, and 5 receivers: r1, r2, r3, r4, r5. Then ReceiverSchedulingPolicy.scheduleReceivers will return (r1 -> host1, r2 -> host2, r3 -> host3, r4 -> host4, r5 -> host1).
    Let's assume r1 starts at first on host1 as scheduleReceivers suggested, and try to register with ReceiverTracker. But the previous ReceiverSchedulingPolicy.rescheduleReceiver will return (host2, host3, host4) according to the current executor weights (host1 -> 1.0, host2 -> 0.5, host3 -> 0.5, host4 -> 0.5), so ReceiverTracker will reject r1. This is unexpected since r1 is starting exactly where scheduleReceivers suggested.

This case can be fixed by ignoring the information of the receiver that is rescheduling in receiverTrackingInfoMap.

  1. Assume there are 3 executors (host1, host2, host3) and each executors has 3 cores, and 3 receivers: r1, r2, r3. Assume r1 is running on host1. Now r2 is restarting, the previous ReceiverSchedulingPolicy.rescheduleReceiver will always return (host1, host2, host3). So it's possible that r2 will be scheduled to host1 by TaskScheduler. r3 is similar. Then at last, it's possible that there are 3 receivers running on host1, while host2 and host3 are idle.

This issue can be fixed by returning only executors that have the minimum wight rather than returning at least 3 executors.

@SparkQA
Copy link

SparkQA commented Aug 20, 2015

Test build #41322 has finished for PR 8340 at commit 5fba8d4.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

val minWeight = sortedExecutors(0)._2
scheduledExecutors ++= sortedExecutors.takeWhile(_._2 == minWeight).map(_._1)
} else {
// This should not happen since "executors" is not empty
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you talking about executorWeights no being empty? And why it cannot be empty? Is it because that by the timerescheduleReceiveris called, all the receivers have already been scheduling byscheduleReceivers` which means that there are some weights for at some executors?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I mean, because idleExecutors is empty and executors is not empty, since sortedExecutors.keys == executors - idleExecutors, sortedExecutors must be not empty.

@SparkQA
Copy link

SparkQA commented Aug 21, 2015

Test build #41370 has finished for PR 8340 at commit f0d1f6e.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@tdas
Copy link
Contributor

tdas commented Aug 21, 2015

Could you update the main text of this PR and the JIRA to document the main change? This is a significant change and good to document. Also link this JIRA with the original receiver scheduling JIRA.

@@ -431,7 +450,8 @@ class ReceiverTracker(ssc: StreamingContext, skipReceiverLaunch: Boolean = false
receiver.preferredLocation,
receiverTrackingInfos,
getExecutors)
updateReceiverScheduledExecutors(receiver.streamId, scheduledExecutors)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not update the scheduled info in case of rescheduleReceiver? Why have these two different code paths / policies in two cases.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zsxwing explained me this offline. In case of resecheduling, the scheduled executors are not stored so that the following scenario does not occur.

  • Initial globally-optimal schedule is stored, but one receiver gets launched incorrectly.
  • The receiver is rejected and therefore has to be rescheduled, but if the rescheduled location (which is locally-optimal for that receiver) is saved, it will overwrite the original globally optimal location, and will get launched somewhere else that does not ensure the proper global balancing.

@SparkQA
Copy link

SparkQA commented Aug 24, 2015

Test build #41452 has finished for PR 8340 at commit 55d9957.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Aug 24, 2015

Test build #41455 has finished for PR 8340 at commit c70690f.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@tdas
Copy link
Contributor

tdas commented Aug 24, 2015

Jenkins, test this please.

@SparkQA
Copy link

SparkQA commented Aug 24, 2015

Test build #41464 has finished for PR 8340 at commit c70690f.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

return false
}

val scheduledExecutors = receiverTrackingInfos(streamId).scheduledExecutors
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be more intuitive rewrite this nested if to the following:

val scheduledExecutors = receiverTrackingInfos(streamId).scheduledExecutors
val accetableExecutors = if (scheduledExecutors.nonEmpty) {
  scheduledExecutors 
} else {
  scheduleReceiver(streamId).contains(hostPort)
}
if (!accetableExecutors.contains(hostPort)) {
  false
} else {
  // existing code to update ReceiverTrackingInfo
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

@SparkQA
Copy link

SparkQA commented Aug 25, 2015

Test build #41502 has finished for PR 8340 at commit 55fe99e.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

asfgit pushed a commit that referenced this pull request Aug 25, 2015
…vers returns balanced results

This PR fixes the following cases for `ReceiverSchedulingPolicy`.

1) Assume there are 4 executors: host1, host2, host3, host4, and 5 receivers: r1, r2, r3, r4, r5. Then `ReceiverSchedulingPolicy.scheduleReceivers` will return (r1 -> host1, r2 -> host2, r3 -> host3, r4 -> host4, r5 -> host1).
Let's assume r1 starts at first on `host1` as `scheduleReceivers` suggested,  and try to register with ReceiverTracker. But the previous `ReceiverSchedulingPolicy.rescheduleReceiver` will return (host2, host3, host4) according to the current executor weights (host1 -> 1.0, host2 -> 0.5, host3 -> 0.5, host4 -> 0.5), so ReceiverTracker will reject `r1`. This is unexpected since r1 is starting exactly where `scheduleReceivers` suggested.

This case can be fixed by ignoring the information of the receiver that is rescheduling in `receiverTrackingInfoMap`.

2) Assume there are 3 executors (host1, host2, host3) and each executors has 3 cores, and 3 receivers: r1, r2, r3. Assume r1 is running on host1. Now r2 is restarting, the previous `ReceiverSchedulingPolicy.rescheduleReceiver` will always return (host1, host2, host3). So it's possible that r2 will be scheduled to host1 by TaskScheduler. r3 is similar. Then at last, it's possible that there are 3 receivers running on host1, while host2 and host3 are idle.

This issue can be fixed by returning only executors that have the minimum wight rather than returning at least 3 executors.

Author: zsxwing <zsxwing@gmail.com>

Closes #8340 from zsxwing/fix-receiver-scheduling.

(cherry picked from commit f023aa2)
Signed-off-by: Tathagata Das <tathagata.das1565@gmail.com>
@asfgit asfgit closed this in f023aa2 Aug 25, 2015
@zsxwing zsxwing deleted the fix-receiver-scheduling branch August 31, 2015 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants