mickhansen / dataloader-sequelize

Batching and simplification of Sequelize with facebook/dataloader
MIT License
284 stars 58 forks source link

Disabling caching / caching invalidation #78

Closed gunar closed 5 years ago

gunar commented 5 years ago

Hi there!

The caching portion of this library is causing my server to return outdated data to my clients. I've thought of two solutions:

1. Disabling caching

Is there any easy way to disable caching? I've tried setting TTL to 0 or 1 but it made the library crash.

I wanted to try setting max to 0 or 1 but I'm not sure anymore what max means. If I understand correctly, max is the maximum number of requests that can get merged into a single request, right? If then, that's not what I want, as it would defeat the purpose of the library (i.e. batching)

2. Caching invalidation

How does dataloader-sequelize do caching invalidation? I believe it does not, and it relies solely on Least Recently Used (LCU) to refresh the values. This is not good enough for any webapp, is it? Half of the time we'd be displaying outdated information. What am I getting wrong?

Ideally, I could add {[EXPECTED_OPTIONS_KEY]: context} to my updaters and have dataloader-sequelize understand that the rows updated need to have their cache invalidated.

Thanks in advance!

mickhansen commented 5 years ago

The library is meant to be used within a context, caching will be request local. Using it globally without the request context would currently result in some stale data yes.

The library is due to be updated to simply ignore batching/caching if request context is not available.

gunar commented 5 years ago

So right now if I call createContext(sequelize) anywhere in my code it will "pollute" my whole sequelize instance?

mickhansen commented 5 years ago

Yes currently it will as there is legacy code from before the library was context dependent, that needs to be removed so we short circuit when no context and then a semver major release.

Generally this library is used with graphql where you'd always have a request local context.

If the context is available the data can/should only be stale within that context.

mickhansen commented 5 years ago

Should be fixed in https://github.com/mickhansen/dataloader-sequelize/releases/tag/v2.0.0

intellix commented 4 years ago

Just realised we're getting stung by this on websockets where all of our mutations happen. On one mutation we change a property and then in another we check for such property where it's still old because the same websocket/context is there.

The ability to invalidate cache within the same context would be awesome (if not already possible?)