Cache MX and A server lookups

[ianbayne]

Closes #179.

[ianbayne]

Hi @micke, please let me know if this isn't what you had in mind. Happy to follow a different path 👍

[micke]

This is a good start, i'm however thinking that we might need to consider two factors still:

1. The cache growing to be too big.
2. The cache going stale, i think it would be really important for us to take the DNS records TTL into consideration to make sure that we don't cache the MX servers for too long.

[ianbayne]

Thanks for the feedback, @micke. Any thoughts on a maximum size for the cache?

[ianbayne]

This is a good start, i'm however thinking that we might need to consider two factors still:

1. The cache growing to be too big.

2. The cache going stale, i think it would be really important for us to take the DNS records TTL into consideration to make sure that we don't cache the MX servers for too long.

@micke, I've gone ahead and pushed up some additional commits that should meet these requirements. I've arbitrarily selected 1,000 as the max cache size. Please feel free to suggest a different value! Also, a lot of this code could be DRY-ed up; if you like the implementation I’ll do so.

[micke]

@ianbayne Great work! What do you think about maybe moving the caching logic to it's own class?

[ianbayne]

@ianbayne Great work! What do you think about maybe moving the caching logic to it's own class?

Thanks! Sure, works for me. I’ll get on it when I’ve next got some spare time 👍

[ianbayne]

I prefer this personally, but feel free to delete if you'd rather not have it.

[ianbayne]

DnsRecordsCache

Definitely open to different/better naming for this!

[ianbayne]

@ianbayne Great work! What do you think about maybe moving the caching logic to it's own class?

This should be ready for another review, @micke. Lots of stubbing going on in the tests, but hopefully not an excessive amount.

[mscrivo]

This is great stuff @ianbayne! One other optimization I can see being useful here is to move the setup of Resolv::DNS::Config into these functions where it's actually used. We're seeing a perf problem with heavy use of this gem where it's spending a lot of time setting up this config even though we're only calling valid? and never actually using it.

[ianbayne]

Sure @mscrivo, happy to make that change. See commit eb794f2.

@micke, please let me know if you'd prefer this commit to be part of a separate PR.

[micke]

@ianbayne huge thank you for this! Great work!

[ianbayne]

@ianbayne huge thank you for this! Great work!

Thanks, @micke! Was a fun feature and I learned a lot.