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

Cache only queries #80

Closed
sipsorcery opened this issue Jul 23, 2020 · 3 comments
Closed

Cache only queries #80

sipsorcery opened this issue Jul 23, 2020 · 3 comments
Assignees
Milestone

Comments

@sipsorcery
Copy link
Contributor

I've recently replaced some decades old DNS logic with DnsClient.NET (went from 10k lines down to 1k). One feature I'm now missing is the ability to get results directly from the in-memory cache. I can implement a custom feature but I thought I'd first check if it's a desired feature for this library in which case I can do a PR?

As background the requirement for the cache lookups is for a SIP (VoIP) library. In a similar manner to HTTP, SIP requests and responses are sent using SIP URI's in the form sip:1234@sipsorcery.com. A difference from HTTP is that SIP is normally sent over UDP and therefore has a built in retransmit mechanism. If a SIP message does not get a response it will retry the send at intervals of 0.5, 1s, 2s, 4s, 4s, 4s ... up to 11 retries.

How that's relevant for DNS cache lookups is that for high volume SIP server applications it's not feasible to block the main SIP transport thread waiting for DNS lookups. The desired approach is to get the initial request at t0, identify a DNS lookup is required and then discard the message. At t0.5 the same message will be retransmitted. If the DNS result is in the cache it gets used otherwise the message is again discarded. And so on at t1, t2, t4 etc.

@MichaCo
Copy link
Owner

MichaCo commented Jul 23, 2020

@sipsorcery happy to hear the library helps to reduce lines of code, that's always good ;)

Interesting use-case.
Isn't it a bit risky though to rely on cache lifetime?
At least with this implementation of DnsClient, the ttl of a record is respected and if that ttl is very low, the response might be cached after your 10th try and then expire before your 11th try...
You could use min/max cache timeouts though to minimize that risk.

Another problem might be that you can get a valid response back which doesn't get cached at all. An empty response with no records for example, because the domain couldn't be resolved. Wouldn't that cause unnecessary retries?

It might be better if you just implement some custom cache on top where you have more control over and can decide what to do with DNS errors or empty responses for example.

Otherwise, yes, we could add a method on the LookupClient to check if the response of a Question is cached by generating the cache key for that question and checking if the key exists.
Feel free to send a PR for that ;)

@sipsorcery
Copy link
Contributor Author

Otherwise, yes, we could add a method on the LookupClient to check if the response of a Question is cached by generating the cache key for that question and checking if the key exists.

Yes that's pretty much what I had in mind. I'll take a look.

For error and empty responses I still add them to the cache but with relatively short timeouts. Without that the same lookup failure can get repeated multiple times wasting resources. Applications with low volumes of lookups may not want that behaviour so it could be adjusted with a configuration setting on the client.

For the cache lifetime, what types of DNS records have a TTL of less than 4 seconds (the SIP retransmit interval)? That being said I do recall in the past putting a min/max lifetime on cache entries just in case.

sipsorcery added a commit to sipsorcery/DnsClient.NET that referenced this issue Jul 30, 2020
…of this is to allow an application to take different actions depending on whether a network lookup needs to be undertaken or not. See MichaCo#80.

Adds a setting to choose whether lookup failures should be cached and if so for how long. The point of this is to allow applciations to avoid high frequency lookups on failing queries.
@MichaCo MichaCo added this to the 1.4.0 milestone Jul 30, 2020
MichaCo pushed a commit that referenced this issue Aug 1, 2020
…#82)

Adds a new public method to allow the cache to be queried. The point of this is to allow an application to take different actions depending on whether a network lookup needs to be undertaken or not. See #80.

Adds a setting to choose whether lookup failures should be cached and if so for how long. The point of this is to allow applications to avoid high frequency lookups on failing queries.
@MichaCo
Copy link
Owner

MichaCo commented Aug 1, 2020

/closing - PR is merged and will be in v1.4.0

Thanks again for your work 👍

:shipit:

@MichaCo MichaCo closed this as completed Aug 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants