fix(rum): sweep the settings entries of releases nobody runs - #43
Merged
Conversation
The settings cache is keyed by application version, because two releases served at the same time are entitled to different rates and one entry between them would have each overwrite the other's at every fetch. The cost of that was an entry per release: nothing ever read or removed the one a previous release used, so on a site that deploys often they accumulated for good in a quota the host application shares. Every write now stamps when it happened, through the one path that writes an entry, and initialisation removes the entries nothing has refreshed for two days. Age is the only thing that can tell an abandoned entry from the entry of a tab still open on yesterday's release: a page that still reads its entry rewrites it at every session renewal, so the threshold only has to clear the longest session plus the longest outage worth surviving. The sweep runs before the first request rather than after each write. A session renewal is a hot path and localStorage is synchronous, and going first is what lets it free room on an origin that is already out of it - the very state the leak produces. This page's own entry is never a candidate: it holds the version floor that lets a late answer be refused. The one path that reaches an entry without storing anything - a response refused for carrying an older version - now rewrites it unchanged, so the entry a client is still asking for cannot be swept out from under it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The remote-configuration cache is keyed by application version, so every release of a customer's site leaves one
localStorageentry behind. Nothing ever read or removed it again, and on a site that deploys often the entries accumulated in a quota the page shares with the host application.Entries written from this release onward carry when they were last written, and initialisation removes the ones nothing has refreshed for two days. Entries written by older SDK builds carry no write time and are retained: an older build still running on the origin cannot add one, so absence alone cannot distinguish a live release from an abandoned one. This leaves a finite legacy residue while preventing further unbounded growth.
Why age, and why two days
Age is the only thing that can tell an abandoned timestamped entry from the entry of a tab still open on yesterday's release. A page that still reads its entry rewrites it at every session renewal, so the threshold only has to clear the longest a live entry can legitimately stay silent: the longest session (four hours, after which a renewal refetches) plus the longest endpoint outage worth surviving, since a failed fetch stores nothing. Erring long is deliberate — deleting an entry too early costs the page reading it one session on its init values, while keeping a dead one costs a few hundred bytes.
Why at initialisation rather than on write
A session renewal is a hot path and
localStorageis synchronous, so repeating the walk at every write buys no new information. Going first also lets the sweep free room before the response it is about to store.Three kinds of entries are never candidates:
_fc_draw_1_) and whatever else the host application stores on the origin are left alone.The path that stores nothing
A response refused for carrying an older version was the one way to reach an entry without writing it. It now rewrites the entry unchanged, so a client left there by a server that broke the only-goes-up contract cannot have the settings it is still asking for swept out from under it.
Tests
Seven specs cover removal past the threshold, retention of a recently refreshed entry, retention of an entry without a write time, the current page's entry surviving whatever its age, foreign and draw-record keys being left alone, the write time being recorded, and the refresh on a refused response.
Full unit suite: 2991 passing, 0 failing.
typecheck, ESLint, and Prettier are clean.