Talking of Dataloader as a Cache confuses developers

[graphql]

Talking of Dataloader as a Cache confuses developers

[ThisIsMissEm]

Indeed! So, essentially we shouldn't talk of DataLoader as a cache, and instead talk of it as a batcher for requests to external services (databases, other services, etc).

If you have a piece of code that has a batch operator that is cache aware, then sure, use that, but talking of dataloader itself as a cache is confusing

[leebyron]

Hey @ThisIsMissEm I'd be happy to accept PRs that improved the documentation. Though I'm curious what you would suggest we use instead of "cache".

It's absolutely true that DataLoader does not and should not replace Redis in an architecture. However maybe the confusion is that "cache" is an overloaded term? For instance most would not suggest you can ditch Redis because Browsers have a cache, but also most would not suggest that browser-cache be called something other than a "cache". It seems like the root of the issue is that "cache" can mean a lot of different things based on context. It's really important for the large majority of (appropriate) uses of DataLoader that it caches loads to avoid repeat loads, but improving confusion around appropriate use of DataLoader and it's cache would be awesome.

I'm open to ideas.

[migueloller]

@ThisIsMissEm, why would using DataLoader with something like Redis or Memcached go against what DataLoader was intended for? Why not use DataLoader both to batch requests (to avoid the n+1 problem) and also as an application-wide cache using a distribute key-value store?

As long as the business model layer is handling authorization correctly it shouldn't be an issue to share the cache between requests, right?

EDIT 1: Or are you saying that it's better to do two tiers of caching, an in-memory one handled by DataLoader (for the purpose of the values resolving to the same one in the same request), and another one that is application-wide which DataLoader itself can query when it doesn't find the data in the database?

EDIT 2: Just went over the source code again and I'm just realizing that the "cache" in DataLoader is simply a promise cache and not an actual store. This makes it more obvious that it isn't intended to be used as an application-wide cache. Specifically because the 4 methods for the cacheMap options are all synchronous so using a map implementation that uses Redis or Memcached would not be possible since the methods would be asynchronous. I seem to have answered my own question but I guess this is useful to other developers that might get confused as myself. Realizing that the "cache" in DataLoader is simply a promise cache (promise are in-memory values that can't be serialized) is what made it click for me.

EDIT 3: It would also seem that having Redis as the first option of a "DataLoader backend" in the README is contributing to the confusion since Redis is often used as an application-wide cache.

[luckydrq]

I was considering a single dataloader instance at app level, and wanted to use IPC to sync data between worker processes. But i haven't considered about the case when there are multiple servers. So yes, now i have got a clear decision on this. Thank you!

[rturk]

@ThisIsMissEm this is good point, needs to be better documented

That is, I've seen people create a dataloader instance at application level, and think that this is safe because they're calling userLoader.clear() after changing a user. The problem with this is that that userLoader.clear() isn't broadcast to all other instances in your cluster.

[ThisIsMissEm]

Not just that, but dataloader is really designed for just the single request lifecycle – to handle cases where, say you have a article with comments, that you don't fetch the user details for every single user who's commented, and instead, you just fetch once per unique user.

i.e., because of the documentation, people are using dataloader for things which it was not intended, and hence, getting odd & unintended side-effects

[leebyron]

I just pushed some initial changes to Readme that hopefully helps make it more clear that Caching behavior should be used in a per-request way.

I'm going to close this issue since we're all in agreement about what needs to be done, though feel free to continue conversation or to keep linking people here who are looking for more information. Any PRs that further improve the language would be really helpful.

[ThisIsMissEm]

@leebyron thanks for the response! I'll try to rework the documentation soon, but it may be a week or two before I get to it.

[timqian]

@leebyron @ThisIsMissEm @migueloller
Let alone the implementation, I think the APIs exposed by dataloader is also suitable for application-level cache, this might be the reason which makes people use it that way.
What do your think of this package(https://github.com/DubFriend/redis-dataloader) that wraps dataloader using redis as the backend, and make the clear method a promise.
I think this lib resolves the problems @migueloller brought out on his comment.