Race condition between store.update() to multiple stores affecting localStorage/AkitaStores

mcroker commented 3 years ago

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[X] Bug report  
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request
[ ] Other... Please describe:

Current behavior

storage.clear() is clearing the akita storage, but the deleted store is almost immediately being restored to localStorage.

I believe I'm hitting rate condition between storage.clear() and store.update()... where if an update is in progress to any store the storage.clear() operation is almost immediately rendered pointless as store.update() overwrites the localstorage with an AkitaStores value that contained the store suposedly just deleted.

`

Expected behavior

I expect to be able to safely and atomically remove a store, including removal from storage without any cross-impact caused by events made to other stores.

Minimal reproduction of the problem with instructions

https://stackblitz.com/edit/github-8bgq4j?file=src%2Fapp%2Fapp.component.ts

Instructions provided in the app. All output is provided to console.

What is the motivation / use case for changing the behavior?

I believe store operations need to be independant of each other, we have no control over what operations are called in parallel so an operation to one store shouldn't impact operations on any other store. This results in race-conditions and other unexpected behaviour.

For further context I am using akita-ng-fire and firestore; the architecture of which I believe means that I require a dynamic EntityStore for each sub-collection...I mention this only as the choice to have many stores is I believe somewhat thrust upon me. e.g. if data-model is


 /teams/{team}
     /matches/{match}
     /players/{player}

I end up with a store for teams... and then a matches_store and players_store for each team.


teams_store
team_{team}_matches_store
team_{team}_players_store

The net-result is I have a number of stores, which are updated via akita-ng-fire in an event-driven manor. I have no direct control over the scheduling/timing of calls to update({}). The operation I am performing which caused me to notice this behaviour is a server-side cascade delete, where there are simultaneous(ish) event calls to delete a team from teams_store, and also triggered operations to clear/remove storage for the two sub-collection stores for matches and players.

Environment


    "@angular/core": "^11.0.2",
    "@datorama/akita": "^5.2.5",
    "@datorama/akita-ngdevtools": "^5.0.3",

Browser:
- [X] Chrome (desktop) version Version 87.0.4280.141 (Official Build) (x86_64) (on Mac).
- [ ] Chrome (Android) version XX
- [ ] Chrome (iOS) version XX
- [ ] Firefox version XX
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [ ] IE version XX
- [ ] Edge version XX

For Tooling issues:
- Node version: XX  
- Platform:  Mac

Others:

I wonder if this might be the cause of #365 ???

mcroker commented 3 years ago

Subsequent to creating this, I think I've also discovered that there is a race-condition between multiple parallel update operations to different stores (i.e. this isn't exclusively restricted to clear storage). The store getValue() call returns the correctly updated value, but the value persisted in localStorage is not correctly updated.

I've updated the stackbltz.com to also show this behaviour

sithwarrior commented 6 months ago

Hi @mcroker this is an old issue, so maybe not relevant anymore, and you could perhaps close it, but just wanted to give my 2 cents..

I think you are "using it wrong" the persist state, is just that, a way to make sure your akita store are persisted if the user closes the page etc, so trying to query both localstorage and the stores simultaneously in race conditions, might give weird results momentarily, until everything is resolved.

If you are trying to clear data, you shouldn't try and clear localstorage/persiststate, but call reset on the store it self, and that will then replicate into localstorage.

calling ClearStore directly on persistState, should imo only be done if you are removing all data, say if the user is logging out or something similar, and then you should probably be calling reset on the stores themselves first, clearing event handlers etc.

salesforce / akita