Skip to content

fix(source-mssql): pin data.query.mode=function to fix CDC hang on large change tables - #84356

Draft
Yarden Carmeli (yardencarmeli) wants to merge 1 commit into
masterfrom
yarden/source-mssql-change-dbz-query-mode
Draft

fix(source-mssql): pin data.query.mode=function to fix CDC hang on large change tables#84356
Yarden Carmeli (yardencarmeli) wants to merge 1 commit into
masterfrom
yarden/source-mssql-change-dbz-query-mode

Conversation

@yardencarmeli

@yardencarmeli Yarden Carmeli (yardencarmeli) commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Fixes:
https://github.com/airbytehq/oncall/issues/13173

What

  • Debezium changed the default value for data.query.mode from function to direct when upgrading to v3.4.3, ref. This mode added LEFT JOIN and ORDER BY that can explain why syncs are slower.

How

Review guide

User Impact

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

@github-actions

Copy link
Copy Markdown
Contributor

👋 Greetings, Airbyte Team Member!

Here are some helpful tips and reminders for your convenience.

💡 Show Tips and Tricks

PR Slash Commands

Airbyte Maintainers (that's you!) can execute the following slash commands on your PR:

  • 🛠️ Quick Fixes
    • /format-fix - Fixes most formatting issues.
    • /bump-version - Bumps connector versions, scraping changelog description from the PR title.
      • Bump types: patch (default), minor, major, major_rc, rc, promote.
      • The rc type is a smart default: applies minor_rc if stable, or bumps the RC number if already RC.
      • The promote type strips the RC suffix to finalize a release.
      • Example: /bump-version type=rc or /bump-version type=minor
    • /bump-progressive-rollout-version - Alias for /bump-version type=rc. Bumps with an RC suffix and enables progressive rollout.
  • ❇️ AI Testing and Review (internal link: AI-SDLC Docs):
    • /ai-prove-fix - Runs prerelease readiness checks, including testing against customer connections.
    • /ai-canary-prerelease - Rolls out prerelease to 5-10 connections for canary testing.
    • /ai-review - AI-powered PR review for connector safety and quality gates.
  • 📝 AI Documentation:
    • /ai-docs-review - AI-powered documentation review for PRs with connector changes.
    • /ai-create-docs-pr - Creates a documentation PR for connector changes, stacked on the current PR.
  • 🚀 Connector Releases:
    • /publish-connectors-prerelease - Publishes pre-release connector builds (tagged as {version}-preview.{git-sha}) for all modified connectors in the PR.
    • /enable-autopilot-rollouts - Enables autopilot progressive rollouts for the modified connector(s) in the PR, remediating "autopilot rollouts not enabled for {connector-name}" auto-merge blockers. Sets defaultRolloutMode: autopilot and enableProgressiveRollout: true, preserving any existing autopilotConfig.
      • Optional args: connector=<CONNECTOR_NAME> (defaults to the modified connectors in the PR), strategy=fast|slow|default (defaults to fast).
      • Example: /enable-autopilot-rollouts or /enable-autopilot-rollouts connector=source-faker strategy=slow
  • ☕️ JVM connectors:
    • /update-connector-cdk-version connector=<CONNECTOR_NAME> - Updates the specified connector to the latest CDK version.
      Example: /update-connector-cdk-version connector=destination-bigquery
  • 🐍 Python connectors:
    • /poe connector source-example lock - Run the Poe lock task on the source-example connector, committing the results back to the branch.
    • /poe source example lock - Alias for /poe connector source-example lock.
    • /poe source example use-cdk-branch my/branch - Pin the source-example CDK reference to the branch name specified.
    • /poe source example use-cdk-latest - Update the source-example CDK dependency to the latest available version.
  • ⚙️ Admin commands:
    • /force-merge reason="<REASON>" - Force merges the PR using admin privileges, bypassing CI checks. Requires a reason.
      Example: /force-merge reason="CI is flaky, tests pass locally"
📚 Show Repo Guidance

Helpful Resources

📝 Edit this welcome message.

@yardencarmeli

Yarden Carmeli (yardencarmeli) commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

/publish-connectors-prerelease connector=source-mssql

Pre-release Connector Publish Started

Publishing pre-release build for connector source-mssql.
PR: #84356

Pre-release versions will be tagged as {version}-preview.f3b88da
and are available for version pinning via the scoped_configuration API.

View workflow run
Pre-release Publish: SUCCESS

Docker image (pre-release):
airbyte/source-mssql:5.0.1-preview.f3b88da

Docker Hub: https://hub.docker.com/layers/airbyte/source-mssql/5.0.1-preview.f3b88da

Registry JSON:

@github-actions

Copy link
Copy Markdown
Contributor

source-mssql Connector Test Results

 13 files   13 suites   1m 14s ⏱️
123 tests 123 ✅ 0 💤 0 ❌
249 runs  249 ✅ 0 💤 0 ❌

Results for commit f3b88da.

@airbyte-support-bot

Copy link
Copy Markdown
Contributor

↪️ Triggering /ai-prove-fix per Hands-Free AI Triage Project triage next step.

Reason: Draft PR, CI checks passing, and no /ai-prove-fix has run yet on this branch — prove-fix is the next pipeline stage before review.

Devin session

@octavia-bot

octavia-bot Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

🔍 AI Prove Fix session starting... Running readiness checks and testing against customer connections. View playbook

Devin AI session created successfully!

@airbyte-support-bot

Airbyte Support Bot (airbyte-support-bot) commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

🟡 No Regression Detected — Fix Not Exercised

Prove-fix validation of source-mssql 5.0.1-preview.f3b88da (this PR, commit f3b88da) against control 5.0.0.

Summary: the change does exactly what it says — the preview provably executes the CDC function query instead of Debezium 3.4's default direct change-table query — and it causes no correctness regression. What could not be shown is that this resolves the reported failure: the originating symptom never reproduced, and the performance rationale behind the change did not survive measurement. Merging is low risk; treating this as a confirmed fix for the incident is not yet justified.

Pre-flight

The diff adds one Debezium property and bumps dockerImageTag to 5.0.1. It does not touch schema types, fields, primary keys, cursors, spec fields, streams, state format, or data scope — non-breaking and reversible by unpinning. Connector tests pass (123 tests / 249 runs, 13 suites). Debezium 3.4 flipped the SQL Server default from function to direct (release notes, DBZ-9394) and documents function as the way to restore the previous behaviour, which matches this PR's intent; source-mssql moved from Debezium 3.3.0.Final to 3.4.3.Final, so the default flip did reach this connector.

1. The property takes effect — proven

Connector logs carry no SQL, so the executed statements were captured server-side with SQL Server Extended Events on a synthetic local database:

Captured CDC streaming statements

Control 5.0.0:

SELECT [__$start_lsn], [__$seqval], [__$operation], [__$update_mask], ...
FROM [CdcTest].cdc.[dbo_users_CT]
WHERE ...
ORDER BY [__$start_lsn] ASC, [__$command_id] ASC, [__$seqval] ASC, [__$operation] ASC

Target 5.0.1-preview.f3b88da:

SELECT [__$start_lsn], [__$seqval], [__$operation], [__$update_mask], ...
FROM [CdcTest].cdc.[fn_cdc_get_all_changes_dbo_users]( @P0, @P1, N'all update old')
WHERE ...
ORDER BY [__$start_lsn] ASC, [__$seqval] ASC, [__$operation] ASC

The target's startup configuration prints data.query.mode = function; the control prints no such line, consistent with Debezium applying its own default.

2. No correctness regression — proven

Drain comparisons (read → extract state → read again until a read emits zero records), same fixtures and same DML for both images:

  • Small fixture (3 baseline deletes, 50 inserts, 25 updates, 10 deletes): both images delivered the complete expected set once the log advanced.
  • Deferred-update fixture (single-transaction update of a UNIQUE-indexed column across 20,000 rows, exactly 40,000 change rows): both images delivered 20,000/20,000 deletes and 20,000/20,000 inserts.

Each experiment ended with a single duplicate delete in the cumulative comparison — on the target side in one run, on the control side in the other — i.e. at-least-once behaviour at a resume boundary, present in both query modes and idempotent at the destination. No records were lost in either mode.

One behaviour worth documenting, affecting both versions identically: a read that reaches its target position mid-transaction stops there (close-reason=RECORD_REACHED_TARGET_POSITION) and the rest of that transaction is not delivered until new activity advances the max LSN. In the deferred fixture one read emitted 53 of 40,000 rows and the following read reported Current position ... equals or exceeds target position and emitted nothing; all 39,947 remaining rows arrived after a single marker insert moved the log on. Alarming-looking, but delayed delivery rather than loss.

3. The performance rationale — refuted at the scale tested

The natural explanation for a hang would be that direct mode must sort a large change set before returning a first row while the function query streams it. Measured against 1.5M change rows (_CT 148 MB):

  • __$command_id is the second key of the CDC change table's clustered index, so the direct query's ORDER BY matches clustered-index order exactly.
  • Direct: clustered index seek, no Sort operator, no memory grant — 3.88s wall, 561ms CPU, 18,948 logical reads.
  • Function: a Sort above a Concatenation, 25,776 KB desired memory — 4.56s wall, 1,426ms CPU, same logical reads plus 15,628 worktable read-ahead reads.
  • Neither connector read hung: first record within 3.3s, completion around 11s, against a 900s timeout.

At this scale the function mode is the one that sorts, and it was slightly slower. That does not rule out different behaviour on a much larger change table with a different schema and index state, but the stated rationale is not supported by anything measurable locally.

The related upstream bug debezium/dbz#2012 — direct mode dropping deferred-update rows when a transaction spans a streaming.fetch.size boundary, because the ORDER BY uses __$command_id while the resume predicate uses only __$seqval — does not apply to this connector as shipped: it sets max.batch.size=2048 / max.queue.size=8192, sets no streaming.fetch.size, and the JDBC fetch size resolves to 148,626,395 rows, so a transaction does not realistically span that boundary. The 40,000-row deferred-update transaction produced no direct-vs-function differential.

Regression testing

Two comparison runs were executed with all four verbs, all streams, debug logs enabled — first with default credentials, then sourcing config/catalog/state from an internal (non-customer) SQL Server CDC connection:

Verb Result
SPEC ✅ identical
CHECK ✅ identical
DISCOVER ✅ catalog unchanged across all 72 streams
READ ❌ failed identically on target and control, both runs

Both READ failures happened before either connector reached a database: the harness serializes the configured catalog with explicit nulls for fields the bulk CDK requires to be non-null (is_file_based, generation_id, sync_id, cursor_field, and others), so both images aborted with ConfigErrorException: Failed to initialize connector operation and zero records. That is a harness defect, not a connector signal — filed as airbytehq/airbyte-ops-mcp#1267. Consequence: CI could not exercise the CDC read path at all, so the regression runs contribute non-breaking evidence only, and the verdict rests on the local proofs above.

Why 🟡 rather than 🟢

The fix's mechanism is confirmed and it is regression-free, but the failure it targets was never reproduced, and the local measurements point the other way on performance. Proving it requires a live run with the preview pinned on an affected connection, comparing time-to-first-record and progress against recent failures — that needs human approval before any pinning and has not been requested. Worth settling first whether the query shape is really the cause: if the stall is platform-side heartbeat handling rather than the change-table query, this property will not address it.

Detailed, connection-specific evidence is in the linked private oncall issue.

@devin-ai-integration devin-ai-integration Bot added the hyd-prove Hydra: ai-prove-fix stage has run label Aug 14, 2026
@airbyte-support-bot

Copy link
Copy Markdown
Contributor

🙋 Escalated per Hands-Free AI Triage Project/ai-prove-fix returned 🟡 No Regression Detected / Fix Not Exercised. It proved the property takes effect (Extended Events captured the target executing fn_cdc_get_all_changes_... where the control queries _CT directly) and proved no correctness regression across two drain comparisons, but the reported failure never reproduced, and the performance rationale did not survive measurement: at 1.5M change rows the direct query is a clustered-index seek with no sort while the function query adds a Sort with a 25 MB memory grant and was slightly slower. Two maintainer decisions are needed: whether to approve pinning the preview on an affected connection to obtain live evidence, and whether the stall is really the change-table query shape rather than platform-side heartbeat handling — this property will not help if it is the latter.


Devin session

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

connectors/source/mssql hyd-prove Hydra: ai-prove-fix stage has run

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants