aglinxinyuan commented on code in PR #5900:
URL: https://github.com/apache/texera/pull/5900#discussion_r3493544278
##########
amber/src/main/python/core/models/state.py:
##########
@@ -25,13 +25,41 @@
class State(dict):
CONTENT = "content"
- SCHEMA = Schema(raw_schema={CONTENT: "STRING"})
+ # Loop-control bookkeeping owned by the worker runtime, NOT user state --
it
+ # never appears in the content JSON. In memory it rides on the StateFrame
+ # envelope; it is materialized/serialized as its own column (parallel to
+ # content) by to_tuple(...). from_tuple() returns the bare State; callers
+ # that need these values read the corresponding columns off the tuple.
+ LOOP_COUNTER = "loop_counter"
+ LOOP_START_ID = "loop_start_id"
+ LOOP_START_STATE_URI = "loop_start_state_uri"
Review Comment:
Dug through the loop branch to answer this properly. Short version: agreed
it's constant config, and I'd go with **(1) hand it to LoopEnd at setup**,
which then lets us drop the persisted column as the cleanup it unlocks (your
**(3)**). I'd skip **(2)**.
One wrinkle worth flagging first — **(3) "in-memory StateFrame only" isn't
where we are today.** LoopStart and LoopEnd are separate workers in separate
regions, and the *only* thing carrying the URI across that boundary to LoopEnd
is the iceberg mat-reader rehydrating the StateFrame from the persisted
`loop_start_state_uri` column
(`input_port_materialization_reader_runnable.py:161-168`; `from_tuple` reads
only `content`, so the loop fields come straight from the columns). At the
write instant LoopEnd does read it from the in-memory
`self._loop_start_state_uri` (`main_loop.py:139`), but that field was populated
from `frame.loop_start_state_uri` (`:406`), whose value came off the persisted
column. So dropping the column with nothing in its place hands LoopEnd the
empty default across the region boundary — (3) needs a replacement carrier; it
can't stand alone.
That's exactly why **(1) fits best**: the URI is fully resolvable from
topology at schedule time — it's `stateURI(LoopStart's input-port base)`, and
that base is the upstream output port's `storageURIBase` the scheduler already
allocates and looks up by `GlobalPortIdentity`
(`CostBasedScheduleGenerator.scala:244-257`). `set_up_port_storage_writer`
already derives `state_uri(base)` the exact same way LoopStart does at runtime
(`output_manager.py:160-169` vs `main_loop.py:129`). Inject it at setup and
both the persisted column **and** the StateFrame field go away — fully
realizing "config, not per-iteration data."
The honest cost of (1): (a) `AssignPort` today only carries a worker's *own*
port URIs, so handing LoopEnd a *sibling's* (LoopStart's input) URI needs a new
field / dedicated setup message; (b) the scheduler has to know the
LoopStart↔LoopEnd pairing at schedule time — today that's only known at runtime
via `loop_start_id` on the frame.
On **(2)**: the controller *can* resolve the URI from `loop_start_id` (it
holds the plan + allocated port URIs and already calls `VFSURIFactory.stateURI`
— `RegionExecutionCoordinator.scala:572-574`), so that half is fine. But the
"controller already does the write or hands it back" premise isn't true today —
`JumpToOperatorRegionHandler` only re-points the schedule level and returns
`EmptyReturn`, and the back-edge write is worker-side (`main_loop.py:146-150`).
Making the controller perform the write is a brand-new cross-language storage
write (plus shipping the state payload controller-ward), and the "return the
URI in the jump reply" sub-variant collapses to (3)'s effect while adding a
round-trip on the hot jump path — so it's dominated by (1).
Net: (1) for the durable fix, (3) as the cleanup it enables, skip (2). I'll
carry this into the loop PR — and you weren't missing a constraint so much as
(3) being further along than it actually is: the cross-region hop is what keeps
the URI on the persisted row today.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
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]
