WICG / background-sync

A design and spec for ServiceWorker-based background synchronization
https://wicg.github.io/background-sync/spec/
Apache License 2.0
639 stars 83 forks source link

Order of firing sync events. #130

Open lucas42 opened 8 years ago

lucas42 commented 8 years ago

Problem

It is unclear from the spec what order sync events should be fired in when a user agent comes online. It only says:

Whenever the user agent changes to online, the user agent SHOULD fire a sync event for each sync registration whose registration state is pending.

There is no reference to what order this will happen in. As a developer, my initial expectation is that the firing of sync events will reflect the order they were registered in. However, there's nothing in the spec to ensure this and my brief play with Chrome's implementation shows they use a different order (possibly alphabetical by tag?)

Example

Here's a worked example to show how this can trip someone up: a user can change the theme of their app whilst offline, and we'd like to sync that setting back to the server so it can be used on their other devices. They change it three times whilst offline and we register the following tags after they make the corresponding change:

When the user agent comes online again, a PUT request is made to a RESTful API for each change. Each request overwrites the previous value, so the user ends up with the theme sent in whichever request comes last. The order here becomes important, and whilst I know variable network conditions can cause race conditions, having an unpredictable order coming from the client just makes that worse. Respecting the order they were registered in results in a blue theme, alphabetical sorting results in a yellow theme, and another sorting algorithm could give a red theme.

Solutions

Please could the spec be changed in one of two ways:

(Personally I'd prefer the first)

jkarlin commented 8 years ago

Thanks for the feedback. I agree that some clarification text is in order.

Requiring that events fire sequentially raises several concerns. If several events are queued, and they each run slowly, then you wind up with battery drain, privacy issues (due to duration of the sync), and starvation for later events. Also, what happens to later events if an event permanently fails? Chrome runs sync events in parallel for these reasons.

lucas42 commented 8 years ago

When you say chrome runs sync events in "parallel", you just mean it doesn't wait around for waitUntilPromise to be resolved, right? It still calls my sync event handler function sequentially, (in alphabetical order by tag).

This means I can't even do anything clever in the service worker to keep track of what happened as the service worker has no access to the order the events were registered.

jkarlin commented 8 years ago

Correct, the events are fired sequentially but continue to run in parallel.

You might find it useful to use a single tag name and store the data you need to transmit to disk. When the event fires, read whatever is available on disk and send it in order. We should add an example of this to the spec as well.

lucas42 commented 8 years ago

Yes, an example like that would be helpful. Presumably writing to disk would happen on the page registering the event, but the reading would happen in the service worker? I've not actually seen any examples of sharing state between the two.

Though I think most important would be something pointing out that the order of sync events can not be depended upon, even if your service worker is using synchronous APIs, such as localStorage.

jkarlin commented 8 years ago

@jakearchibald Jake, can you add an example like this to the spec? I believe this is the approach you've taken in your demos.

I'll add the bit about event order.

h4rikris commented 7 years ago

@jkarlin If we use single tag name to process multiple jobs/requests sequentially it might hit the time limit imposed on serviceWorker in case of too many requests. Also in low internet conditions processing sync events in parallel might slow down all sync events which might lead to serviceWorker termination (since sync events won't able finish within imposed time limits). On low internet conditions it might be good idea to run sync events in sequence rather than parallel.

jkarlin commented 7 years ago

@harikrishnakanchi It's true that you might run out of time, but that's to satisfy privacy/resource constraints. The API currently doesn't require a permission, and that's largely because we can cap the duration of all of the sync events.

h4rikris commented 7 years ago

@jkarlin Is there any other APIs available that requires a user permission and able to achieve background sync? I know periodicSync (in design) requires user permission. Don't know whether periodicSync API suits for this use case or not. Is there any other APIs that are coming in to achieve this kind of use cases?

jkarlin commented 7 years ago

Push messaging is one example that could be used to fire again in the future if necessary. I'm not aware of anyone pushing periodicSync or other background sync enhancements at the moment.

@clelland or @jakearchibald might know more

h4rikris commented 7 years ago

On the topic of push messaging, even if we fire a push event it also should have time limit right? Since it could also lead to privacy and resource concerns.

jkarlin commented 7 years ago

It does have a time limit, but you could push again if necessary.