fix: Metadata record optimization - #33
Merged
Merged
Conversation
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.
Fixes two problems with
PATCH /api/collections/:owner/:slug/metadataon large collections.1. The endpoint reported failure on writes that succeeded
Building a metadata patch version takes longer than 100s at a few million records, so Cloudflare
returned
524. Nothing aborts the handler on client disconnect, so the write then committed anyway.This has already happened on dev: a
524to the caller, and a complete patch version created a coupleof minutes later. A caller who retried raced the version that did get created and failed on the
(collection_id, semver)unique constraint after tens of minutes of work.?async=truereturns202 {job_id}; pollGET …/metadata/jobs/:jobIdforrunning/completed/failed. Modelled on the existing async negotiate commit.metadata_jobstable. Stranded jobs are failed out bytool:cleanupSessions, already on cron.409with the in-flightjob_idinsteadof racing for the same semver.
Async is opted into by query param only, deliberately not a body flag as negotiate allows: here the
body is the metadata, so an
asynckey would be merged in and persisted.Retrying the same edit after a silent success is already safe —
latesthas moved on, the metadatamatches, and the existing unchanged-check returns
{unchanged: true}before doing any work.2. A metadata edit copied the entire record set
A patch version has, by definition, the same records as its base, but we wrote it a full private copy:
one
version_recordsrow per record. On a collection of a few million records that is roughly 2 GB withindexes, spent to correct a readme, and again on every subsequent edit.
versions.records_from_version_id. A metadata patch points at the version that owns the rowsinstead of copying them: one row written instead of millions.
ON DELETE RESTRICT, so deleting a version whose rows others share fails loudly rather than silentlyemptying them.
recordsVersionId(). Record bodies were never duplicated;record_objectsis unchanged.recordsFromVersionIdis a required field onrecordsVersionId(), so any version fetched with aprojection missing the column is a compile error rather than a silently empty record set. That caught 6
real cases during the sweep.
Two read paths needed judgement rather than mechanical resolution: the record provenance list now
OR-joins on the pointer (otherwise patch versions vanish from a record's provenance), while theaccess-control and file-reference joins deliberately stay ownership-only — they are collection-scoped,
and the owning version is always in the same collection. Both are commented in place.
The pointer is internal storage detail and is stripped from API responses.
Migrations
Both additive — no table rewrite, no backfill.
0013—metadata_jobs0014—versions.records_from_version_id(nullable + self-FK)Existing versions keep
NULLand go on owning their rows.Testing
tsc --noEmit,oxlint,oxfmt --checkclean; 97 unit tests pass.pnpm tool:verifyRecordSharing— 13 assertions against real Postgres, including that theunresolved query returns 0 (the failure mode is confirmed, not assumed),
RESTRICTblockingdeletion of a shared base, and chained edits staying one hop. Kept out of
pnpm test, which is pureunit tests that run anywhere.