State: Refactor state persistence #9957

aduth · 2016-12-10T00:00:53Z

This pull request seeks to make a number of enhancements to Redux store persistence:

Perform persistence inside a Web Worker instance
Allow application to render prior to persisted state loading, queuing dispatched actions to be replayed after persisted state restored
Apply per-key state persistence TTL (time-to-live)
Extract persistence logic to standalone library module

Implementation notes:

WIP: This a very early prototype, and is not very functional at the moment.

Unfinished:

The application does not render well with a uninitialized state
Decide how to import localForage inside the webworker
- Currently loads from CDN, which is not ideal. We may need to create a separate script for the worker which is bundled with our lib/localforage module
Server deserialization (I think this is planned to be eliminated?)
Per-key state persistence TTL
Testing

Finished:

Synchronous application rendering (albeit broken)
Reimplementation as standalone store enhancer module
Web worker messaging (serialization, deserialization)
Action queueing while persisted state yet to be restored

Related refactoring includes:

Creating separate modules for createStore between server and browser to minimize typeof window conditions
In doing so, moving reducers to common state/reducer.js file

Testing instructions:

TBD

cc @gwwar

matticbot · 2016-12-10T00:00:55Z

Test live: https://calypso.live/?branch=update/state-persistence-web-worker

aduth · 2016-12-10T00:11:15Z

Performing persistence in a web worker might also make it more reasonable to consider using something like jsonpack or lz-string for compression. @blowery I think you may have mentioned one or the other of these in the past?

blowery · 2016-12-12T02:08:28Z

Doing it in a worker would be really interesting. What are the limitations on storage there?

…

On Fri, Dec 9, 2016, 7:11 PM Andrew Duthie ***@***.***> wrote: Performing persistence in a web worker might also make it more reasonable to consider using something like jsonpack <https://github.com/sapienlab/jsonpack> or lz-string <https://github.com/pieroxy/lz-string> for compression. @blowery <https://github.com/blowery> I think you may have mentioned one or the other of these in the past? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9957 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAA4DghaYNxbsIQcRg1kERytX-P0-RzBks5rGe4qgaJpZM4LJerm> .

aduth · 2016-12-12T14:17:27Z

@blowery Based on the following, it would seem that localStorage is unavailable in workers, but that IndexedDB thankfully is:

Edit: A comment on this answer suggests IndexedDB is unavailable in Safari web workers. We might need to investigate that. Perhaps localForage will have a reasonable fallback for this case inside the worker?

aduth · 2016-12-12T14:19:14Z

Another related avenue to explore may be putting the reducer execution inside a web-worker, assuming we're not expecting it to run synchronously (let's hope).

blowery · 2016-12-12T14:34:30Z

client/lib/state-persistence/enhancer.js

+		STATE_REPLACE: 'STATE_REPLACE'
+	},
+	throttle: 500,
+	maxAge: 604800000


could we express this in terms of some unit? appears to be 7000 days?

could we express this in terms of some unit? appears to be 7000 days?

I guess @gwwar was on to something with her implementation 😄

The time is 7 days (Date.now is measured in milliseconds since epoch, not seconds)

At the minimum a comment would help, though those can drift over time.

though those can drift over time.

like any clock! hehe

blowery · 2016-12-12T14:37:05Z

client/lib/state-persistence/enhancer.js

