Disable DNSSD cache globally in preparation for address resolve #16029
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In project-chip#15934 caching is generally not used, with the rationale of: - cache is doubled/trippled: - Platform MDNS servers are expected to cache at least on linux and mac due to avahi/bonjour being system wide and enforcing TTLs - We have cache both on CASESession and Platform, with separate caching between operational and commissionable - MinMdns does not have the dnssd cache like platform, but would likely benefit the most of it (since it would have TTLs) - we do not have a good way to clear caches and we do not update them on boot advertising (platform dnssd implementations would, however since we have an internal cache that seems to break) Looking to disable caching and either build it anew if it required/helps and at least make use of only one.
boring-cyborg
bot
added
app
controller
efr32
esp32
lib
linux
nrf connect
platform
telink
labels
pullapprove
bot
requested review from
anush-apple,
austinh0,
balducci-apple,
Byungjoo-Lee,
bzbarsky-apple,
carol-apple,
cecille,
chrisdecenzo,
chshu,
chulspro,
Damian-Nordic,
dhrishi,
electrocucaracha,
emargolis,
erjiaqing and
franck-apple
pullapprove
bot
requested review from
jmartinez-silabs,
lazarkov,
LuDuda,
lzgrablic02,
mrjerryjohns,
msandstedt,
sagar-apple,
saurabhst,
selissia,
tecimovic,
turon,
vijs,
vivien-apple,
wbschiller,
woody-apple,
xylophone21,
yufengwangca and
yunhanw-google
|
PR #16029: Size comparison from cd44f9b to c430b8e Decreases (38 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Full report (38 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
bzbarsky-apple
approved these changes
Mar 9, 2022
I believe that was already gated by the flag (or I assumed so). Will double-check. |
|
Already gated: https://github.com/project-chip/connectedhomeip/blob/master/src/lib/dnssd/Discovery_ImplPlatform.cpp#L41 so the change to make size 0 should disable it. |
cecille
approved these changes
Mar 9, 2022
|
PR #16029: Size comparison from 3382a5f to 1d90e31 Decreases (38 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Full report (38 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
DNSSD caching is not consistent : extra caching layers (both case and platform), likely doubling platform caches, no cache expiry, missing where we really need it (device side where platform service does not exist).
see #11632 and #12762
Change overview
Set cache size to 0, add more ifdefs. This is in preparation to removing it everywhere or moving it to a central place, in which case we either delete or reuse the DNSSD class.
Testing
CI will validate, expect smoke tests to also pick it up to validate.