Skip to content

Enable in-memory caching in DnsOverHttps and AndroidDns#9594

Open
swankjesse wants to merge 1 commit into
masterfrom
jwilson.0725.enable_caches
Open

Enable in-memory caching in DnsOverHttps and AndroidDns#9594
swankjesse wants to merge 1 commit into
masterfrom
jwilson.0725.enable_caches

Conversation

@swankjesse

Copy link
Copy Markdown
Collaborator

The default cache size is 1,000, which is effectively unbounded for mobile apps, which likely call a few dozen API servers, ad networks, and analytics systems.

I don't expect DnsOverHttps to be deployed in other environments. If so, it's possible this cache will be problematic, as even 10-second stale results can be problematic for server environments. I'll put an appropriate warning about this in our release notes.

/** Drives [StateMachineDnsCall] using the system resolver and [DnsResolver]. */
private val queryFactory =
DnsQuery.Factory { question ->
dnsCache.`-delegate`.wrap { question ->

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think if we do this, we are also responsible for evicting on network changes. So it's if it's the default, we need to ship that wiring also.

This is fine until some oblivious well intentioned grunt makes it the default in an unrelated PR.

Base automatically changed from jwilson.0725.simpler_cancels to master July 25, 2026 21:53
The default cache size is 1,000, which is effectively unbounded
for mobile apps, which likely call a few dozen API servers, ad
networks, and analytics systems.

I don't expect DnsOverHttps to be deployed in other environments.
If so, it's possible this cache will be problematic, as even
10-second stale results can be problematic for server
environments. I'll put an appropriate warning about this in our
release notes.
@swankjesse
swankjesse force-pushed the jwilson.0725.enable_caches branch from e250c7d to 3cdfe81 Compare July 25, 2026 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants