Yicong-Huang commented on code in PR #7122:
URL: https://github.com/apache/texera/pull/7122#discussion_r3755031871
##########
amber/src/main/scala/org/apache/texera/amber/engine/architecture/scheduling/RegionExecutionManager.scala:
##########
@@ -196,37 +198,43 @@ class RegionExecutionManager(
}.toSeq
val endWorkerFuture: Future[Unit] =
- Future.collect(endWorkerRequests).unit
+ Future
+ .collect(endWorkerRequests)
+ .within(killTimeout)
+ .unit
- // 2. Send GracefulStops only after 1 has finished
+ // 2. Send GracefulStops with timeout
val gracefulStopRequests: Future[Unit] =
endWorkerFuture.flatMap { _ =>
val gracefulStops =
regionExecution.getAllOperatorExecutions.flatMap {
case (_, opExec) =>
opExec.getWorkerIds.map { workerId =>
val actorRef = actorRefService.getActorRef(workerId)
- // Remove the actorRef so that no other actors can find the
worker and send messages.
- actorRefService.removeActorRef(workerId)
- // Restarted regions reuse actorId. Remove stale control
channels so the
- // coordinator does not reuse old control-message sequence
numbers for new workers.
- asyncRPCClient.inputGateway.removeControlChannel(workerId)
- asyncRPCClient.outputGateway.removeControlChannel(workerId)
gracefulStop(actorRef, ScalaDuration(5,
TimeUnit.SECONDS)).asTwitter()
}
}.toSeq
- Future.collect(gracefulStops).unit
+ Future
+ .collect(gracefulStops)
+ .within(killTimeout)
Review Comment:
Partly addressed: the numbers moved but the new one is also wrong.
`.within(killTimeout)` is applied twice per attempt — once on the `EndWorker`
collect (`:204-208`) and once on the `gracefulStop` collect (`:222-225`) — so
an attempt bounds two sequential stages, not one. Worst case is ~45 s (`4 × (6
s + 5 s) + 1.4 s`), not 25.4 s, which makes it longer than the ~30 s the
comment cites as the thing being improved on. Leaving this open; details in the
new review.
--
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]
