Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/6600#discussion_r31650613
--- Diff:
yarn/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala ---
@@ -359,7 +360,9 @@ private[spark] class ApplicationMaster(
}
logDebug(s"Number of pending allocations is
$numPendingAllocate. " +
s"Sleeping for $sleepInterval.")
- Thread.sleep(sleepInterval)
+ allocatorLock.synchronized {
+ allocatorLock.wait(sleepInterval)
--- End diff --
Yeah, I think it's not strictly necessary but good practice. Though here
we're technically in a loop, just a bigger one. Since we intend to allocate
stuff periodically the worst thing that can possibly happen is some additional
latency, and this only happens if there is a 3rd thread somewhere (does not
exist).
In other words the spurious wake ups are benign and not possible given the
changes here. I'm fine with this being merged as is.
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]