+			queue.push( action );
+		}
+
+		getState().catch( noop ).then( ( persistedState ) => {


why the catch( noop )?

why the catch( noop )?

Since we need to replace the reducer and replay the queued action regardless of whether we were able to load anything from persistence.

Though, given we're never handling errors in either the getState or below in setState, maybe this should be swallowed in worker.js instead of here.

blowery · 2016-12-12T14:43:35Z

client/lib/state-persistence/worker.js

+
+	function createWorker() {
+		const blob = new Blob( [ `
+			importScripts( 'https://cdnjs.cloudflare.com/ajax/libs/localforage/1.4.3/localforage.js' );


might be useful to define this in an actual js file and pull that in using webpack's raw loader. maybe with a .worker-js extension or something to avoid the js / babel loader?

might be useful to define this in an actual js file and pull that in using webpack's raw loader. maybe with a .worker-js extension or something to avoid the js / babel loader?

Yeah, I'm thinking we'll probably have to create a separate bundle that includes the worker handler logic and which depends on our lib/localforage module. Most examples of workers load from a standalone script file, not generated as a Blob like here.

If we don't end up using lib/localforage, let's at least use a named db. The default will use localforage:

blowery · 2016-12-12T14:47:11Z

This is really cool.

gwwar · 2016-12-12T20:05:05Z

client/lib/state-persistence/enhancer.js

+	maxAge: 604800000
+};
+
+const DESERIALIZE_INIT = '@@DESERIALIZE_INIT';


Maybe we should namespace it to @@calypso/DESERIALIZE_INITto make it clear that it's our internal event that should not be handled.

reduxjs/redux#186

gwwar · 2016-12-12T21:10:59Z

Perform persistence inside a Web Worker instance

Interesting idea here. We played around a bit with service workers last year at a meetup, but it didn't have the best browser support, and was a nightmare to try and update the scripts. So we ended up with a lot of zombie workers. (It was very heavily cached). With that thought, would changes to the web worker ever require us to flush our local cache? If so, perhaps we should have some mechanism in place to detect this.

Allow application to render prior to persisted state loading, queuing dispatched actions to be replayed after persisted state restored

Very cool stuff.

I haven't looked at web workers in a while, but one more thing to consider is server state for SSR pages. Currently this is being passed down on the window object, which I believe the workers do not have access to. Server state should override any cached local state, but should defer to any fresh data. I'll need to step through this, but we should take care not to introduce any regressions here.

Apply per-key state persistence TTL (time-to-live)

This makes a lot of sense, though we'll likely need to perform an audit and make sure our current data structure can handle this. eg, making sure parts of the tree don't make assumptions on what's available, and that we don't load an inconsistent application state.

gwwar · 2016-12-12T21:18:14Z

Perhaps localForage will have a reasonable fallback for this case inside the worker?

For Safari it sounds like we can only rely on message passing and we do not have access to localStorage/indexedDB/webSQL. Some folks also worked around this by using a web worker polyfill.

localForage/localForage#541

dmsnell · 2017-04-07T10:49:02Z

After some inclinations that the actual serialization was a major source of delay in the persistence I had another idea I would like to explore: a tradeoff between lag and memory consumption…

Instead of serializing the state tree in the main thread, why not simply create a duplicate store in a web worker and send every single Redux action across the gap to that persister thread. it could batch those actions and then replay them at intervals large enough to fit the serialization and storage too (for example, we could queue the actions and then fire them off in succession every 1000ms, then store the tree to localStorage when it's done (would give the worker thread 1000ms to serialize and store) and it would do all this without blocking the UI

core to this idea is the suspicion that we can significantly reduce the lag introduced if we serialize hundreds of tiny objects (Redux actions) spread out over time vs. serializing a huge object (Redux state tree) at a single point in time.

my plan has been to code up a PR but we see if that ever happens…

aduth · 2017-04-07T12:41:58Z

I think that's a really neat idea @dmsnell . My main concern which you noted would be implications of memory usage of having two copies of the store. We should do some measurements to see just how large the state object is after some common usage, because it seems.... big.

Another thing I'd meant to look at in getting back around to this is easier integration of web workers with worker-loader, allowing a standalone file with its own dependencies to be the worker source.

dmsnell · 2017-06-21T10:28:27Z

@aduth what's the status of this PR? I've noticed that it is large in the sense that it accomplishes several goals and it's out of date. are there ways we could spit it into smaller pieces? what implications does @gwwar's work on opt-in persistence bring here?

aduth · 2017-06-21T13:06:28Z

@dmsnell I'll close this as being severely out-of-date. There is still value in the objectives here, but they could be split into separate tasks, reflected by items noted in the original comment:

Perform persistence inside a Web Worker instance

Allow application to render prior to persisted state loading, queuing dispatched actions to be replayed after persisted state restored

Apply per-key state persistence TTL (time-to-live)

Extract persistence logic to standalone library module

I don't believe that opt-in persistence has any notable impact on any of these goals.

aduth added 2 commits December 9, 2016 18:50

Framework: Create statePersistenceEnhancer Redux store enhancer

6dbcbce

State: Use statePersistenceEnhancer in place of initialState

8ff5e8a

aduth added Framework State [Status] In Progress labels Dec 10, 2016

aduth requested a review from gwwar December 10, 2016 00:00

blowery reviewed Dec 12, 2016

View reviewed changes

gwwar reviewed Dec 12, 2016

View reviewed changes

dmsnell self-requested a review December 14, 2016 00:39

nb mentioned this pull request Jan 5, 2017

Calypso Framework: Increase throttle for serializing state #10435

Merged

aduth mentioned this pull request Jan 19, 2017

State: Push the reducer tree and createReduxStore to a chunk #10673

Closed

3 tasks

matticbot added the [Size] XL Probably needs to be broken down into multiple smaller issues label Mar 30, 2017

aduth closed this Jun 21, 2017

aduth deleted the update/state-persistence-web-worker branch June 21, 2017 13:06

matticbot removed the [Status] In Progress label Jun 21, 2017

gwwar mentioned this pull request Dec 12, 2017

Framework: Load extension reducer from it's section bundle (alternative proposal) #20542

Closed

snyk-bot mentioned this pull request Nov 19, 2020

[Snyk] Fix for 1 vulnerabilities awaisab172/wp-calypso#49

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

State: Refactor state persistence #9957

State: Refactor state persistence #9957

aduth commented Dec 10, 2016 •

edited

Loading

matticbot commented Dec 10, 2016

aduth commented Dec 10, 2016

blowery commented Dec 12, 2016 via email

aduth commented Dec 12, 2016 •

edited

Loading

aduth commented Dec 12, 2016

blowery Dec 12, 2016

aduth Dec 12, 2016

gwwar Dec 12, 2016

dmsnell Apr 7, 2017

blowery Dec 12, 2016

aduth Dec 12, 2016

blowery Dec 12, 2016

aduth Dec 12, 2016 •

edited

Loading

gwwar Dec 12, 2016

blowery commented Dec 12, 2016

gwwar Dec 12, 2016 •

edited

Loading

gwwar commented Dec 12, 2016 •

edited

Loading

gwwar commented Dec 12, 2016 •

edited

Loading

dmsnell commented Apr 7, 2017

aduth commented Apr 7, 2017

dmsnell commented Jun 21, 2017

aduth commented Jun 21, 2017

State: Refactor state persistence #9957

State: Refactor state persistence #9957

Conversation

aduth commented Dec 10, 2016 • edited Loading

matticbot commented Dec 10, 2016

aduth commented Dec 10, 2016

blowery commented Dec 12, 2016 via email

aduth commented Dec 12, 2016 • edited Loading

aduth commented Dec 12, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aduth Dec 12, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blowery commented Dec 12, 2016

gwwar Dec 12, 2016 • edited Loading

Choose a reason for hiding this comment

gwwar commented Dec 12, 2016 • edited Loading

gwwar commented Dec 12, 2016 • edited Loading

dmsnell commented Apr 7, 2017

aduth commented Apr 7, 2017

dmsnell commented Jun 21, 2017

aduth commented Jun 21, 2017

aduth commented Dec 10, 2016 •

edited

Loading

aduth commented Dec 12, 2016 •

edited

Loading

aduth Dec 12, 2016 •

edited

Loading

gwwar Dec 12, 2016 •

edited

Loading

gwwar commented Dec 12, 2016 •

edited

Loading

gwwar commented Dec 12, 2016 •

edited

Loading