Skip to content

Commit

Permalink
[SPARK-31248][CORE][TEST] Fix flaky ExecutorAllocationManagerSuite.in…
Browse files Browse the repository at this point in the history
…terleaving add and remove

### What changes were proposed in this pull request?

This PR (SPARK-31248) uses `ManualClock` to disable `ExecutorAllocationManager.schedule()`  in order to avoid unexpected update of target executors.

### Why are the changes needed?

`ExecutorAllocationManager` will call `schedule` periodically, which may update target executors before we checking https://github.com/apache/spark/blob/496f6ac86001d284cbfb7488a63dd3a168919c0f/core/src/test/scala/org/apache/spark/ExecutorAllocationManagerSuite.scala#L864

And fail the check:

```
sbt.ForkMain$ForkError: org.scalatest.exceptions.TestFailedException: 12 did not equal 8
	at org.scalatest.Assertions.newAssertionFailedException(Assertions.scala:530)
	at org.scalatest.Assertions.newAssertionFailedException$(Assertions.scala:529)
	at org.scalatest.FunSuite.newAssertionFailedException(FunSuite.scala:1560)
	at org.scalatest.Assertions$AssertionsHelper.macroAssert(Assertions.scala:503)
	at org.apache.spark.ExecutorAllocationManagerSuite.$anonfun$new$51(ExecutorAllocationManagerSuite.scala:864)
	at org.scalatest.OutcomeOf.outcomeOf(OutcomeOf.scala:85)
	at org.scalatest.OutcomeOf.outcomeOf$(OutcomeOf.scala:83)
	at org.scalatest.OutcomeOf$.outcomeOf(OutcomeOf.scala:104)
	at org.scalatest.Transformer.apply(Transformer.scala:22)
	at org.scalatest.Transformer.apply(Transformer.scala:20)
	at org.scalatest.FunSuiteLike$$anon$1.apply(FunSuiteLike.scala:186)
	at org.apache.spark.SparkFunSuite.withFixture(SparkFunSuite.scala:151)
	at org.scalatest.FunSuiteLike.invokeWithFixture$1(FunSuiteLike.scala:184)
```

### Does this PR introduce any user-facing change?

No.

### How was this patch tested?

Update test.

Closes apache#28084 from Ngone51/spark_31248.

Authored-by: yi.wu <yi.wu@databricks.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
  • Loading branch information
Ngone51 authored and Seongjin Cho committed Apr 14, 2020
1 parent d73ab29 commit f6de6b8
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ private[spark] class ExecutorAllocationManager(
} else {
logDebug(s"Lowering target number of executors to" +
s" ${numExecutorsTargetPerResourceProfileId(rpId)} (previously " +
s"$targetNum.oldNumExecutorsTarget for resource profile id: ${rpId}) " +
s"${targetNum.oldNumExecutorsTarget} for resource profile id: ${rpId}) " +
"because not all requested executors " +
"are actually needed")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -841,7 +841,10 @@ class ExecutorAllocationManagerSuite extends SparkFunSuite {
}

test ("interleaving add and remove") {
val manager = createManager(createConf(5, 12, 5))
// use ManualClock to disable ExecutorAllocationManager.schedule()
// in order to avoid unexpected update of target executors
val clock = new ManualClock()
val manager = createManager(createConf(5, 12, 5), clock)
post(SparkListenerStageSubmitted(createStageInfo(0, 1000)))

val updatesNeeded =
Expand Down

0 comments on commit f6de6b8

Please sign in to comment.