aglinxinyuan commented on code in PR #7028:
URL: https://github.com/apache/texera/pull/7028#discussion_r3678876308
##########
amber/src/main/scala/org/apache/texera/amber/engine/common/Utils.scala:
##########
@@ -59,33 +57,6 @@ object Utils extends LazyLogging {
}
val AMBER_HOME_FOLDER_NAME = "amber";
- /**
- * Retry the given logic with a backoff time interval. The attempts are
executed sequentially, thus blocking the thread.
- * Backoff time is doubled after each attempt.
- *
- * @param attempts total number of attempts. if n <= 1 then it
will not retry at all, decreased by 1 for each recursion.
- * @param baseBackoffTimeInMS time to wait before next attempt, started
with the base time, and doubled after each attempt.
- * @param fn the target function to execute.
- * @tparam T any return type from the provided function fn.
- * @return the provided function fn's return, or any exception that still
being raised after n attempts.
- */
- @tailrec
- def retry[T](attempts: Int, baseBackoffTimeInMS: Long)(fn: => T): T = {
Review Comment:
Sure — restored in 88ac489. `Utils.scala` and `UtilsSpec.scala` are
byte-identical to `main` again, so this PR is now just the `OrderingEnforcer`
deletion. Retitled to match.
Two notes on the EndWorker case, in case they're useful later.
`LoopIntegrationSpec` doesn't use `retry`/`eventually`/`Thread.sleep` today,
and the EndWorker retry that does exist is `RegionExecutionManager`'s own
(`DefaultMaxTerminationAttempts = 150` at `DefaultKillRetryDelay = 200ms`) —
that path is Twitter-`Future`-based, so it couldn't call `Utils.retry` without
blocking a thread inside `Thread.sleep`. So a future async retry there probably
wants a different shape regardless.
And one optional follow-up now that it's staying: `retry` catches
`Throwable` rather than `Exception`, so it retries `OutOfMemoryError` and —
more awkwardly — `ControlThrowable`, meaning a non-local `return` inside `fn`
gets swallowed and retried. The `Thread.sleep` also sits inside the `catch`
block, so an `InterruptedException` during backoff escapes with the interrupt
flag cleared. Both are one-line fixes; happy to do them in a separate PR if
you'd like.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
