carloea2 commented on code in PR #6729:
URL: https://github.com/apache/texera/pull/6729#discussion_r3662869211
##########
amber/src/main/scala/org/apache/texera/amber/engine/architecture/coordinator/execution/ExecutionUtils.scala:
##########
@@ -81,20 +82,30 @@ object ExecutionUtils {
)
}
+ /**
+ * Rolls a group of execution states up into one workflow-level state.
+ *
+ * When `cachedState` is provided and every state equals it, the group is
+ * reported as CACHE_REUSED. `cachedState` defaults to None, so an
+ * empty cache leaves this method byte-identical to before.
+ */
def aggregateStates[T](
states: Iterable[T],
completedState: T,
terminatedState: T,
runningState: T,
uninitializedState: T,
pausedState: T,
- readyState: T
+ readyState: T,
+ cachedState: Option[T] = None
): WorkflowAggregatedState = {
states match {
case _ if states.isEmpty =>
WorkflowAggregatedState.UNINITIALIZED
case _ if states.forall(_ == completedState) =>
WorkflowAggregatedState.COMPLETED
case _ if states.forall(_ == terminatedState) =>
WorkflowAggregatedState.COMPLETED
- case _ if states.exists(_ == runningState) =>
WorkflowAggregatedState.RUNNING
+ case _ if cachedState.isDefined && states.forall(_ == cachedState.get) =>
Review Comment:
Could you help confirm the intended behavior for mixed cached states? My
reading of #5883 is that CACHE_REUSED should behave as a terminal state. With
the current fallback, combinations such as COMPLETED + CACHE_REUSED appear to
become UNKNOWN, and cached plus paused/ready may also fall through to UNKNOWN.
Would it make sense to preserve the all-cached CACHE_REUSED case, then exclude
completed, terminated, and cached states before classifying the remainder? This
may also matter when resolving the conflict with #6011.
--
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]
