chore(source-google-ads): Update CDK with heartbeat diagnostics + add HTTP socket timeout#76315
chore(source-google-ads): Update CDK with heartbeat diagnostics + add HTTP socket timeout#76315devin-ai-integration[bot] wants to merge 8 commits intomasterfrom
Conversation
… heartbeat diagnostics and deadlock fix Co-Authored-By: gl_anatolii.yatsuk <gl_anatolii.yatsuk@airbyte.io>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Note 📝 PR Converted to Draft More info...Thank you for creating this PR. As a policy to protect our engineers' time, Airbyte requires all PRs to be created first in draft status. Your PR has been automatically converted to draft status in respect for this policy. As soon as your PR is ready for formal review, you can proceed to convert the PR to "ready for review" status by clicking the "Ready for review" button at the bottom of the PR page. To skip draft status in future PRs, please include |
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksPR Slash CommandsAirbyte Maintainers (that's you!) can execute the following slash commands on your PR:
📚 Show Repo GuidanceHelpful Resources
|
|
Co-Authored-By: gl_anatolii.yatsuk <gl_anatolii.yatsuk@airbyte.io>
|
Deploy preview for airbyte-docs ready!
Deployed with vercel-action |
|
/publish-connectors-prerelease
|
…worker hangs Add TimeoutHTTPAdapter that enforces a 300s socket-level idle timeout on all Google Ads API HTTP calls. This prevents workers from hanging indefinitely on unresponsive API endpoints (e.g., ssl.recv_into() blocking for hours), as observed in sync job 79758185. The timeout is per-recv() call (not total request time), so streaming responses that keep sending data are unaffected. Co-Authored-By: gl_anatolii.yatsuk <gl_anatolii.yatsuk@airbyte.io>
Co-Authored-By: gl_anatolii.yatsuk <gl_anatolii.yatsuk@airbyte.io>
|
/publish-connectors-prerelease
|
Google Ads API is HTTPS-only, so the http:// mount is unnecessary and triggers the 'Connectors must use HTTPS only' pre-release check. Co-Authored-By: gl_anatolii.yatsuk <gl_anatolii.yatsuk@airbyte.io>
Reduce the HTTP socket idle timeout from 5 minutes to 2 minutes so we can exercise the ReadTimeout path on the heartbeat-test connection and verify that the retry/backoff behavior handles it correctly. Co-Authored-By: gl_anatolii.yatsuk <gl_anatolii.yatsuk@airbyte.io>
|
Resolves conflicts in metadata.yaml, pyproject.toml, and google-ads.md by bumping to 4.2.5 (master is at 4.2.4) and merging changelog entries. Co-Authored-By: gl_anatolii.yatsuk <gl_anatolii.yatsuk@airbyte.io>
|
…of failing Co-Authored-By: gl_anatolii.yatsuk <gl_anatolii.yatsuk@airbyte.io>
|
|
Devin is currently unreachable - the session may have died. |
What
Updates
source-google-adsto use CDK pre-release7.13.0.post13.dev23806774478from airbyte-python-cdk#953 to diagnose a recurring sync hang where the Google Ads source goes silent near completion (~98.6% of records read) and is eventually killed by heartbeat timeout.Additionally adds a 5-minute HTTP socket timeout to all Google Ads API calls to prevent workers from hanging indefinitely on unresponsive API endpoints — the root cause identified from thread dump analysis of sync job 79758185 (worker stuck on
ssl.recv_into()for 3.5+ hours).Related: oncall#11852
How
airbyte-cdkdependency from7.9.2→7.13.0.post13.dev23806774478inpyproject.toml4.2.2→4.2.3in bothmetadata.yamlandpyproject.tomlTimeoutHTTPAdapterincomponents.pythat enforces a 300s socket-level idle timeout viarequests.adapters.HTTPAdapterGoogleAdsHttpRequesterandCustomGAQueryHttpRequester(Google Ads API is HTTPS-only)The CDK pre-release from PR #953 includes:
queue_size,queue_full,print_blocked)sys._current_frames()queue.put()inConcurrentMessageRepositorywhen called from the main thread, preventing self-deadlock on a full queueThe timeout is per-
recv()call (socket idle timeout), not a total request timeout. Streaming responses that keep sending data every few seconds are unaffected — only connections where the server goes completely silent for 5+ minutes will time out. Timed-out requests are automatically retried by the CDK sincerequests.ReadTimeoutis inTRANSIENT_EXCEPTIONS.Review guide
components.py—TimeoutHTTPAdapterclass (lines 46–58) and mounting inGoogleAdsHttpRequester.__post_init__(lines 316–319) andCustomGAQueryHttpRequester.__post_init__(lines 864–867)self._http_client._session(private CDK attribute) — fragile if CDK internals changeCustomGAQueryHttpRequesterextendsHttpRequesterdirectly (notGoogleAdsHttpRequester), so timeout is added independently in both placeshttps://—http://was intentionally excluded to satisfy the CI "Connectors must use HTTPS only" check. Google Ads API is HTTPS-only so this is safe.pyproject.toml— CDK version bump (note: this is a pre-release version, for testing only)metadata.yaml— connector version bumpdocs/integrations/sources/google-ads.md— changelog entry (note: PR number in changelog references chore(source-google-ads): Update CDK with deadlock fix + HTTP socket timeout + retry on HTTP 500 #76074, should be updated to this PR's number)poetry.lock— regenerated with Poetry 1.8.5 (lock-version2.0) vs original Poetry 2.0.1 (lock-version2.1). Removesgroupsandmarkersmetadata from package entries — verify this doesn't cause issues with CI or contributors using Poetry 2.x.User Impact
No user-facing behavior change in normal operation. When syncs stall:
This is a testing-only change — the pre-release CDK should not be shipped to production without further validation.
Can this PR be safely reverted and rolled back?
Link to Devin session: https://app.devin.ai/sessions/ad184113df474f0ba37ede09cdac7eaf