Xiao-zhen-Liu commented on code in PR #6661:
URL: https://github.com/apache/texera/pull/6661#discussion_r3648708075
##########
amber/src/main/scala/org/apache/texera/amber/engine/architecture/messaginglayer/OutputManager.scala:
##########
@@ -192,9 +192,16 @@ class OutputManager(
buffersToFlush.foreach(_.flush())
}
- def emitState(state: State): Unit = {
- networkOutputBuffers.foreach(kv => kv._2.sendState(state))
- saveStateToStorageIfNeeded(state)
+ /**
+ * Emit a State to every network buffer and (if configured) the state
+ * storage. `loopCounter` / `loopStartId` are the loop envelope riding
+ * alongside the State (see `StateFrame`); a JVM hop inside a loop body
+ * passes the incoming envelope through unchanged, while a Scala-originated
+ * state (start/end-channel handlers) uses the "no loop" defaults.
+ */
+ def emitState(state: State, loopCounter: Long = 0L, loopStartId: String =
""): Unit = {
Review Comment:
Non-blocking follow-up. These defaults (`0` / `""`) are right for the
forwarded-state path, which now carries the real loop id and counter. But two
callers keep the defaults for operator-originated boundary state:
`StartChannelHandler:45` and `EndChannelHandler:46` (`produceStateOnStart` /
`OnFinish`).
For a stateless passthrough (`Limit`, `Sleep` — what the e2e cases use)
that's fine, since they emit no boundary state. But a stateful JVM operator in
a loop body would emit boundary state here with `(0, "")`, and LoopEnd captures
its back-jump target from every state it consumes at counter 0
(`main_loop.py:380`, `self._loop_start_id = frame.loop_start_id`). On
`endChannel` that boundary state arrives after the forwarded loop state, so it
overwrites the captured id with `""` — reproducing the same `no loop-back state
URI configured for LoopStart ''` crash, through a path this fix doesn't cover.
I reasoned this from the code but haven't run it: worth either confirming a
stateful JVM operator in a loop body works, or documenting that as unsupported.
Related: because these args default silently, a future state path that misses
them would drop the loop id and counter with no compile-time error — worth a
short comment noting the two boundary-state callers are the deliberate default
cases.
--
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]
