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.

Sign up for GitHub

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

Jump to bottom

Cache Query Responses #90

Closed
mohammed90 opened this issue Oct 3, 2018 · 4 comments
Closed

Cache Query Responses #90

mohammed90 opened this issue Oct 3, 2018 · 4 comments
Assignees
@bemasc
Labels
bug Something isn't working enhancement New feature or request

Comments

@mohammed90
Copy link

Most of the queries are for the same hosts. Caching the result for sometime could provide significant increase in response time.
@bemasc
Copy link
Contributor

bemasc commented Oct 3, 2018

Are you sure they are for the exact same host and type? Normally I see one "A" and "AAAA" for each name. However, I do often see a lot of queries under the same main domain, so the queries might look like the same domain if you don't expand the query details.

@mohammed90
Copy link
Author

Yes. Here are 2 queries for the same host and same type within about 20 seconds of each other:

  • First query at 01:01:28
    image

  • Second query at 01:01:41
    image

@bemasc bemasc added bug Something isn't working enhancement New feature or request labels Oct 3, 2018
@mohammed90
Copy link
Author

I thought I'll leave a few notes of my venture hoping it'll help whoever picks it up for implementation.

I looked at the code thinking I might implement it myself if it's very trivial (decorate the resolver somewhere). The project already uses Guava, so Guava's Cache{Builder,Loader} could fit neatly. Not so easy. The Loader requires an isolated method that returns the object to be cached. Currently I don't see any method that returns DnsTransaction. Therefore I believe a refactor is needed somewhere between DnsResolverUdpToHttps and DnsVpnService.

You're definitely more familiar with your project's code than I am, so correct my instinct if I'm wrong.

@bemasc bemasc self-assigned this Dec 21, 2018
bemasc pushed a commit that referenced this issue Dec 21, 2018
Add caching
520101f 
This change adds DoH-compliant caching and TTL rewriting.
It uses Caffeine, a cache framework that supports custom
expiration times for each entry and load coalescing, so
that a query can be fulfilled from cache if a load action
is in progress for that query.

Caching is disabled by default, subject to a Firebase
remote configuration experiment.

Fixes #90
bemasc pushed a commit that referenced this issue Dec 21, 2018
Add caching
d12e2ec 
This change adds DoH-compliant caching and TTL rewriting.
It uses Caffeine, a cache framework that supports custom
expiration times for each entry and load coalescing, so
that a query can be fulfilled from cache if a load action
is in progress for that query.

Caching is disabled by default, subject to a Firebase
remote configuration experiment.

NOTE: Currently this change is blocked by a compatibility
bug in Caffeine: ben-manes/caffeine#293

Fixes #90
@bemasc bemasc mentioned this issue Dec 21, 2018
Closed
@bemasc bemasc mentioned this issue Jan 17, 2019
Merged
@bemasc
Copy link
Contributor

bemasc commented Jan 17, 2019

It turns out that this was largely happening because of the threading bug that was fixed in #152. With that change, the OS's built-in cache should be sufficient.

@bemasc bemasc closed this as completed Jan 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bemasc bemasc

Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bemasc @mohammed90