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

estuary-cdk: additional SIGQUIT logging & patching requests.Session.send for Airbyte imports #2380

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Alex-Bair
Copy link
Member

@Alex-Bair Alex-Bair commented Feb 11, 2025

Description:

There have been tricky, intermittent cases of imported Airbyte connectors hanging with essentially no network traffic. I've added some additional logging when a SIGQUIT is injected & in the CaptureShim's methods to help debug when these imports hang.

I also have a theory that since Airbyte imports use the requests package to make synchronous HTTP requests, the requests package doesn't have any default timeouts, and I can't find anywhere in the airbyte_cdk where a timeout is specified for requests, that setting a timeout for requests might prevent Airbyte imports from hanging until the connector is restarted after 24 hours. I've made requests_session_send_patch.py to patch in a default timeout, and imported it in all Airbyte imports. I'm not certain the missing timeouts are the problem, but it's currently the best, actionable theory I have.

We've only observed source-zendesk-support and source-jira-native hanging like this so far, but I chose to patch requests.Session.send for all currently imported connectors to avoid playing whack-a-mole with imports that we haven't seen have this issue yet. If any imports still hang for DEFAULT_TIMEOUT seconds, then we'll know that the issue isn't due to a mising requests timeout.

Documentation links affected:

(list any documentation links that you created, or existing ones that you've identified as needing updates, along with a brief description)

Notes for reviewers:

Confirmed Airbyte imports can still make HTTP requests & receive responses. Confirmed that using requests to send an HTTP request to a server that accepts the request but doesn't send data back will timeout after DEFAULT_TIMEOUT seconds.


This change is Reviewable

…connectors

There have been instances where imported Airbyte connectors just hang
without any apparent network traffic. Debugging this has been difficult,
so this commit adds some additional logging to help troubleshoot what
coroutines exist when a SIGQUIT is injected.
The `requests` package does not have a default timeout for its `send`
method. I did not see any code in version 0.52.10 of the `airbyte_cdk`
(the version almost all of the imported connectors use) that passes a
timeout argument, so I'm patching the `send` method to use a default
timeout if one isn't provided. This is to help troubleshoot & hopefully
avoid imported Airbyte connectors that infrequently hang without any
network traffic.
@Alex-Bair Alex-Bair force-pushed the bair/cdk-sigquit-logging-and-requests-patch branch from b03b02c to 4e17524 Compare February 13, 2025 21:26
@Alex-Bair Alex-Bair marked this pull request as ready for review February 13, 2025 21:39
Copy link
Member

@williamhbaker williamhbaker left a comment

Choose a reason for hiding this comment

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

LGTM

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