aglinxinyuan commented on code in PR #4560:
URL: https://github.com/apache/texera/pull/4560#discussion_r3165770541


##########
amber/src/main/python/core/runnables/main_loop.py:
##########
@@ -192,7 +192,6 @@ def process_input_tuple(self) -> None:
     def process_input_state(self) -> None:
         self._switch_context()
         output_state = self.context.state_processing_manager.get_output_state()
-        self._switch_context()

Review Comment:
   I don't think this allows DataProcessor to advance — DP stays parked on the 
condition variable until MainLoop notifies it, and MainLoop doesn't notify here.
   
   Walking through the same scenario after the change: MainLoop's single 
`self._switch_context()` returns when DP fires its per-task `finally: 
self._switch_context()` (i.e. after the executor has run and 
`current_output_state` has been written). At that moment DP is at its 
`finally`'s `wait()` — it has notified MainLoop but is now blocked waiting for 
a notify back. MainLoop then runs `get_output_state()` and `emit_state(...)`; 
neither touches the condition variable, so no notify reaches DP. DP cannot 
"move to process next data" because `wait()` is still blocking. It stays parked 
until MainLoop's *next* call to `_switch_context()` (the next state cycle, the 
next `process_input_tuple` iter, etc.).
   
   The second `self._switch_context()` that this PR removes was effectively a 
one-off round-trip: notify DP from its finally, DP exits the finally and 
re-parks at the run-loop's end-of-body position (line 65 on `main`), notifies 
MainLoop back, MainLoop returns. No new task ran during that round-trip, and 
`current_output_state` had already been read by MainLoop *before* the second 
switch ran. So removing it doesn't change what DP gets to do between MainLoop's 
read and the next state cycle — DP is parked either way.
   
   (The redesign also removes the corresponding init/end-of-body switches in 
`DataProcessor.run` so DP parks at its per-task `finally` between tasks, 
instead of at line 65. Functionally the same parking-once-per-cycle invariant, 
just at a different location.)



-- 
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]

Reply via email to