Github user reggert commented on a diff in the pull request:
https://github.com/apache/spark/pull/9264#discussion_r44568927
--- Diff: core/src/main/scala/org/apache/spark/FutureAction.scala ---
@@ -116,57 +119,26 @@ class SimpleFutureAction[T] private[spark](jobWaiter:
JobWaiter[_], resultFunc:
}
override def ready(atMost: Duration)(implicit permit: CanAwait):
SimpleFutureAction.this.type = {
- if (!atMost.isFinite()) {
- awaitResult()
- } else jobWaiter.synchronized {
- val finishTime = System.currentTimeMillis() + atMost.toMillis
- while (!isCompleted) {
- val time = System.currentTimeMillis()
- if (time >= finishTime) {
- throw new TimeoutException
- } else {
- jobWaiter.wait(finishTime - time)
- }
- }
- }
+ jobWaiter.completionFuture.ready(atMost)
this
}
@throws(classOf[Exception])
override def result(atMost: Duration)(implicit permit: CanAwait): T = {
- ready(atMost)(permit)
- awaitResult() match {
- case scala.util.Success(res) => res
- case scala.util.Failure(e) => throw e
- }
+ jobWaiter.completionFuture.ready(atMost)
--- End diff --
I don't think so. `Future.ready` takes an implicit `CanAwait` as a
parameter, and in this case we're using the `CanAwait` passed in via the
enclosing call. `Await.ready` would completely ignore the passed-in `CanAwait`
and pass its own. The whole point of `CanAwait` is to verify that the developer
explicitly intended to block. At this point in the code, that verification has
already been done (by virtue of the `CanAwait` from the enclosing call).
---
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]
