Prevent deleted documents from being resurrected in the runtime cache - #8567
Open
bjorkert wants to merge 2 commits into
Open
Prevent deleted documents from being resurrected in the runtime cache#8567bjorkert wants to merge 2 commits into
bjorkert wants to merge 2 commits into
Conversation
A dataloader query in flight while a delete commits can merge stale results into the freshly flushed cache. The deleted document then stays in ddata until the retention period expires or the server restarts, because incremental loads only cover the last 15 minutes and never revisit its time window. Connected and freshly loaded clients keep rendering the deleted treatment. Track a removal generation per datatype in the cache and retry the entries, treatments, and devicestatus loads when a removal lands while their query is in flight.
Publishes ghcr.io/bjorkert/nightscout on push to this branch, multi-arch amd64+arm64. Builds each architecture on a native runner and merges the manifest, rather than building both under QEMU: this repo's npm ci compiles native modules, and emulated arm64 takes the better part of an hour against a few minutes natively. Tags come from the version already in package.json plus the commit SHA. Deliberately no version-bump commit like the other image repos have, since this is a fork that keeps merging upstream and rewriting package.json in CI would conflict on every merge.
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
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.
The runtime cache is flushed when documents are deleted through the v1 API, but a dataloader query already in flight can read Mongo before the delete commits and merge its results into the cache after the flush. Because the merge only adds and updates, the deleted document comes back, and since incremental loads only cover the last 15 minutes it is never reconciled again until the retention period (60 hours for treatments) expires or the server restarts. Connected and even freshly loaded clients keep rendering the deleted treatment, while the REST API (reading Mongo directly) shows it gone.
Observed in production with Trio, which deletes and re-uploads an override treatment when its duration changes: the original entry, uploaded with a 30 day placeholder duration, stayed on the chart as a never ending exercise event for every client, while Mongo only contained the corrected entry.
The window is real but narrow: reproducing the sequence against a running server (flush plus reload, then a delete fired 0 to 55 ms into the reload query) resurrected the deleted treatment in 5 of 24 attempts on current dev, always when the delete landed within about 4 ms of the reload starting.
The cache now tracks a removal generation per datatype. The entries, treatments, and devicestatus loaders capture it before querying and retry the load when a removal landed while the query was in flight, instead of merging results that may contain deleted documents. With the change applied, the same reproduction stays clean across 54 attempts, including 30 aimed entirely at the hot window.
The new regression test in tests/dataloader.test.js replays the interleaving: the treatments query returns a document, the delete flushes the cache before the merge runs, and the loader is expected to reload rather than resurrect it.