Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lift SyncFlowWorkflow into activity entirely #2371

Merged
merged 4 commits into from
Dec 24, 2024
Merged

Conversation

serprex
Copy link
Contributor

@serprex serprex commented Dec 17, 2024

No description provided.

flow/activities/flowable.go Outdated Show resolved Hide resolved
@@ -554,12 +546,6 @@ func CDCFlowWorkflow(

if shared.ShouldWorkflowContinueAsNew(ctx) {
restart = true
if syncFlowFuture != nil {
if err := model.SyncStopSignal.SignalChildWorkflow(ctx, syncFlowFuture, struct{}{}).Get(ctx, nil); err != nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have to decide whether cdc_flow restarts hard pause sync flow, otherwise have to setup a whole session state thing so workflow can signal activity to shutdown

@serprex serprex force-pushed the sync-flow-activity branch 6 times, most recently from 249fc44 to deac49b Compare December 19, 2024 17:41
@serprex serprex marked this pull request as ready for review December 19, 2024 17:42
@serprex serprex force-pushed the sync-flow-activity branch 2 times, most recently from b5b8c97 to cdb3f8f Compare December 20, 2024 18:13
@serprex
Copy link
Contributor Author

serprex commented Dec 20, 2024

tested StartToCloseTimeout being hit (1 year in this PR). Result is activity retries. So that's fine

@serprex serprex force-pushed the sync-flow-activity branch 3 times, most recently from d4f9da2 to a16e168 Compare December 23, 2024 17:45
serprex added a commit that referenced this pull request Dec 23, 2024
important once #2371 prevents seeing batch history in temporal
@serprex serprex force-pushed the sync-flow-activity branch 2 times, most recently from d22fc5d to 75c035a Compare December 24, 2024 14:31
@heavycrystal
Copy link
Contributor

code looks fine, but need extensive testing due to niche cases
set up a long running mirror on a test instance for instance

@serprex serprex force-pushed the sync-flow-activity branch 2 times, most recently from 467fd28 to 85dbc04 Compare December 24, 2024 20:27
@serprex serprex merged commit f388a3f into main Dec 24, 2024
9 checks passed
@serprex serprex deleted the sync-flow-activity branch December 24, 2024 20:35
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.

2 participants