style revalidation vs. runtime styling

[jfirebaugh]

Currently, a style loaded via URL is revalidated on a schedule defined by the caching-related headers on the HTTP response. If a revalidation indicates that the style has changed, a new style object is created, replacing the old one wholesale. This poses a challenge for the runtime styling API. In the current naive implementation, any mutations to the style via runtime styling APIs are discarded when revalidation results in a new style:

• Changes to style properties revert to the original values
• Custom sources or layers disappear
• Pointers and references to Source or Layer objects in the existing style are invalidated

All of these effects are likely to vex SDK users, many of whom are unaware of the revalidation behavior, and often not in control of the caching headers in any case. See #5581 for an early example.

To mitigate this, there are several approaches we could pursue:

• Document and keep the current behavior, but add a "style reloaded" event, with the intent that users attach a handler that restores their desired modifications whenever a reload occurs. This is the approach suggested in Sets the loading flag in the style request handler to ensure that the… #5581, but it doesn't strike me as very user-friendly because it doesn't address the root issue of the default behavior being surprising.
• Turn off revalidation of styles that have been mutated via runtime styling APIs. This is a relatively simple change which would avoid the surprising behavior, and likely not be detrimental to many users. The intersection of use cases that rely on both style revalidation and runtime styling is probably small. With this approach, we would hopefully still be able to do an initial revalidation when a map loads a previously-cached style, prior to any mutations.
• Turn off midstream revalidation of styles entirely -- like the above, but not conditioned upon use of runtime styling. The rationale for this is that revalidation of tiles is good, and revalidation of the style at initial load is good, but I don't know that anyone actually expects or depends on periodic revalidation of the style throughout the lifetime of a map. My impression is we implemented that more for consistency than user demand.
• Replace the wholesale reload with a "diff and patch" based approach, similar to that discussed in Efficiently apply style changes by diff-ing new and old styles (i.e. smartSetStyle) mapbox-gl-js#1989. This is a far more complex approach, with many unknown unknowns, and several thorny known unknowns like:
  • What do you do if a user mutates a style property, and then an update to that style also mutates that property?
  • What do you do if a user adds a layer referencing source S originally found in the style, and then an update to that style removes S?

I currently lean toward one of the two middle options.

[1ec5]

Document and keep the current behavior, but add a "style reloaded" event, with the intent that users attach a handler that restores their desired modifications whenever a reload occurs.

This would require the developer to maintain their own representation of the sources, layers, and styles independently of the platform wrappers around Source and Layer. Developers are very reluctant to maintain redundant state. On iOS, we get pushback whenever we ask developers to hang onto their MGLAnnotations and MGLAnnotationViews for any reason.

Turn off revalidation of styles that have been mutated via runtime styling APIs.

Turn off midstream revalidation of styles entirely -- like the above, but not conditioned upon use of runtime styling.

Either of these sounds great to me. The main use case for refreshing a style would be that the developer has published an update to their style in Studio and wants to see it reflected in the client application. (But anyways, the real answer for that need would be an explicit reload mechanism rather than an implicit one that runs on a timer.)

[tmpsantos]

Sounds good to me. This will also align with the behavior of setStyleJSON that is not network request based and thus will not reload the style.

[brunoabinader]

Turn off midstream revalidation of styles entirely

Good insight @jfirebaugh - I ought to prefer this option as it involves less complexity and avoids corner cases.

[PaulZed]

I think adding a "style reloaded" event is a mistake, since it seems redundant and just bloats the API.

"Turn off midstream revalidation of styles entirely...", makes the most sense to me for exactly the reasons you mention. Revalidate upon initial load, but I think relying on periodic revalidation is a mistake. If the goal is to support live-authoring styles, or to support live-updating customer maps, then I think that feature should be implemented explicitly, not hidden in the request handler for the style asset.

"Diff and patch" is really more of an optimization on top of reloading the style. It doesn't really answer the question of "should we be reloading the style on revalidation?". If the answer to that question is "we should reload only what changed", then i think the "correct" thing to do is probably reload the whole style now, and optimize the style updates later.

[jfirebaugh]

Sounds like the consensus is converging on revalidating the style once, at load, and then not revalidating while in use. @kkaefer does this strategy sound ok to you?

[kkaefer]

add a "style reloaded" event

I don't think this is the way to go either, since it means users have to keep track of all of their edits.

revalidating the style once, at load, and then not revalidating while in use

This sounds good, but one thing to consider is the fully-offline use case: We are currently relying on loading expired/outdated/stale styles from the cache with the expectation that they could be updated later one once the user gained internet connectivity. Disabling style modifications until the refresh succeeded is awkward since that could potentially never happen, and allowing style modification before the initial refresh happens puts us into the same position.

[PaulZed]

revalidating the style once at load...

I don't think we would wait for the revalidation to succeed. We would attempt a reload when the style is first loaded, but if it fails then we would not retry when the user gets connectivity. I don't think we should ever disable runtime edits to the style, but if we can't guarantee that we can replay changes on top of a refreshed style due to potential differences, then we should never implicitly cause a style reload that would destroy runtime changes by the user. If the user made their own changes to the style, I think it's safe to say that for that particular user the local changes are more important than any possible remote changes. An explicit style change would initiate a new request, which would cause the correct didFinishLoading events to fire, so if the user wanted to try to update their style they have the option of re-requesting it manually.

[jfirebaugh]

OK, so:

1. Load style from (possibly stale) cache.
2. Start revalidation.
3. If style is modified before revalidation finishes, allow revalidation to finish so that the updated style is cached for future sessions, but don't actually use it in the current session.
4. If style is not modified before revalidation finishes, reload with the updated style.
5. As long as the style is not modified, we have the option of continuing to revalidate, or not. I lean towards doing it only once.

[PaulZed]

I like it. FWIW I also lean towards revalidating once.

[incanus]

Another vote for:

revalidating the style once, at load, and then not revalidating while in use

Wasn't this:

My impression is we implemented that more for consistency than user demand.

Because of wanting the ability to refresh annotation-like objects on the map midstream by way of backend data updates changing the style? If so, is that use case met sufficiently by the runtime style API now?

[jfirebaugh]

@incanus Data updates typically happen via revalidation of tiles or (less commonly) GeoJSON source URLs. I'm not aware of any use cases that require style revalidation to update data.

[incanus]

You are right @jfirebaugh, I was confusing style and tile here.