Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/16620#discussion_r99660324
--- Diff:
core/src/test/scala/org/apache/spark/scheduler/SchedulerIntegrationSuite.scala
---
@@ -391,17 +391,18 @@ private[spark] abstract class MockBackend(
* scheduling.
*/
override def reviveOffers(): Unit = {
- val newTaskDescriptions =
taskScheduler.resourceOffers(generateOffers()).flatten
- // get the task now, since that requires a lock on TaskSchedulerImpl,
to prevent individual
- // tests from introducing a race if they need it
- val newTasks = taskScheduler.synchronized {
- newTaskDescriptions.map { taskDescription =>
- val taskSet =
taskScheduler.taskIdToTaskSetManager(taskDescription.taskId).taskSet
- val task = taskSet.tasks(taskDescription.index)
- (taskDescription, task)
- }
- }
- synchronized {
+ // Need a lock on the entire scheduler to protect freeCores --
otherwise, multiple threads
+ // may make offers at the same time, though they are using the same
set of freeCores.
+ taskScheduler.synchronized {
+ val newTaskDescriptions =
taskScheduler.resourceOffers(generateOffers()).flatten
--- End diff --
Is this change fixing an existing bug in the test? (if so would you mind
putting the change in its own small PR that we can quickly merge?)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
