[apollo-datasource-rest] Possible memory leak due to TTL failure about datasource-rest HOT 4 CLOSED

I can confirm that we see the same behavior.

from datasource-rest.

Do you have the same behavior when you're not using cachecontrol?

from datasource-rest.

@michielmulders Yes same behavior.

from datasource-rest.

I'd love to investigate this now that we are maintaining RESTDataSource again, but the original report isn't a complete reproduction recipe that allows us to try it out without a bunch of creative choices to try to reproduce what you were seeing. The code snippets are incomplete, and also it doesn't say what kind of operations you run (the same operation over and over? different ones?)

To be honest, I would expect exactly the opposite issue here: back in 2020 when you filed this, the default KeyValueCache used by ApolloServer had an unbounded size, so without any TTL I would expect a leak, where as with a TTL I would expect no leak. Looking into it further, it looks like lru-cache@6 (which powered InMemoryLRUCache back then) doesn't have any special handling of a ttl of -1 (which we convert to a maxAge of -1000) and this is interpreted not as "there is no TTL, keep forever" but "it's already out of date, drop as soon as you bother to check". So it's not too surprising that ttl: 1 uses more memory than ttl: -1.

Additionally, the lru-cache library does not actually active purge things that are out of date by default. It's just that if you try to fetch something that is out of date, it will get dropped. The older lru-cache@6 version used when this was filed had no way of automatically dropping old data other than manually calling prune() every so often. The newer version used by today's InMemoryLRUCache does have a ttlAutopurge flag, though it's not recommended by the author. More importantly, the newer version of lru-cache doesn't let you configure an unbounded cache, and so the current version of InMemoryLRUCache is always bounded.

So basically, I think everything you're seeing here is addressed by the fact that the current InMemoryLRUCache (in @apollo/utils.keyvaluecache) is backed by lru-cache v7, which is always a bounded cache.

If you're able to reproduce this with the latest versions of RESTDataSource (in @apollo/datasource-rest, not apollo-dataousrce-rest) we can reopen (ideally with a full self-contained reproduction!).

from datasource-rest.