Skip to content

feat: nightly connector switch sweep with watermark-rewind handover - #5049

Merged
mbani01 merged 3 commits into
mainfrom
feat/issue-833-switch-sweep
Oct 20, 2026
Merged

mbani01 merged 3 commits into
mainfrom
feat/issue-833-switch-sweep

Conversation

@mbani01

@mbani01 mbani01 commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

Refs linuxfoundation/lfx-cdp-insights-ops#833

What

Nightly sweep. A Temporal schedule connectors-switch-sync-pipeline-sweep (02:00Z, overlap SKIP, catch-up 1h) starts switchSyncPipelineSweep, which lists every repo that still has a non-emitting, active, incremental GitHub shadow unit and runs switchSyncPipeline forward for it as a child workflow, one repo at a time. Each child gets only that repo's unswitched syncs, so already-switched units are not re-paused or touched again. The child uses the same per-repo workflow id as the pod script, so a manual switch and the sweep can never interleave: a repo whose switch is already running is counted as skipped. ParentClosePolicy.ABANDON lets a running switch finish even if the sweep is terminated. No cap: every sync that passes the clean gate is switched. Pausing the schedule in Temporal is the off switch.

Handover by watermark rewind instead of shadow replay. The forward switch no longer replays a unit's shadow rows through the results path. It reads the Nango sync's last run (finishedAt from the connection status), pauses Nango, and in one transaction flips emitEnabled, rewinds the unit's watermark to since = finishedAt - 1h with cursor = null and no covered windows (exactly what a completed run leaves behind), and deletes every shadow row of the unit. The reactivated unit then re-fetches from GitHub everything updated since Nango's last run, edits included, and the sink dedupes the overlap. Forks are cursor-walked in creation order and have no handoff gap, so their watermark is kept. A unit that is already emitting (rerun) only gets its shadow rows cleared. A Nango sync with no completed run is refused.

Why

The replay walked every remaining shadow row of a unit. Prod holds 2.6M shadow rows (7.9 GB), 2.25M of them older than five days, because the nightly diff only prunes matched rows whose event time falls inside the diffed day; re-captured old records never do. A fleet-wide sweep would have re-emitted all of them on night one and overwritten fresher Nango-delivered edits with stale captures. Rewinding since closes the same handoff gap with no replay code, and deleting the rows at switch time empties the table as units migrate.

Tests

  • Pure: sweep grouping of units into repos, handoverSince (1h overlap, forks exempt).
  • Workflow: sweep tallies switched/refused/unchanged/skipped/failed across children; forward switch order gate → quiesce → drain → re-gate → last run → pause → take over → activate, with the never-ran refusal and the heal path.
  • Activities: last-run read from the connection status, take-over rewinds or keeps the cursor walk.
  • DAL (CI DB): takeOverSyncUnit flips, rewinds a since walk, keeps a cursor walk, deletes the unit's shadow rows, and is a no-op on an emitting unit.

Deploy

connectors-worker only. No migration.

Copilot AI balanced review requested due to automatic review settings October 20, 2026 10:27
@cursor

cursor Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

PR Summary

High Risk
Changes production sync cutover semantics and can rewind watermarks, pause/resume Nango, and bulk-delete shadow data across many repos on a schedule.

Overview
Replaces shadow-record replay on forward Nango→connector switches with a watermark rewind handover: read Nango connection status and CDP consumption to pick a handover time, pause Nango, then takeOverSyncUnit enables emit, rewinds since by one hour (cursor walks keep their watermark), and deletes all shadow rows in one transaction. Failures after pause resume Nango and reactivate the unit; already-emitting units only get shadow cleanup.

Adds a nightly Temporal sweep (connectors-switch-sync-pipeline-sweep, 02:00Z) that lists shadow-diff repos, groups unswitched syncs per repo, and runs switchSyncPipeline forward serially as child workflows with the same per-repo workflow id as the CLI (skip if already running). Removes unused shadow batch list/delete DAL helpers tied to replay.

Reviewed by Cursor Bugbot for commit 304f85f. Bugbot is set up for automated code reviews on this repo. Configure here.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Handover data preservation, failure recovery, and compatibility with in-flight Temporal workflows remain unresolved.

5 open issues
What changed in this PR

Automates nightly migration from Nango to connectors, replacing shadow-record replay with watermark-based refetching.

