evdns caching #42

merged 0 commits into from Jun 10, 2013


None yet
4 participants

ghazel commented Mar 5, 2012

This is a simple splay tree implementation of caching in evdns. I toyed with the idea of implementing it in my app, but the lack of ttl information in the addrinfo callback made it impossible to do accurate expiry. Any other implementation seemed to require copying large sections of evdns code, so I patched evdns instead.

If a cache implementation in evdns is not a welcome change, one of those interfaces changes would be very helpful for applications interested in caching.


errzey commented Mar 5, 2012

I think I was the one who said you might want to look at using a splay for a cache like this. but in this case you are using event timers to expire based on ttl; so a hash might be a better bet.

How do you deal with NX cache here? Are we going to deal with cname type caching? Are we ignoring authority / additional?


ghazel commented Mar 5, 2012

A splay seemed expedient because there was an implementation handy. Happy to swap it out for a hash table if you can recommend a suitable implementation. Both seem performant enough for reasonable cache sizes.

The cache is inserted basically at the getaddrinfo api level (except /etc/hosts is not cached), and does not store NX responses or any error case. Authority is not handled differently - the cache just duplicates whatever addrinfo chain the caller would receive.


ghazel commented May 29, 2013

I moved this work to a branch ( https://github.com/ghazel/Libevent/tree/evdns_caching ), but Github won't let me change the branch this pull request points to.

This pull request is getting stale anyway. Should I re-submit it? Redo the work with a hashtable (the splay tree has worked flawlessly for me all year)? Submit some other sort of API which exposes TTL so that my app can implement the caching itself?


azat commented Jun 2, 2013

As far as I can see there is no option to turn it off, I suggest to add some flag to evdns_base_new().

What about changing branch - look here http://stackoverflow.com/a/11983910/328260

@nmathewson nmathewson merged commit c307a25 into nmathewson:master Jun 10, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment