Skip to content

feat: make ServiceInfo aware of question history#1348

Merged
bdraco merged 25 commits into
masterfrom
info_question_history
Dec 16, 2023
Merged

feat: make ServiceInfo aware of question history#1348
bdraco merged 25 commits into
masterfrom
info_question_history

Conversation

@bdraco

@bdraco bdraco commented Dec 16, 2023

Copy link
Copy Markdown
Member

If we have multiple consumers of service info asking the same question over and over they all get asked even though we only need to ask once. This can generate excessive amount of network traffic.

There was also a problem with ServiceInfo.async_request would ask questions that would get suppressed by the duplicate question suppression of the responder. We now delay the question to ensure it will not get asked in the suppression window. Now that python-zeroconf ServiceInfo is aware of the question history, the problem became visible because it was unable to get the expected answers from another python-zeroconf instance.

fixes #1337

--

testing history:

  • review logs
  • look for leaks
  • run profile
  • py-spy to look for regressions
  • check TimerHandle dump
  • homekit testing

@codecov

codecov Bot commented Dec 16, 2023

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9b595a1) 99.77% compared to head (d7fea11) 99.75%.
Report is 14 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1348      +/-   ##
==========================================
- Coverage   99.77%   99.75%   -0.03%     
==========================================
  Files          30       30              
  Lines        3164     3240      +76     
  Branches      528      536       +8     
==========================================
+ Hits         3157     3232      +75     
  Misses          5        5              
- Partials        2        3       +1     

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

@bdraco

bdraco commented Dec 16, 2023

Copy link
Copy Markdown
Member Author

need to let this run overnight to look for any errors in the log in the morning, but its working as expected and really helps a lot

@bdraco

bdraco commented Dec 16, 2023

Copy link
Copy Markdown
Member Author

Can also dump TimerHandle objects to make sure the ServiceBrowser is still running

Comment thread src/zeroconf/_history.pxd
@bdraco

bdraco commented Dec 16, 2023

Copy link
Copy Markdown
Member Author

testing looks good over night. memory is stable as well

@bdraco bdraco marked this pull request as ready for review December 16, 2023 16:23
@bdraco bdraco merged commit b9aae1d into master Dec 16, 2023
@bdraco bdraco deleted the info_question_history branch December 16, 2023 16:23
bdraco added a commit to bdraco/home-assistant that referenced this pull request Dec 16, 2023
changelog: python-zeroconf/python-zeroconf@0.129.0...0.130.0

Major traffic reduction (fixed python-zeroconf/python-zeroconf#821
and python-zeroconf/python-zeroconf#1337)

The ServiceBrower query scheduler has been rewritten from the ground up to
use a heapq: python-zeroconf/python-zeroconf#1335

ServiceInfo requests are now aware of history and will now space asking
questions far enough apart to avoid being suppressed
python-zeroconf/python-zeroconf#1348
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.

Service info is unaware of question history

1 participant