Skip to content

Conversation

@vsouza
Copy link

@vsouza vsouza commented Nov 7, 2024

This PR adds functionality to cache DNS resolutions using the dnscache package. The goal is to improve DNS resolution performance by avoiding repeated lookups for addresses that have already been resolved.

Main changes:

  • Implemented DNS resolution caching using dnscache.Resolver{}.
  • Added unit tests to validate the DNS cache behavior.
  • Updated documentation to reflect the use of DNS caching.

Motivation:

By introducing this feature, we aim to reduce the time spent on frequent DNS lookups, especially in high-load scenarios where multiple resolutions for the same domain are needed. Using dnscache, we expect to optimize the performance of HTTP requests by ensuring the cache is properly used for DNS resolutions.

@KalilCazes
Copy link
Contributor

Amazing work @vsouza !

@KalilCazes
Copy link
Contributor

KalilCazes commented Jan 27, 2025

Hi, @vsouza . All good?
I was rethinking how the goroutine was created in the SetDNSCache() method. It seems to me that there is a chance of a leak happening, should we have another approach?

Something like this:

ctx, cancel := context.WithCancel(context.Background())
defer cancel()

go func() {
    t := time.NewTicker(refreshCacheTime)
    defer t.Stop()
    for {
        select {
        case <-ctx.Done():
            return
        case <-t.C:
           if err := r.Refresh(true); err != nil {...}
        }
    }
}()

@vsouza
Copy link
Author

vsouza commented Jan 29, 2025

@KalilCazes good catch, I'll fix it!

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