eugenegujing opened a new pull request, #8936: URL: https://github.com/apache/texera/pull/8936
### What changes were proposed in this PR? **Root cause.** The frontend learns that a run has ended only from the engine, over the selected unit's websocket. When the selected unit is terminated from the unit dropdown, or drops out of the unit list, that message can never arrive and nothing resets the execution state, so the run keeps being reported as running. On top of that, the Kill button's gate exempted `NoComputingUnit` from its socket check, so Kill stayed enabled although its request could no longer be delivered. #3400 (a910bcfd5) added the socket check to Kill together with that exemption, copying the Run button's translation of the old single-mode flag `!environment.computingUnitManagerEnabled`; that mode no longer exists. ``` Before: unit terminated mid-run -> no end-of-run message -> state stays Running: clock counts, Form View says "Running", Kill enabled but dead After: unit terminated mid-run -> selection drops to none -> run reset: clock stopped, Form View idle, Kill disabled; console and results kept ``` Both screenshots: a run was in flight on "Analytics Unit", which was then terminated from the unit dropdown, and Kill was clicked. | Before | After | | --- | --- | | <img width="1512" height="805" alt="kill-button-2-after-terminate-kill-still-enabled" src="https://github.com/user-attachments/assets/e0919b87-0c2b-4138-a30b-12295b749175"; /> | <img width="1456" height="829" alt="kill-button-4-after-fix-terminated-kill-disabled-canvas-relocked" src="https://github.com/user-attachments/assets/729eb0df-d01a-4e4c-90e4-e2c4ac684398"; /> | | Kill is enabled and does nothing; the clock keeps counting. | Kill is greyed out; the clock is back at 0:00:00. The info message comes from the separate check described under "Scope boundary". | Form View after the same steps no longer reports a run: <img width="1456" height="829" alt="kill-button-6-after-fix-form-view-not-running" src="https://github.com/user-attachments/assets/bd68cdd3-161c-4479-a2c2-889a74b5d2d8"; /> **Changes.** 1. `ExecuteWorkflowService` calls its existing `resetExecutionAndWorkers()` when the selected unit drops to none while an execution is in flight. That puts the execution back to Uninitialized, which Form View and the canvas lock already follow, clears the worker ids and stops the clock. It is the narrower of the two existing resets: console messages and results stay, unlike the `connectionReset` path a manual unit switch takes. A direct switch from one unit to another never passes through none, so it does not trigger. 2. The Kill button is disabled whenever the websocket is down, without the `NoComputingUnit` exemption. This is defense in depth: with change 1, a run that is still in flight with no unit and no socket should not occur any more, and if it does, Kill no longer looks usable. **Scope boundary.** The canvas can be locked again right away by a second, independent check, which this PR leaves unchanged. Whenever the selected unit changes, including when it drops to none, `ComputingUnitSelectionComponent.updateWorkflowModificationStatus` asks the database for Running or Initializing executions of the workflow. If it finds any, it disables modification and shows "There are ongoing executions on this workflow. Modification of the workflow is currently disabled." At that moment the lost unit's execution row is still RUNNING: | Unit type | Why the row is RUNNING | Effect | | --- | --- | --- | | Local | Disconnect does not stop the shared engine, which keeps running the execution. | Accurate. The check passes again on the first reload or unit selection after that execution finishes. | | Kubernetes | The terminate endpoint deletes the pod but never updates the execution row. | The workflow is locked on every later load. That needs a backend fix, which is outside this PR. | Also not changed here: on a dropdown terminate, the existing `clearStatus()` call in `ComputingUnitStatusService.terminateComputingUnit` resets the operators' colors, while their port row counts keep the last values. When the unit leaves the list any other way, for example a Kubernetes eviction or a terminate from another tab, nothing resets the colors, so the operators keep their last state after the reset. **Side effect of change 1.** It should rarely matter in practice: this tab can reset a run that is in fact still going, but only when all of these hold: - the unit is a local one; terminating a Kubernetes unit deletes its pod, so there the run really has ended; - this tab has a run in flight on that unit; - the same unit is disconnected from another tab or page before the run ends; - this tab then refreshes its unit list before the run ends, which in the workspace happens on opening the unit dropdown, renaming or terminating a unit, or a websocket status change, not on a timer. The run itself is not affected: the reset only changes this tab's state and sends nothing to the engine, so the shared engine keeps running the execution and finishes it normally. This tab just stops showing it as running and disables Kill, whereas on main Kill still worked over this tab's open socket. Requiring the socket to be down as well would avoid this, but would make the reset depend on #8855, which makes a clean close mark the socket disconnected. ### Any related issues, documentation, discussions? Closes #8934 Related: - #8618: a stuck non-terminal execution row keeps its unit off the idle sweep. - #8853, fixed by #8855: a clean websocket close never marks the socket disconnected; the Kill gate relies on `isConnected` being accurate. ### How was this PR tested? New tests in `frontend/src/app/workspace/service/execute-workflow/execute-workflow.service.spec.ts`: - ends a running execution when the selection empties - ends a paused execution when the selection empties - ends a later run as well when its unit is lost too - stops the clock when the selection empties - ends a run that is Initializing, Running, Pausing, Paused, Resuming or Recovering when the selection empties, one case per state - leaves a run that is Uninitialized, Completed, Terminated, Failed or Killed untouched when the selection empties, one case per state - leaves a finished execution alone when the selection empties - keeps the error messages of a failed run when the selection empties - does nothing when no execution has started - keeps the run when switching straight from one unit to another - keeps the run when a unit is first selected - keeps the run when the selected unit reports Pending, Failed, Unknown or Terminating, one case per status New tests in `frontend/src/app/workspace/component/menu/menu.component.spec.ts`, for the Kill button: - is disabled while the socket is down and no unit is selected - stays enabled while the socket is up and no unit is selected - is disabled while the socket is down on a running unit - stays enabled while the socket is up on a running unit - is disabled once the execution is over, with the socket connected or disconnected, one case each The shared `beforeEach` of the service spec now installs a `BehaviorSubject` as the selected-unit stream before the service is constructed, since the service subscribes in its constructor. Form View needs no test of its own: it reads `isRunning` from the execution state stream that `resetExecutionAndWorkers()` already broadcasts. `ExecuteWorkflowService` does not inject the console or result services, so this change cannot clear them. ``` cd frontend NODE_OPTIONS="--no-experimental-webstorage" npx ng test --watch=false --include src/app/workspace/service/execute-workflow/execute-workflow.service.spec.ts # 81 passed NODE_OPTIONS="--no-experimental-webstorage" npx ng test --watch=false --include src/app/workspace/component/menu/menu.component.spec.ts # 141 passed NODE_OPTIONS="--no-experimental-webstorage" npx ng test --watch=false # 233 files, 6607 passed | 1 skipped npx tsc --noEmit -p tsconfig.json # clean npx eslint <changed files> # clean npx prettier --check <changed files> # clean ``` The `NODE_OPTIONS` setting is only needed locally on Node 25, whose built-in `localStorage` shadows jsdom's. CI runs Node 24 with `yarn --cwd frontend run test:ci` and needs no flag. Mutation check: each of these makes at least one new test fail: restoring the `NoComputingUnit` exemption, resetting regardless of the execution state, treating Paused as not in flight, removing the subscription, using `resetExecutionState()` instead of `resetExecutionAndWorkers()`, treating Completed, Terminated or Failed as in flight, treating Initializing or Recovering as settled, resetting when the selected unit only reports a non-Running status, letting only the first lost unit reset, and a reset that forgets to clear the worker ids. Manual check in Chrome, 1512x861 viewport, against a build of this branch on the local-dev stack with local computing units: 1. Workflow: Text Input -> Python UDF whose `process_tuple` does `time.sleep(900)`. 2. Select a unit and a warehouse, click Run, and wait until the UDF operator is running. 3. Open the unit dropdown, click the trash icon of the selected unit, confirm "Disconnect", then click Kill. 4. Before: Kill stays enabled and does nothing; the clock keeps counting; Form View keeps saying "Running"; operators cannot be moved. After: the execution is back to Uninitialized, the clock reads 0:00:00, Kill is disabled, and Form View no longer reports a run; console and results stay. The canvas is locked again right away with the info message quoted under "Scope boundary", because the shared local engine is still running the execution. ### Was this PR authored or co-authored using generative AI tooling? Yes. Generated-by: Claude Code (claude-fable-5-1, claude-opus-5-5). Co-authored with Claude; the author reviewed the change before submission. -- 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]
