Repository navigation
Huge blocking time as soon as persistQueryClient/createWebStoragePersistor clears the old entries from cache #3034
Description
Activity
Can you create a codesandbox example with that amount of data please? The auto removal feature is rather new so it could be that its not efficient enough. On the other hand, I’m not sure what we can do apart from removing the oldest query and then try to persist again 🤔
@NewFuture FYI, maybe you’d have an idea here?
maybe we can also agree that always doing what we are trying to do (remove the oldest item and try to persist again) is not really applicable for all situations. Maybe we could let the user provide an
overflowStrategy, which could be eitherfirstInFirstOut(what we have currently), or something else that works better? Maybe even a custom function where the user can returntrueto keep persisting the query andfalseto throw it away?overflowStrategy?: 'firstInFirstOut' | (query: Query) => booleanwhat do you think?
we use
try-serialize-catchto detect the maximum space to store now.
It's a performance-sensitive way,
It will cost lots of resources when frequently called with continually increasing queries.@tilman can try to increase the
throttle time, or adddehydrateOptions.shouldDehydrateQueryto filter the persistQueries.@TkDodo it's a good solution that users can provide a function handling this situation.
But it's hard to define the API.
overflowStrategy?: (query: persistedClient ) => persistedClient|false?
or
saveStrategy?: (query: persistedClient ,storage:Storage,retryTimes) => boolean?I was working with tilman together on this and we eventually settled on using this custom
Storageimplementation.
It's basically the same as the default storage from react-query, just way more aggressive when purging dataclass LRUStorage implements Storage { private storage: Storage; private queryClient: QueryClient; constructor(queryClient: QueryClient) { if (typeof window === "undefined") { throw new Error("window is undefined"); } this.storage = window.sessionStorage; this.queryClient = queryClient; } public get length() { return this.storage.length; } public getItem(key: string) { return this.storage.getItem(key); } public removeItem(key: string) { return this.storage.removeItem(key); } public clear() { return this.storage.clear(); } public key(index: number): string | null { return this.storage.key(index); } public setItem(key: string, value: string): void { let err: Error | undefined; const MAX_ATTEMPTS = 10; for (let attempt = 0; attempt <= MAX_ATTEMPTS; attempt++) { try { return this.storage.setItem(key, value); } catch (error) { err = error as Error; const cache = this.queryClient.getQueryCache(); const oldest = [...cache.getAll()].sort( (a, b) => a.state.dataUpdatedAt - b.state.dataUpdatedAt, ); oldest.slice(0, 2 ** attempt).forEach((entry) => cache.remove(entry)); } } console.error(err); } }
And then calling it
const queryClient = new QueryClient({ ... }) persistQueryClient({ queryClient, persistor: createWebStoragePersistor({ storage: new LRUStorage(queryClient), }), });
we didn't notice any performance issues after using this.
I think being able to provide anoverflowStrategyor similar would be nice, since our solution requires quite a bit of boilerplate code.Reacted by Vijay Pemmarajunow that the plugin will become stable in v4, I think it's time to decide what we want to do here. The current approach seems to "work", but it's also computationally expensive. Also, we've only added it to the
webStoragePersister, but not theAyncStorageone. But in react-native, there is also a limit on how much you can store, and it would be good to also support differentoverflowStrategiesthere.So if someone wants to contribute this to v4 (currently
alphabranch), I'd be happy to see your proposals :)Another idea: instead of providing
overflowStrategy, we could simply provide anhandlePersistError(persistedClient: PersistedClient, error: Error, errorCount: number): Promisable<PersistedClient>callback function where everyone can react however they want to any kinds of errors while persisting, including overflow errors. We would expect to get a new
persistedClientback that might have fewer entries with which we would re-try the save.additionally, we could export some default strategies to handle these errors that conform to this interface. One would be the current try-remove-old-things handler, but given it has perf issues, I wouldn't use this one per default anymore. We can think about providing more errr handling strategy functions (should be treeshakable if unused), and they should work for both the async persister and the webstorage persister.
not doing LRU anymore per default is breaking, but all of this should be for v4 anyways, and the fix would be simply to import and use the error handler.
thoughts?
Reacted by New Future
Describe the bug
If using
persistQueryClientandcreateWebStoragePersistorwithwindow.sessionStoragelike in the example below, then the Performance of the Web-App drastically decreases as soon as the sessionStorage is full. In my example each query result can vary from very few data (few numeric values) to very huge amount of data (arrays with length ca. 10000 of objects with ca. 10 keys). Therefore deleting one item from the cache often means that a big item can still not fit into it, resulting in many failed write requests in the current implementation which furthermore results in very huge blocking times of the main thread.To Reproduce
Expected behavior
No huge blocking time of the main thread.
Screenshots
Desktop (please complete the following information):
Smartphone (please complete the following information):
not tested yet