graphql-flutter icon indicating copy to clipboard operation
graphql-flutter copied to clipboard

Cache invalidation/deletion options

Open micimize opened this issue 5 years ago • 16 comments

If I'm not mistaken, cache entities currently live forever. This is not good.

The related apollo discussion probably has some good ideas

micimize avatar Jul 02 '19 16:07 micimize

@micimize do you think some sort of simple garbage collector would work? Invalidating cache keys based on:

  • expire date(default 7 days or something like that).
  • persistent storage limit(128MB by default) and in-memory limit(should be configurable for device ram size and scale based on that).
  • not enough space left for a new key.

Also adding to cache should have some boundaries:

  • If the value is too big(greater than storage limit), then it shouldn’t be added.

I think persistent storage should have a polling interval to fetch and save new keys that were created / updated / deleted, and not the entire tree.

I could do most of the implementation if we agree on a spec. Maybe we could make it a third package for cache(reuse it in other projects and do-couple it from graphql)

eusdima avatar Jul 04 '19 07:07 eusdima

One thing we could do is just make the cache's Map storage injectable and default it to LruMap.

I think ideally, caching and persistent storage are concerns we externalize/generalize so that pretty much anything with a Map interface can be used, and we just provide defaults.

For example, we make a separate package, persistent_maps, that handles all the storage details, and auto-saves/re-saturates based on configuration, and then depend on it as a default. That way we don't have to worry about the eccentricities of caching.

micimize avatar Jul 04 '19 21:07 micimize

@micimize If a use login using an account and then logs in with another account, the previous login account's cache is still available in second login account. I think we need a function to just simply invalidate the entire cache as well.

serendipity1004 avatar Oct 10 '19 08:10 serendipity1004

https://github.com/zino-app/graphql-flutter/blob/519381cd1c57515fd26cedb33c68a68d563a6eac/packages/graphql/lib/src/cache/in_memory_io.dart#L68-L72

is there a way to have this available without running Query or Mutation ?

serendipity1004 avatar Oct 10 '19 09:10 serendipity1004

@serendipity1004 We should probably expose the reset method, maybe at the client level, so you can invalidate the cache easily.

And on a side note maybe we should switch to using an interface instead of an abstract class. This way, we all methods we need must be overridden by the implementing class and can't be ignored. This is because some cache have not implemented the reset() method override and am guessing an interface would guarantee some security in that regard.

mainawycliffe avatar Oct 10 '19 11:10 mainawycliffe

okay so something like adding clearCache below and exposing clear in Cache

https://github.com/zino-app/graphql-flutter/blob/519381cd1c57515fd26cedb33c68a68d563a6eac/packages/graphql/lib/src/graphql_client.dart#L15-L58

Should I work on that now or wait till interface implementation?

serendipity1004 avatar Oct 16 '19 09:10 serendipity1004

@serendipity1004 you can go ahead with cache.clear - I don't think we need client.cache is exposed already anyhow, so client.cache.clear() will be callable from the client.

micimize avatar Oct 16 '19 16:10 micimize

At what point is one meant to do client.cache.clear?

I am really disturbed by this problem as my cache is not clearing and data from old accounts show in other accounts. I would appreciate seeing a use case of how you implemented it.

bamoha avatar Feb 13 '20 03:02 bamoha

@bamoha client.cache.clear hasn't been implemented yet, but if you do the following on user log out it should be fine:

client.cache.reset(); // empty the hash map
await client.cache.save(); // persist empty map to storage

if you're doing so and the cache still isn't clearing, please open new issue with code demonstrating the issue

micimize avatar Feb 13 '20 16:02 micimize

@micimize I got OS error when calling client.cache.save()

FileSystemException: Cannot open file, path = 'cache.txt' (OS Error: Operation not permitted, errno = 1)

I'm using InMemoryCache

xfumihiro avatar May 28 '20 06:05 xfumihiro

4.0.0-alpha.7 adds a client.resetStore and exposes the cache.store.

In v4, storage is decoupled from the cache, so writing a 3rd party store with eviction capabilities should be straightforward with something like https://pub.dev/packages/stash

micimize avatar Sep 17 '20 16:09 micimize

I think it will be really helpful if we have chache keys. So we can have option to invalidate a certain cache like client.cache.invalidate('my-posts-key') and only posts cache will be deleted leaving the rest caches intact. This will really be helpful in things like refresh etc.

kateile avatar Dec 24 '20 14:12 kateile

@kateile given the current design of the cache, which uses normalize, one would have to go through all keys in the cache to find posts to delete – which is probably less expensive than it seems.

There are two decent invalidation approaches I can think of:

  1. Provide a a special invalidation request type, that would allow for the invalidation of top-level keys based on a filter, with a few default filters provided, like "match the top-level keys of these operation ASTs," "match all Post keys," and "match everything except what is referenced by the given operation ASTs."

  2. Read a set of operation ASTs from the cache, then delete all references seen from the store. It is a bit more difficult than it appears, as the nested normalized results might be depended on by other operations one doesn't want to invalidate. This would mean we'd have to comb through all other operations aside from the invalidated ones, collect their references, and filter against them.

micimize avatar Dec 27 '20 16:12 micimize