fix: collapse a bulk invalidation's URL-map writes into one per domain - #4
Merged
Merged
Conversation
Statamic keeps every cached URL for a domain in a single cache entry, and AbstractCacher::invalidateUrls() walks the list one URL at a time. Each call reads that entry, and each match calls forgetUrl(), which reads it again and writes the whole thing back. Clearing n URLs out of a map of m cost O(n*m). That is the graph's own precision turned into work: targeted invalidation hands the cacher a long list of individually correct URLs, which is the point, and the per-URL rewrite makes a longer list quadratically more expensive. On a site with a couple of thousand cached URLs the map is a few hundred kilobytes, every write takes an exclusive lock on one file, and live traffic rewrites the same entry on every uncached render. The queued Invalidate job ran past its timeout, was killed part-way through the list, and every URL it had not reached went on serving the old page. Nothing reported it: the only trace was a TimeoutExceededException in failed_jobs, while the editor saw a saved entry that never appeared. Buffering getUrls() for the duration of a pass leaves the parent's matching, response deletion and event dispatch exactly as they are — nothing here knows what a cached response looks like — while turning m reads and writes into one read and one write per domain actually touched. Also stop handing the cacher URLs that are not cached any more. The graph outlives the cache by design, so a tag routinely resolves to two or three times as many URLs as the cache holds, and each of those costs a lookup that can only miss. A cacher whose contents cannot be enumerated still receives the full set: an empty list there means "unknown", not "nothing is cached", and narrowing against it would clear nothing at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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 symptom
Editors save an entry and the page does not change. Another entry saved in the same minute updates fine. Nothing in
laravel.log.The trace is in
failed_jobs. On the site this was found on: 47 failed jobs over 12 days, every one of themStatamic\StaticCaching\Invalidatefailing withTimeoutExceededException, with the stack ending inApplicationCacher::invalidateUrl().A job that dies part-way through the URL list clears the URLs it reached and leaves the rest stale. That is why it looks arbitrary from the outside — it depends only on where in the list the worker ran out of time.
The cause
Statamic keeps every cached URL for a domain in one cache entry, and
AbstractCacher::invalidateUrls()walks the list one URL at a time:Clearing n URLs out of a map of m costs O(n*m). Measured on the affected site:
So a single save rewrote 230 KB roughly 1,400 times, each write taking an exclusive lock on one file that live traffic is also rewriting on every uncached render. Idle, that is ~7s; under traffic it went past the worker's 60s timeout.
This is the graph's precision turned into work. Targeted invalidation hands the cacher a long list of individually correct URLs — that is the whole point — and the per-URL rewrite makes a longer list quadratically more expensive.
The fix
1. Buffer the URL map for the duration of a pass (
BatchesInvalidation, used by both tracking cachers).getUrls()is served from a per-domain buffer while a batch is in flight andforgetUrl()mutates that buffer; the map is written once per domain at the end, and only if something actually changed. The parent's matching, response deletion and event dispatch are untouched — nothing in the trait knows what a cached response looks like, which is why it works for theapplicationandfilecachers alike.Wildcards, query-string variants, multiple domains and
refreshUrls()all go through the parent as before.2. Stop handing the cacher URLs that are no longer cached (
GraphInvalidator).The graph outlives the cache by design — rows are pruned as they are invalidated, not when a page falls out — so on the site above a tag resolved to 4,615 URLs of which 1,382 existed. The rest cost a lookup that can only miss.
One subtlety, and the reason this is not a plain
array_intersect:CachedUrls::all()returns[]both for "nothing is cached" and for "this cacher cannot be enumerated" (it gives up on anything that is not anAbstractCacher). Narrowing against an empty list would clear nothing at all on a host app's own cacher. An empty list is therefore read as "unknown" and the full set is passed through. The existing suite caught this — two wiring tests went red on the first attempt.Result
Tests
144 passing, 8 new. Both fixes mutation-checked — with the trait removed the batching tests report 40, 3 and 2 writes where they expect 1; with the narrowing reverted,
it_does_not_hand_over_graph_urls_that_are_no_longer_cachedfails.Known limitation, unchanged here
On the
fullstrategy, Statamic'sFileCacher::invalidateUrl()also scans its cache directory once per URL. That is its own code and a separate cost; this PR does not touch it. The map fix applies to both strategies.Changelog
2.0.0 has never been tagged as a stable release — only
rc.1…rc.7— so the[Unreleased]fixes from the RC cycle are folded into## [2.0.0]along with this one, and the date moved to today.🤖 Generated with Claude Code