Changes:

  • Adds a scheduled, sequential sweep with shared per-repository workflow IDs.
  • Introduces transactional takeover that enables emission, rewinds watermarks, and deletes shadow records.
  • Updates workflow, activity, helper, and database tests.
File Description
services/​libs/​data-access-layer/​src/​connectors/​syncUnits.ts Adds transactional takeover.
services/​libs/​data-access-layer/​src/​connectors/​syncUnits.test.ts Tests takeover and cleanup.
services/​libs/​data-access-layer/​src/​connectors/​shadowRecords.ts Removes replay-specific helpers.
services/​apps/​connectors_worker/​src/​workflows/​switchSyncPipelineSweep.ts Adds sequential repository sweep.
services/​apps/​connectors_worker/​src/​workflows/​switchSyncPipelineSweep.test.ts Tests sweep outcomes and conflicts.
services/​apps/​connectors_worker/​src/​workflows/​switchSyncPipeline.ts Replaces replay with watermark takeover.
services/​apps/​connectors_worker/​src/​workflows/​switchSyncPipeline.test.ts Updates handover and recovery tests.
services/​apps/​connectors_worker/​src/​workflows.ts Exports the sweep workflow.
services/​apps/​connectors_worker/​src/​syncPipelineSwitch.ts Adds grouping and rewind helpers.
services/​apps/​connectors_worker/​src/​syncPipelineSwitch.test.ts Tests grouping and rewind rules.
services/​apps/​connectors_worker/​src/​schedules/​switchSyncPipelineSweep.ts Defines the nightly schedule.
services/​apps/​connectors_worker/​src/​main.ts Registers the schedule.
services/​apps/​connectors_worker/​src/​bin/​switch-sync-pipeline.ts Reuses the shared workflow ID.
services/​apps/​connectors_worker/​src/​activities/​switchSyncPipelineActivities.ts Adds status, takeover, and sweep activities.
services/​apps/​connectors_worker/​src/​activities/​switchSyncPipelineActivities.test.ts Tests status reads and takeover.
services/​apps/​connectors_worker/​src/​activities.ts Updates activity registration.

Review effort: Balanced

Comment thread services/apps/connectors_worker/src/activities/switchSyncPipelineActivities.ts Outdated
Comment thread services/apps/connectors_worker/src/syncPipelineSwitch.ts Outdated
Comment thread services/apps/connectors_worker/src/workflows/switchSyncPipeline.ts Outdated
Comment thread services/apps/connectors_worker/src/workflows/switchSyncPipeline.ts Outdated
Comment thread services/apps/connectors_worker/src/workflows/switchSyncPipeline.ts

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

Comment thread services/apps/connectors_worker/src/workflows/switchSyncPipeline.ts Outdated
Copilot AI balanced review requested due to automatic review settings October 20, 2026 10:38

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 304f85f. Configure here.

Comment thread services/apps/connectors_worker/src/syncPipelineSwitch.ts

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Exhausted activation retries after takeover can leave both pipelines paused without recovery by subsequent sweeps.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Medium severity Recover connectors left paused after failed activation takeover

services/​apps/​connectors_worker/​src/​workflows/​switchSyncPipeline.ts:140

If activation fails on all three attempts before updating the unit, takeover has already committed and Nango is paused, but the connector remains paused too. The sweep records a failed child and moves on. Later sweeps cannot heal it because listShadowDiffUnits selects only active, non-emitting units (services/libs/data-access-layer/src/connectors/syncUnits.ts:382–383). Ingestion therefore remains stopped after the outage ends unless someone intervenes. Use durable activation retries after takeover, as below, or explicitly track interrupted handovers for recovery.

5 resolved since last review

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 20, 2026 11:54
@mbani01
mbani01 merged commit 947fe0b into main Oct 20, 2026
12 checks passed
@mbani01
mbani01 deleted the feat/issue-833-switch-sweep branch October 20, 2026 11:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Candidate identity and incomplete-cutover recovery remain unresolved, risking missed migrations and interrupted ingestion.

3 open issues

Review effort: Balanced

if (existing) {
existing.syncNames.push(unit.syncName)
} else {
byRepo.set(key, { ...parseRepoChannel(unit.channelName), syncNames: [unit.syncName] })
Comment on lines +156 to 159
await activity.resumeNangoRepoSync({
connectionId: target.connectionId,
nangoSync: sync.nangoSync,
})
Comment on lines +64 to +65
log.error('sweep child switch failed', { ...repo, err })
return 'failed'
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.

3 participants