aglinxinyuan commented on code in PR #7154:
URL: https://github.com/apache/texera/pull/7154#discussion_r3697196458
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/loop/LoopOpDesc.scala:
##########
@@ -94,7 +94,14 @@ abstract class LoopOpDesc extends LogicalOp {
operatorName,
operatorDescription,
OperatorGroupConstants.CONTROL_GROUP,
- inputPorts = List(InputPort()),
+ // A loop operator takes exactly one link on its input port. Every reader
+ // on a materialized input port replays that port's states independently,
+ // so a second link would deliver the loop state twice per iteration; a
+ // Loop Start additionally needs a single reader for the scheduler to
+ // resolve its bookkeeping URIs from. Declaring it here is what makes the
+ // GUI refuse to draw the second link, instead of the plan being rejected
+ // only at StartWorkflow (discussion #6966).
+ inputPorts = List(InputPort(disallowMultiLinks = true)),
Review Comment:
Correcting my earlier reply on this thread: the answer is the opposite of
what I said. Loop End is *meant* to take two or more inbound links — a loop
body may branch and converge on it — so the gap is not a missing guard, it's
that the runtime double-consumes.
Changed in this PR:
- `disallowMultiInputLinks` is now overridable and defaults to false; only
Loop Start sets it, so the GUI restricts Loop Start alone. (Loop Start stays
single-input because the scheduler resolves the loop's bookkeeping URIs from
that port's single reader.)
- `MainLoop` consumes a Loop End's loop state once per iteration. Each
reader replays its own branch's copy of the same state, and running `update`
per copy would advance the loop variables once per branch and end the loop
early. The copies are identical (one emission from the matching Loop Start) and
a consume emits nothing downstream, so the extras are dropped.
#7246 is retitled to that bug and is closed by this PR rather than left as a
follow-up. Your point about `filter(_.isLoopStart)` still stands and is now the
documented reason the `require` is Loop-Start-only.
--
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]
