The GitHub Actions job "Benchmarks" on texera.git/main has succeeded. Run started by GitHub user github-merge-queue[bot] (triggered by github-merge-queue[bot]).
Head commit for run: d1ac8dc3b76d3248bebc5c289ece6aa8c84f0766 / Xinyuan Lin <[email protected]> fix(amber): pass unstamped boundary states through LoopEnd instead of consuming them (#6913) ### What changes were proposed in this PR? Follow-up to #6661, addressing [this review comment](https://github.com/apache/texera/pull/6661#discussion_r3648708075): a loop-body operator that emits its own boundary state (`produce_state_on_start/finish` — a public API on both engine sides) sends it with the "no loop" envelope (counter `0`, `loop_start_id ""`). The LoopEnd matching branch treated **every** counter-0 frame as the loop's own boundary state: ``` LoopStart ──(0, LS-id)──▶ stateful body op ──▶ LoopEnd │ produce_state_on_finish └──(0, "")────────▶ LoopEnd ← arrives AFTER the loop state ``` Consuming the unstamped state (1) clobbers the captured back-jump id with `""` → `no loop-back state URI configured for LoopStart ''`, and (2) hands `run_update` a State with no `table` payload → `KeyError`. Either way the loop breaks. The reviewer reasoned this for a stateful JVM operator; it is language-independent (a Python UDF hits the identical path — no JVM built-in currently overrides `produceStateOnFinish`, so the Python UDF is also the practical repro). **Fix (consumer-side).** A real loop state is always stamped — the matching LoopStart stamps its own id on every iteration's output state — so the LoopEnd runtime now keys on the stamp: | Frame at LoopEnd (counter 0) | before | after | |---|---|---| | stamped (`loop_start_id` set) | consume + capture id | unchanged | | unstamped (`""`) | consume → id clobber / `KeyError` | forward downstream unchanged, skip the operator (default pass-through semantics), captured id untouched | **Keeping the loud failure.** Forwarding unstamped frames also swallows the symptom of the bug class #6660/#6661 fixed: a hop that blanks the envelope makes the loop's *own* state arrive unstamped, and forwarding it leaves `_loop_table` `None` → `condition()` returns `False` → the loop stops after one iteration and reports **success with a wrong row count**. So a Loop End that forwarded an unstamped state and never took a stamped one now raises: ``` Loop End received a loop-boundary state with no LoopStart stamp and never received its own (stamped) loop state: the loop envelope was lost upstream, so this loop would silently stop after one iteration ``` Three properties of where that check lives: - **`_process_end_channel`, not `complete()`** — `complete()` runs after `port_completed` has gone out for the input port and every output port, and region completion is port-based, so a raise there would be reported only once the coordinator already considers the region done. - **Order-independent** — the body operator's state may arrive before or after the loop's; `EndChannel` is `PORT_ALIGNMENT`, so the port is drained by then. - **Reads nothing out of the `State`** — deliberately not keyed on the reserved `table` key, which #6971 removes from the loop state (and which an ordinary body UDF may legitimately emit), so the guard cannot rot green. It is narrow on purpose — *forwarded an unstamped state* **and** *never took a stamped one*. A Loop End completing without any matching state is legal (`LoopEndOperator.eval_condition`'s `_loop_table` guard), so only positive evidence of an unstamped boundary state counts. Also documents why a Loop **Start** must do the opposite — MERGE an unstamped counter-0 state rather than forward it: the back-edge writes the next iteration's variables to the Loop Start's own input-port state URI with that same "no loop" envelope (`State.to_tuple(0)`), so a Loop Start cannot tell its own state from an upstream operator's, while a Loop End can. The key-collision hazard that follows from the merge is filed as #7248. ### Any related issues, documentation, discussions? Follow-up to #6661 (review discussion r3648708075). Related engine context: #6660. Follow-up filed: #7248. ### How was this PR tested? - **Unit** (`test_main_loop.py`), all verified red before the corresponding change: - an unstamped counter-0 frame at a LoopEnd is forwarded with its envelope unchanged, the operator is not invoked, the captured back-jump id is not clobbered, and it does not count as taking the loop's own state; - a LoopEnd that only ever saw an unstamped state reports the error from `_process_end_channel` and sends **no** `port_completed`, so the region is held; - the legitimate shape (body-operator state *plus* the loop's own stamped state) stays silent in **both** arrival orders; - an unstamped counter-0 frame at a Loop **Start** is merged, not forwarded. - **E2E** (`LoopIntegrationSpec`, CI-only): `TextInput → LoopStart → stateful Python UDF → LoopEnd` where the UDF emits boundary state via `produce_state_on_finish` — it crashes without this fix and completes exactly 3 iterations with it. - Full pyamber suite, `scalafmtCheckAll` + `scalafixAll --check` + full test-compile + ruff format/check pass locally (Java 17). ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 5) --------- Co-authored-by: Yicong Huang <[email protected]> Report URL: https://github.com/apache/texera/actions/runs/31357444388 With regards, GitHub Actions via GitBox
