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.

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

Add DNS RR cache #3

Merged
merged 3 commits into from
Apr 14, 2021
Merged

Add DNS RR cache #3

merged 3 commits into from
Apr 14, 2021

Conversation

vyzo
Copy link
Collaborator

@vyzo vyzo commented Apr 14, 2021

this ensures that we don't do redundant DNS lookups; we do have the TTL after all.

@vyzo vyzo requested review from lidel and aschmahmann April 14, 2021 17:00
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

TTL handling LGTM.
(I don't like having a single TTL for A and AAAA records, but I don't think this will be a problem in the real world: those will usually be the same, and in case they differ, we pick the lower one, so the end user would not see any stale caches, which is ok.)

I also confirmed the cache speeds up subsequent lookups in patched go-ipfs:

$ time ipfs name resolve /ipns/brave.crypto
/ipfs/QmWrdNJWMbvRxxzLhojVKaBDswS4KNVM7LvjsN7QbDrvka
ipfs name resolve /ipns/brave.crypto  0.22s user 0.01s system 7% cpu 3.070 total

$ time ipfs name resolve /ipns/brave.crypto
/ipfs/QmWrdNJWMbvRxxzLhojVKaBDswS4KNVM7LvjsN7QbDrvka
ipfs name resolve /ipns/brave.crypto  0.03s user 0.02s system 170% cpu 0.030 total

% time ipfs name resolve /ipns/brave.crypto
/ipfs/QmWrdNJWMbvRxxzLhojVKaBDswS4KNVM7LvjsN7QbDrvka
ipfs name resolve /ipns/brave.crypto  0.04s user 0.02s system 177% cpu 0.030 total

Works as expected, thank you @vyzo 👌

@vyzo vyzo merged commit 01bec93 into master Apr 14, 2021
@vyzo vyzo deleted the feat/resolver-cache branch April 14, 2021 19:10
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.

3 participants