fix(media): run the filesystem index in the background - #111
Merged
Conversation
The index could never complete over HTTP. It walks ~145k filesystem entries on
the real corpus and takes minutes, but it ran inside the request on
r.Context(), so the first proxy to give up killed it: Nginx cuts an idle
upstream read at 60s (observed: status=500 duration_ms=60045, surfacing to the
client as Cloudflare's 504), and Cloudflare would cut in at ~100s regardless.
The walk aborted cleanly and committed what it had — 6485 of ~24k originals —
but no amount of retrying could finish it, and the Media -> Reindex button was
unusable by construction.
POST /v1/media/index now starts the walk on a background context that outlives
the request and returns 202 immediately; GET /v1/media/index reports
{running, started_at, finished_at, progress, error}. A second POST while a run
is in flight returns 409 rather than starting a duplicate. Runs are capped at
two hours so a wedged filesystem cannot leave the job permanently "running" and
block every later attempt.
Progress counts entries walked, not files indexed: the corpus is mostly
WordPress derivatives skipped before any stat, so an indexed-file counter
appears frozen for long stretches. MediaIndexResponse gains Walked for this.
The frontend polls every 2s and shows live counts in the button. A 409 is
treated as "join the run in progress" rather than an error, so clicking Reindex
while one is already going attaches to it.
Also fixes a real bug this surfaced: refresh() was setSearch(current => current),
which React bails out of, so the list never actually reloaded after an upload or
index. It now bumps a token the load effect depends on.
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 media index could never complete over HTTP. Fixes the 504 seen on the first
production reindex attempt.
The bug
The index walks ~145k filesystem entries on the real corpus and takes minutes,
but it ran inside the request on
r.Context(). The first proxy to give upkilled it:
That is Nginx's default
proxy_read_timeoutof 60s — Cloudflare would have cutin at ~100s regardless. The walk checked
ctx.Err()and aborted cleanly, so the6,485 rows it had inserted were committed and consistent, but no amount of
retrying could finish the job and the Media → Reindex button was unusable by
construction on a corpus this size.
The fix
POST /v1/media/indexstarts the walk on a background context that outlivesthe request and returns 202 immediately.
GET /v1/media/indexreports{running, started_at, finished_at, progress, error}.POSTwhile a run is in flight returns 409 instead of starting aduplicate.
runningand block every later attempt.WordPress derivatives that are skipped before any
stat, so an indexed-filecounter appears frozen for long stretches.
MediaIndexResponsegainsWalked.as "join the run already in progress" rather than an error.
Unrelated bug this surfaced
refresh()wassetSearch(current => current). React bails out of a same-valuesetState, so the effect never re-fired and the list silently never reloadedafter an upload or index. It now bumps a token the load effect depends on.
Testing
go build,go vet,go test ./...clean;tsc -b,eslint, and theproduction build clean.
failure recording, reset-on-restart, plus 409 and status handlers.
CMS_TEST_DSN-gated, run against real MariaDB 11.8)asserting the POST returns 202 in under 2s and the walk completes after the
request returned, with the derivative skipped and rows actually written by the
background goroutine.
agrees with the returned one, including on trees smaller than the callback
interval.
Operational note
deploy/README.mdupdated. Once this is deployed, Media → Reindex works from theUI; the partial index from the failed run is preserved and the rerun resumes,
skipping what is already there and keeping any alt text.