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

Use a manual timeout implementation in Analysis to handle I/O gracefully #5263

Merged
merged 3 commits into from
Mar 10, 2025

Conversation

rmosolgo
Copy link
Owner

@rmosolgo rmosolgo commented Mar 6, 2025

Timeout::timeout may interrupt I/O operations leading to messed up connection handles. By implementing the timeout in Ruby instead, we can be sure that I/O operations will have terminated cleanly.

Downsides: a long-running I/O operation will not be killed halfway through. To mitigate this, you'll need timeouts on your client libraries themselves. Also, the timeout is only checked periodically so it's not 100% accurate (runtime may be a tiny bit longer than the timeout).

Also I removed on_fragment_definition. This is because the super(query.selected_operation) line in def initialize means that fragment definitions are never actually visited. I discovered this because I added a check_timeout to the old def on_fragment_definition and I couldn't get a test to fail for it 🤦

part of #5261

@rmosolgo rmosolgo added this to the 2.4.12 milestone Mar 6, 2025
@rmosolgo rmosolgo merged commit 41281d3 into master Mar 10, 2025
14 checks passed
@rmosolgo rmosolgo deleted the manual-analysis-timeout branch March 10, 2025 13:56
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.

1 participant