Skip to content

fix(destination-mssql): reduce memory usage to prevent OOM during large syncs#76114

Draft
devin-ai-integration[bot] wants to merge 3 commits intomasterfrom
devin/1775524470-fix-destination-mssql-oom
Draft

fix(destination-mssql): reduce memory usage to prevent OOM during large syncs#76114
devin-ai-integration[bot] wants to merge 3 commits intomasterfrom
devin/1775524470-fix-destination-mssql-oom

Conversation

@devin-ai-integration
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot commented Apr 7, 2026

What

Resolves https://github.com/airbytehq/oncall/issues/11899:

Original issue: #76113

The destination-mssql JVM process crashes with OOM during large syncs. The -XX:+ExitOnOutOfMemoryError flag causes immediate termination without graceful shutdown, surfacing as "Broken pipe" and "No exit code" errors.

How

Reduced three memory-related configuration defaults in MSSQLConfiguration.kt:

Parameter Before After Effect
recordBatchSizeBytes 10 MB 5 MB Halves max data accumulated per loader
batchEveryNRecords 5,000 2,500 Flushes JDBC batches twice as often
maxNumOpenLoaders 8 4 Halves concurrent loaders (each holds a JDBC connection + PreparedStatement + batch buffer)

Peak theoretical loader memory drops from approximately 80 MB (8 x 10 MB) to approximately 20 MB (4 x 5 MB). The synchronized(parent) block in MSSQLDirectLoader.executeBatchSafely() means only one loader flushes at a time — with fewer loaders, less data accumulates while waiting.

Also removed the now-unused ObjectStorageUploadConfiguration import (the 10 MB default came from there; the new value is an inline literal).

Review guide

  1. MSSQLConfiguration.kt — the actual fix (3 constant changes + removed import)
  2. MSSQLConfigurationTest.kt — new unit tests asserting the tuned defaults
  3. metadata.yaml — version bump 2.2.15 → 2.2.16
  4. docs/integrations/destinations/mssql.md — changelog entry

Key things to verify

  • Are the new values too aggressive? This halves throughput capacity in exchange for OOM stability. Prior art: airbytehq/airbyte#55252 previously fixed an OOM by correcting recordBatchSizeBytes.
  • Should any of these be user-configurable via the spec rather than hardcoded?
  • The synchronized(parent) serialization bottleneck in MSSQLDirectLoader.kt:67 is unchanged — with 4 loaders instead of 8, contention is lower, but flush throughput is also lower.
  • Unit tests only verify the new default values are set correctly. There is no integration-level validation that the fix prevents OOM under load — this would require a live MSSQL instance with a large dataset.

User Impact

Syncs that previously crashed with OOM / "Broken pipe" / "No exit code" errors on large datasets should now complete successfully. Throughput may decrease for very high-volume syncs due to the lower concurrency and smaller batch sizes.

Can this PR be safely reverted and rolled back?

  • YES 💚

Link to Devin session: https://app.devin.ai/sessions/04c46da808d046528937552da2b5a9c0

…ge syncs

Lower maxNumOpenLoaders from 8 to 4, batchEveryNRecords from 5000 to 2500,
and recordBatchSizeBytes from 10MB to 5MB. These changes reduce the peak
theoretical memory footprint of concurrent loaders from ~80MB to ~20MB,
preventing OOM crashes during large syncs.

Co-Authored-By: bot_apk <apk@cognition.ai>
@devin-ai-integration
Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Co-Authored-By: bot_apk <apk@cognition.ai>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

👋 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.
  • 🚀 Connector Releases:
    • /publish-connectors-prerelease - Publishes pre-release connector builds (tagged as {version}-preview.{git-sha}) for all modified connectors in the PR.
  • ☕️ 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.

Co-Authored-By: bot_apk <apk@cognition.ai>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

Deploy preview for airbyte-docs ready!

✅ Preview
https://airbyte-docs-hcuf2nw5p-airbyte-growth.vercel.app

Built with commit 1729d78.
This pull request is being automatically deployed with vercel-action

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

destination-mssql Connector Test Results

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit 1729d78.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant