Skip to content

Fallback to sock_state_cb if event_thread creation fails#151

Merged
bdraco merged 17 commits intomasterfrom
resolver_fallback
May 8, 2025
Merged

Fallback to sock_state_cb if event_thread creation fails#151
bdraco merged 17 commits intomasterfrom
resolver_fallback

Conversation

@bdraco
Copy link
Member

@bdraco bdraco commented May 8, 2025

What do these changes do?

On some systems (see #150), the creation of the event_thread can fail when the system runs out of available inotify watches; often because other processes have consumed them all.
This change adds a fallback to sock_state_cb in such scenarios, allowing socket state handling to continue gracefully even when inotify resources are exhausted.

Are there changes in behavior for the user?

These changes should be mostly transparent to the user. On Windows, the fallback is unlikely to be triggered since we we can't run out of inotify handles.

The main downside is that updates to the resolver configuration won’t be picked up automatically.

Related issue number

close #150

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes

bdraco added 3 commits May 8, 2025 11:59
In #150 we see systems that are out of inotify watches because
something else on the system is using them all up. Fallback to
sock_state_cb when this happens
In #150 we see systems that are out of inotify watches because
something else on the system is using them all up. Fallback to
sock_state_cb when this happens
In #150 we see systems that are out of inotify watches because
something else on the system is using them all up. Fallback to
sock_state_cb when this happens
@codecov
Copy link

codecov bot commented May 8, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.25%. Comparing base (5c4b29c) to head (2457e40).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #151      +/-   ##
==========================================
+ Coverage   92.55%   94.25%   +1.69%     
==========================================
  Files           3        3              
  Lines         336      400      +64     
  Branches       23       29       +6     
==========================================
+ Hits          311      377      +66     
+ Misses         15       14       -1     
+ Partials       10        9       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@saghul
Copy link
Contributor

saghul commented May 8, 2025

Neat idea!

@bdraco bdraco marked this pull request as ready for review May 8, 2025 19:00
@bdraco bdraco requested a review from Copilot May 8, 2025 19:00
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a fallback mechanism in the DNSResolver to use socket state callbacks when creating an event thread fails due to exhausted inotify watches, while also ensuring Windows-specific requirements are met.

  • Fallback logic is added in _make_channel() to gracefully handle pycares.AresError exceptions on Linux.
  • New tests are introduced to verify behavior across platforms and various event loop scenarios.
  • pytest.ini is updated to adjust asyncio default settings.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
tests/test_aiodns.py Added test cases to verify fallback behavior and proper warnings for errors.
pytest.ini Updated asyncio configuration options for the test suite.
aiodns/init.py Refactored _make_channel() to add fallback logic and integrated logging support.
Comments suppressed due to low confidence (1)

aiodns/init.py:107

  • [nitpick] The timeout adjustment logic in _start_timer() is a bit opaque; adding inline comments to explain the conditions and rationale for adjusting the timeout value would improve code clarity.
def _start_timer(self):

@bdraco bdraco closed this May 8, 2025
@bdraco bdraco reopened this May 8, 2025
@bdraco
Copy link
Member Author

bdraco commented May 8, 2025

github is being flakey, close reopen to restart ci

@bdraco
Copy link
Member Author

bdraco commented May 8, 2025

I'm going to format and clean up the CI a bit after this and than do a release since #150 seems rather urgent to get a fix out

@bdraco bdraco merged commit c856d44 into master May 8, 2025
15 checks passed
@bdraco bdraco deleted the resolver_fallback branch May 8, 2025 19:25
@bdraco bdraco mentioned this pull request May 8, 2025
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.

Resolver cannot be created if inotify is broken or out of file descriptors

3 participants