Xiao-zhen-Liu commented on code in PR #6729:
URL: https://github.com/apache/texera/pull/6729#discussion_r3801607189
##########
amber/src/main/scala/org/apache/texera/amber/engine/architecture/coordinator/execution/ExecutionUtils.scala:
##########
@@ -77,7 +77,11 @@ object ExecutionUtils {
dataProcessingTimeSum,
controlProcessingTimeSum,
idleTimeSum
- )
+ ),
+ // A logical operator is reused from cache only when every one of its
+ // physical operators is. `metrics` is non-empty here, so this cannot
+ // hold vacuously.
+ reusedFromCache = metrics.forall(_.reusedFromCache)
Review Comment:
Partial reuse is possible: reuse is decided per region, and a logical
operator's physical operators can sit in different regions (HashJoin's build
and probe). So the forall is a deliberate choice of fully-reused semantics: a
partially reused operator reports false at the operator level, and the per-port
marks in #5886 carry the partial detail from the cache entries. That also makes
the window you describe real, so the flag should get the same late-region
handling as the state field; that rides with the producer in #5884. The comment
now states both.
##########
amber/src/main/scala/org/apache/texera/web/model/websocket/event/OperatorStatisticsUpdateEvent.scala:
##########
@@ -30,7 +30,9 @@ case class OperatorAggregatedMetrics(
numWorkers: Long,
aggregatedDataProcessingTime: Long,
aggregatedControlProcessingTime: Long,
- aggregatedIdleTime: Long
+ aggregatedIdleTime: Long,
+ // Provenance: the operator completed by reusing cached results (no
workers ran).
+ reusedFromCache: Boolean = false
Review Comment:
Dropped the default; both sites pass it explicitly, so the next construction
site fails to compile instead of silently sending false.
--
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]
