Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/21068#discussion_r184215914
--- Diff:
resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala
---
@@ -160,26 +155,12 @@ private[yarn] class YarnAllocator(
private[yarn] val containerPlacementStrategy =
new LocalityPreferredContainerPlacementStrategy(sparkConf, conf,
resource, resolver)
- /**
- * Use a different clock for YarnAllocator. This is mainly used for
testing.
- */
- def setClock(newClock: Clock): Unit = {
- clock = newClock
- }
-
def getNumExecutorsRunning: Int = runningExecutors.size()
- def getNumExecutorsFailed: Int = synchronized {
- val endTime = clock.getTimeMillis()
+ def getNumExecutorsFailed: Int =
failureWithinTimeIntervalTracker.getNumExecutorsFailed
- while (executorFailuresValidityInterval > 0
- && failedExecutorsTimeStamps.nonEmpty
- && failedExecutorsTimeStamps.head < endTime -
executorFailuresValidityInterval) {
- failedExecutorsTimeStamps.dequeue()
- }
-
- failedExecutorsTimeStamps.size
- }
+ def getFailureWithinTimeIntervalTracker:
FailureWithinTimeIntervalTracker =
--- End diff --
This name is kinda awkward. How about just `getFailureTracker` or something
simple along those lines? That could be applied to the class name also.
(e.g. if you add any logic that is not time-based to that class, then its
name becomes inaccurate.)
Also instead of a getter you could just make the field public (it's already
a `val`).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
