Questions about _DNSCacheTable and its memory leak

[Snawoot]

Long story short

TCPConnector by default uses _DNSCacheTable and this table only grows, no matter how many expired records it has. Simply put, it's memory leak.

Also, it uses fixed TTL, instead of real TTL of record. I'm aware getaddrinfo doesn't return TTL for record, but aiodns does.

Expected behaviour

Expired cache records get evicted.

Actual behaviour

Expired DNS records stored forever.

Steps to reproduce

Normal, documented usage.

Your environment

Any valid environment.

Full story

I've created this issue to highlight memory leak issue and inquire your vision of proper order of things. It'll be good to know your opinion before I or someone else even consider doing something with it.

Here is my questions:

1. Why aiohttp needs own, non-compliant with standards cache in first place? Why it is trying to solve problem which nscd and caching resolvers do solve? Our community of asyncio users suggests aiohttp tries to avoid slow invocations of getaddrinfo via thread pool, but I'd rather ask.
2. Which solution you likely approve? I see following options:
   • Schedule eviction of added records via heapq and perform it in lazy fashion, on cache add/query operations. This also will allow to implement independent TTL for records in future.
   • Allow users to specify own DNS table class.
   • Both of it.

[theSage21]

I'm using aiohttp for a websocket server which multiplexes different legacy apis for the browser. This issue causes the server to MemoryError or cannot open a new thread after around 30 minutes.
The fix I'm using right now is to create the TCPConnector with ttl_dns_cache=None.

[asvetlov]

sorry, my time is very limited right now not I'll return back to the question soon

[theSage21]

You've done a wonderful job with the library and my life is easier because of it. Thank you for that. I'm currently going through the library and if I am able to wrap my head around it I'll submit a PR

[yushizhao]

This is a serious issue, any updates?
Is there a work around available now?
Does use aiodns solve the issue?

[theSage21]

I'm using aiohttp for a websocket server which multiplexes different legacy apis for the browser. This issue causes the server to MemoryError or cannot open a new thread after around 30 minutes.
The fix I'm using right now is to create the TCPConnector with ttl_dns_cache=None.

Creating the TCPConnector with ttl_dns_cache=None gets rid of the cache entirely and so fixes the symptoms. I haven't had time to fix the underlying issue yet so I guess that's the simplest way to get things to work right now.

[yushizhao]

@theSage21 Thank you!
I have read the doc about the TCPConnector, here are two parameters I think related:

use_dns_cache (bool) –

use internal cache for DNS lookups, True by default.

Enabling an option may speedup connection establishing a bit but may introduce some side effects also.

ttl_dns_cache (int) –

expire after some seconds the DNS entries, None means cached forever. By default 10 seconds.

By default DNS entries are cached forever, in some environments the IP addresses related to a specific HOST can change after a specific time. Use this option to keep the DNS cache updated refreshing each entry after N seconds.

Also, there is a method:

clear_dns_cache(self, host=None, port=None)
Clear internal DNS cache.

Remove specific entry if both host and port are specified, clear all cache otherwise.

So, in my understanding,

1 ttl_dns_cache=None actually means cached FOREVER which will only make the problem WORSE. (no offense @theSage21, maybe there has been an update on the code )
2 use_dns_cache=False may solve the problem ("gets rid of the cache entirely").
3 the downside of not using dns cache is slow down your program when you made repeated calls to the same endpoint. If that is what you do, your dns cache will not grow anyway. In other words, only close it when you call a lot of different endpoints.
4 call clear_dns_cache on a regular basis may be a good alternative in some cases.

Would you confirm these 4 points? @theSage21

[theSage21]

@yushizhao you're right, it seems that my knowledge on this is outdated.

1. You're right. This actually ends up being cached forever.
2. I haven't tested this but looks like it would work.
3. Valid point.
4. I can't argue against that.

TBH I've stopped working with aiohttp a while back so I might not be the right person to answer these anymore. @sruthipradeep @niazangels perhaps there's a more recent fix for this that I'm not aware of?

[yushizhao]

@theSage21 Thank you again! I will run some tests on these.

[xlorepdarkhelm]

This leak is affecting/impacting the project I have been working on at work. We start getting socket.gaierror: [Errno -5] No address associated with hostname and OSError: [Errno 16] Device or resource busy on every request, 100% error rate until I bounce the server. Takes about 3-4 days for this to happen now, with increasing traffic making the time before server bounce getting shorter.

[Dreamsorcerer]

It would be a good start if someone were to write a test that reproduces the problem. e.g. Make some dummy tests, patch the clock to move past the expiry times and then assert whether the keys exist in the table.

Currently, I see there is a function to clear hosts from the cache:

aiohttp/aiohttp/connector.py

Line 792 in f4146fa

def clear_dns_cache(

But, I'd assume it's not getting called automatically anywhere. Maybe we can do something along the lines of checking for expired keys when a new request is made and clear them from the dict, or bound the dict to a given size, so it will only store the last 1000 hosts or similar.