Skip to content

Conversation

@seongjinyoon
Copy link
Contributor

@seongjinyoon seongjinyoon commented Sep 26, 2025

Description:

Ensure workflow executions are always attributed to the correct workflow even when reusing a computing unit across multiple workflows.

Closes #3759

Problem:

When users switch workflows without changing the computing unit, the frontend keeps the existing WebSocket session so the backend continues to use the previous wid and execution history is midfiled.

In Texera, a WebSocket connection requires wid and cuid. When we switch to a different workflow, we call registerWorkflowMetadataSubscription(). This function calls onComputingUnitChange() when wid changes, but onComputingUnitChange() only checks for changes in cuid without checking wid changes.

Solution:

Have the Angular computing-unit status service track the wid and cuid, forcing the websocket to reconnect whenever either of them change and clear the cached identifiers when the selection disappears or is terminated.
Additionally, fixed typo in core/gui/src/app/workspace/component/power-button/computing-unit-selection.component.ts

Copilot AI review requested due to automatic review settings September 26, 2025 23:15
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a bug where workflow execution history was incorrectly attributed to the previous workflow when users switched workflows without changing computing units. The websocket connection was not being reopened when only the workflow ID changed.

  • Added tracking of workflow ID (wid) in addition to computing unit ID (cuid) to detect when websocket reconnection is needed
  • Modified the reconnection logic to trigger when either wid or cuid changes
  • Added cleanup of cached identifiers when computing units are deselected or terminated

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@seongjinyoon
Copy link
Contributor Author

@aglinxinyuan @kunwp1

@aglinxinyuan aglinxinyuan requested review from bobthomson70 and removed request for bobthomson70 September 27, 2025 00:04
@chenlica
Copy link
Contributor

@seongjinyoon Thanks for fixing the issue. Can you recommend 1-2 reviewers?

@seongjinyoon
Copy link
Contributor Author

@seongjinyoon Thanks for fixing the issue. Can you recommend 1-2 reviewers?

I have asked @kunwp1 to review the code.

@chenlica
Copy link
Contributor

I also assigned @Xiao-zhen-Liu as a reviewer.

Copy link
Contributor

@kunwp1 kunwp1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed the PR and left a few clarification questions. If you can address them, please add comments to the code. Also, can you update the comments of the registerWorkflowMetadataSubscription and the PR description to replace onComputingUnitChange with selectComputingUnit?

@kunwp1
Copy link
Contributor

kunwp1 commented Sep 30, 2025

It looks good to me now. Well done!

@Xiao-zhen-Liu Can you review the PR?

Copy link
Contributor

@Xiao-zhen-Liu Xiao-zhen-Liu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@Xiao-zhen-Liu Xiao-zhen-Liu merged commit f406e25 into apache:main Sep 30, 2025
10 checks passed
@seongjinyoon seongjinyoon deleted the fix/wid branch September 30, 2025 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Execution History not shown sometimes when a workflow is ran.

4 participants