Fix pipelines get stuck in a paused state #5223
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,3 @@ | ||
| import type { NamesInUnion } from '$lib/functions/common/union' | ||
| import type { | ||
| ExtendedPipeline, | ||
|
|
@@ -13,6 +11,13 @@ import { useReactiveWaiter } from './useReactiveWaiter.svelte' | |
| import { unionName } from '$lib/functions/common/union' | ||
| import { page } from '$app/state' | ||
| import { match } from 'ts-pattern' | ||
| import { hasSelectedRelations } from '$lib/components/pipelines/editor/TabChangeStream.svelte' | ||
|
|
||
| let postPipelineAction: (pipeline_name: string, action: PipelineAction, callbacks?: { | ||
| onPausedReady?: (pipelineName: string) => Promise<void>; | ||
| }) => Promise<{ | ||
| waitFor: () => Promise<boolean>; | ||
|
Comment on lines
+16
to
+19
There was a problem hiding this comment. The module-level |
||
| }> = undefined! | ||
|
|
||
| /** | ||
| * Composition for handling pipeline actions with optimistic updates and state management. | ||
|
|
@@ -64,8 +69,9 @@ export const usePipelineAction = () => { | |
| 'Replaying' | ||
| ] | ||
| const reactiveWaiter = useReactiveWaiter(() => pipelineList.pipelines) | ||
| // We initialize a global state that will be reused across all calls to `getPipelineAction`. | ||
| // This allows keeping the same waiters alive while switching UI context | ||
|
Contributor
There was a problem hiding this comment. I don't know who the waiters are or what switching UI context means, but perhaps I have missed too many reviews of this code to understand |
||
| postPipelineAction ??= async ( | ||
| pipeline_name: string, | ||
| action: PipelineAction | 'resume', | ||
| callbacks?: { | ||
|
|
@@ -95,58 +101,67 @@ export const usePipelineAction = () => { | |
|
|
||
| // Handle 'start' action with hidden paused intermediate state | ||
| if (action === 'start') { | ||
| // Optimization: Skip pause-unpause sequence if no relations are selected in Change Stream tab | ||
| const needsPauseUnpause = hasSelectedRelations(pipeline_name) | ||
|
|
||
| if (needsPauseUnpause) { | ||
| // Relations are selected, use pause-unpause sequence to allow callbacks to run | ||
| // First start in paused state | ||
| await api.postPipelineAction(pipeline_name, 'start_paused') | ||
|
|
||
| // Wait for paused state and run callbacks | ||
| const pausedWaiter = reactiveWaiter.createWaiter({ | ||
| predicate: (ps) => { | ||
| const p = ps.find((p) => p.name === pipeline_name) | ||
| if (!p) { | ||
| throw new Error('Pipeline not found in pipelines list') | ||
|
Contributor
There was a problem hiding this comment. is this the right thing to do? |
||
| } | ||
| if (ignoreStatuses.includes(unionName(p.status))) { | ||
| return null | ||
| } | ||
| if ( | ||
| (['Paused', 'AwaitingApproval'] satisfies PipelineStatus[]).findIndex( | ||
|
Contributor
There was a problem hiding this comment. you really like to write code that is hard to understand |
||
| (status) => status === p.status | ||
| ) !== -1 | ||
| ) { | ||
| return { value: true } | ||
| } | ||
| if (p.status === 'Stopped') { | ||
| return { value: false } | ||
| } | ||
| throw new Error( | ||
|
Contributor
There was a problem hiding this comment. so if any of these errors happens no callbacks are run. is this what you expect? |
||
| `Unexpected status ${JSON.stringify(p.status)} while waiting for pipeline ${pipeline_name} to reach paused state` | ||
| ) | ||
| } | ||
| }) | ||
|
|
||
| try { | ||
| const shouldContinue = await pausedWaiter.waitFor() | ||
| if (!shouldContinue) { | ||
| return { | ||
| waitFor: async () => { | ||
| // Pipeline was stopped before it could be resumed, listeners should not wait for running state | ||
| return false | ||
| } | ||
| } | ||
| } | ||
| } catch (e) { | ||
| return { | ||
| waitFor: async () => { | ||
| throw e | ||
| } | ||
| } | ||
| } | ||
|
|
||
| updatePipeline(pipeline_name, (p) => ({ ...p, status: 'Initializing' })) | ||
| await callbacks?.onPausedReady?.(pipeline_name) | ||
|
|
||
| // Then start normally | ||
| await api.postPipelineAction(pipeline_name, 'resume') | ||
| } else { | ||
| // No relations selected, start directly without pausing first | ||
| await api.postPipelineAction(pipeline_name, 'start') | ||
| } | ||
| } else { | ||
| await api.postPipelineAction(pipeline_name, action) | ||
| } | ||
|
|
@@ -205,5 +220,10 @@ export const usePipelineAction = () => { | |
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| export const getPipelineAction = () => { | ||
| return { | ||
| postPipelineAction | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
updatePipelinefunction was removed from destructuring but may still be needed elsewhere in the file. Verify that removing this doesn't break any functionality, as the original code imported bothupdatePipelinesandupdatePipeline.