Repository navigation
feat(platform): store knowledge vectors in a table per width - #4445
Conversation
|
REQUEST CHANGES — exact head d519ccc [blocker][DATA] Rolling rollback loses vectors. In The per-width joins and schema-qualified tables otherwise preserve corpus/tenant scoping, and the migration copy/FK cascades are mostly idempotent for supported widths. HNSW is created only through 1536; wider tables scan, while the synchronous copy/index build doubles storage and can hold locks for corpus-scale time. CI evidence at this SHA: Backend integration passed. The three failures are Knip ( |
d519ccc to
decdfdd
Compare
|
@yannickmonney Addressed in 235162d: the new writers now also write While playing the previous image in the 7fea8f5 drops the export Knip flagged, which this PR had orphaned. The Browser failure was the dashboard shell test fixed on main in #4446; the branch is rebased onto it. |
|
ACCEPT at head 7fea8f5.\n\nDATA blocker CLOSED. The migration creates the per-width tables before copying legacy vectors and installs the legacy mirror trigger (private migration lines 51-66, 68-105); the new writer also mirrors the legacy width (indexing.ts lines 603-608). The regression lane covers the required sequence: old image pins chunks.embedding at 256, new image indexes Acme at 256 and Globex at 384, the old image reads Acme during the roll/back after rollback, an old-image write is mirrored into chunk_vectors_256, then Acme moves to 384 and clears the legacy column (vector-width.integration.ts lines 262-321). The old code would fail the two-width same-database assertion and the mirrored-write/previous-reader assertions. Forward-only down sections preserve vectors.\n\nDATA/SECURITY delta review:\n- Forward-only and existing-row safe: both migrations use CREATE TABLE IF NOT EXISTS, copy existing rows with conflict protection, and retain chunks.embedding for the roll. The down migration is deliberately empty.\n- Mixed-version readers/writers: covered by the old-column pin/read/write and trigger assertions above; unsupported legacy widths remain in the old column with an explicit warning and require re-indexing.\n- Tenant isolation: private queries bind org_slug and the chunk/document relationship; public-web queries join website_org_memberships. The integration creates two organizations on one database and asserts each sees only its own width/document. Per-width tables have chunk foreign keys with cascade deletion.\n\nCICD CI ready aggregators at this head: Build — success; CLI — success; E2E — success; Checks — success; SAST — success; Commitlint — success; Security — success.\n\nReal PostgreSQL execution is NOT_RUN in this coordination-only review. It must run the vector-width.integration.ts scenario against PostgreSQL with migrations 15/16 applied: two organizations sharing one database at 256/384, mixed old/new reader-writer roll and rollback with trigger mirroring, move one organization to the other width, verify no cross-tenant hits, and verify migration interruption/re-run leaves existing vectors intact. |
7fea8f5 to
bd645cf
Compare
The reaper finds a process that left an exec's group by the exec id in its environment. A process in the middle of an execve has no environment to read: from the moment the new image's memory replaces the old until its arguments and environment are laid out, the kernel answers an empty one (a probe reading /proc/<pid>/environ of a process that execs in a loop got an empty answer in 3,341 of 20,000 reads). A leftover is often right there when a round comes: `cmd &` and the command's exit are an instant apart, and `setsid cmd` is two execs. The SIGTERM round counted such a process as untagged and skipped it; only the SIGKILL round, SIGKILL_GRACE_MS later, ended it. That is how exec-completion.test.ts failed on CI (#4445, Checks run 37752204357): its final barrier gave the reaper five seconds to end the `setsid sleep 60` a finished exec left, and the SIGKILL round came a few milliseconds after that. The test now waits for the reaper's own bound (the SIGKILL grace plus the scan deadline), and the scan reads an empty environment again, four times ten milliseconds apart, before the process counts as untagged; a process whose environment really is empty costs a scan that long. No user-visible change: no messages, docs under docs/ or READMEs.
bd645cf to
7fa9fcc
Compare
The reaper finds a process that left an exec's group by the exec id in its environment. A process in the middle of an execve has no environment to read: from the moment the new image's memory replaces the old until its arguments and environment are laid out, the kernel answers an empty one (a probe reading /proc/<pid>/environ of a process that execs in a loop got an empty answer in 3,341 of 20,000 reads). A leftover is often right there when a round comes: `cmd &` and the command's exit are an instant apart, and `setsid cmd` is two execs. The SIGTERM round counted such a process as untagged and skipped it; only the SIGKILL round, SIGKILL_GRACE_MS later, ended it. That is how exec-completion.test.ts failed on CI (#4445, Checks run 37752204357): its final barrier gave the reaper five seconds to end the `setsid sleep 60` a finished exec left, and the SIGKILL round came a few milliseconds after that. The test now waits for the reaper's own bound (the SIGKILL grace plus the scan deadline), and the scan reads an empty environment again, four times ten milliseconds apart, before the process counts as untagged; a process whose environment really is empty costs a scan that long. No user-visible change: no messages, docs under docs/ or READMEs.
7fa9fcc to
912a6dd
Compare
The reaper finds a process that left an exec's group by the exec id in its environment. A process in the middle of an execve has no environment to read: from the moment the new image's memory replaces the old until its arguments and environment are laid out, the kernel answers an empty one (a probe reading /proc/<pid>/environ of a process that execs in a loop got an empty answer in 3,341 of 20,000 reads). A leftover is often right there when a round comes: `cmd &` and the command's exit are an instant apart, and `setsid cmd` is two execs. The SIGTERM round counted such a process as untagged and skipped it; only the SIGKILL round, SIGKILL_GRACE_MS later, ended it. That is how exec-completion.test.ts failed on CI (#4445, Checks run 37752204357): its final barrier gave the reaper five seconds to end the `setsid sleep 60` a finished exec left, and the SIGKILL round came a few milliseconds after that. The test now waits for the reaper's own bound (the SIGKILL grace plus the scan deadline), and the scan reads an empty environment again, four times ten milliseconds apart, before the process counts as untagged; a process whose environment really is empty costs a scan that long. No user-visible change: no messages, docs under docs/ or READMEs.
A knowledge database held every vector in one column of one declared width, narrowed by the first embedding model used with it. From then on a model of another width was refused for the whole database: an organization that changed its vector width had every document fail with "this knowledge database stores 1536-dimensional ones", even on an empty corpus, and organizations sharing the database had to agree on one width. Vectors now live beside the chunks, in a table per supported width (`chunk_vectors_<width>`: 256, 384, 512, 768, 1024, 1536, 2048, 3072, 4096), created by the knowledge-db migrations and never at runtime. An organization's vectors are written to, and searched in, the table of the width its model states, in both corpora. - Migrations 15 (private_knowledge) and 16 (public_web) create the tables, copy the existing vectors and build the HNSW indexes. `chunks.embedding` stays for one release: the previous image keeps using it while a deployment rolls, and a trigger copies what it writes into the table of that width. - The per-database pin is gone. A width with no table is refused by the settings form, by the save and by the indexer, naming the list; a vector that is not as wide as stated is still not written. - A document indexed under another width is embedded again from its first chunk, and an identical document is copied only when it is embedded at this width. - On a website several organizations added, each has its own vectors for the same chunks: a scan gives the stored chunks the missing width and leaves the other widths alone. - Saving the embedding settings re-queues the indexed documents and email bodies that have no vector of the saved width, as it already scans the websites in that state. - A chunk and its vector are written as two statements in one transaction: the BM25 index (pg_search) fails an INSERT that runs in a CTE nothing reads from, which is the case of a repeated passage. KNOW-R11 is rewritten and KNOW-R17 added; the docs and the settings copy follow in en/de/fr. The backend integration fixtures state a supported width (256), and a new lane proves two widths in one database, the move to another width and the re-queue on save.
The new writers stored a vector in the table of its width alone and left `chunks.embedding` NULL, while the previous image reads that column alone. During a rolling deployment, and after `tale rollback`, every document and page this image had indexed was missing from that image's search by meaning, with nothing reporting it. The migrations' trigger covered the other direction only. A vector is now written to the previous release's column as well whenever the column is declared at exactly that width (`legacyColumnWidth`, read from the catalog per document slice and per crawl link): the one width the previous image can read, and the one it refused every other model for. The document indexer carries it in the chunk row, and sets the column to NULL otherwise, so a vector of another width never outlives its text; the identical-document copy takes it along with the chunk; the crawler's page write and its vectorless sweep write it the same way. The width table's write replaces a vector that differs and touches nothing else, since the trigger mirrors the column into it first. The mirror trigger names no column any more. `UPDATE OF embedding` and a WHEN on the column made the column part of the trigger's definition, and Postgres then refuses the `ALTER COLUMN embedding TYPE vector(<width>)` with which the previous image pins an undeclared column on its first index — a hard failure for that image during the roll on a database nothing had pinned yet. The checks moved into the function, which also returns at once for an unchanged vector. The `checkVectorWidths` lane plays the previous image on real Postgres: pins the column as that image did, reads with that image's statement after this image indexed at the column's width, holds the column NULL for the organization at another width and after a move, and finds that image's own write in the width's table.
`isProgramLimitExceeded` answered the HNSW build above pgvector's dimension ceiling, which `pinDimensions` ran at runtime; the per-width tables build their indexes in the migrations, below the ceiling, and nothing calls it since. Knip flagged the export.
912a6dd to
09138a5
Compare
2 participants
Summary
A knowledge database held every vector in one column of one declared width, narrowed by the first embedding model used with it. From then on a model of another width was refused for the whole database. An organization that set Vector width to 1024 under Settings > Data residency had every document fail with "produced 1024-dimensional vectors, but this knowledge database stores 1536-dimensional ones", on a corpus that held no vector at all, and organizations sharing the database had to agree on one width.
Vectors now live beside the chunks, in a table per supported width, created by the knowledge-db migrations and never at runtime. An organization's vectors are written to, and searched in, the table of the width its model states.
chunks.embedding, one width per databasechunk_vectors_<width>, one table per width, in both corpus schemasWhat changes
private_knowledge) and 16 (public_web) create the tables, copy the existing vectors and build the HNSW indexes (none above pgvector's 2000 dimensions, as before). A guard test holds the tables the migrations create equal toKNOWLEDGE_VECTOR_WIDTHS.pinDimensions).dimensions.tsnow only routes a width to its table and keeps the check that a vector is as wide as stated.storePageTextno longer reportsvectorless: the sweep at the end of each link asks for the scanning organization's width, which also covers the pages fix(platform): render a crawled page again only when it changed #4439 no longer stores when they did not change.Found on a real database
The first version wrote a chunk and its vector in one statement, with the chunk's
INSERTin a CTE. On ParadeDB that fails for a repeated passage (push_insert_state: ExecutorRunEntry should have already been pushed onto the stack): a CTE nothing reads from runs after the main statement, where the BM25 index does not expect an insert. They are two statements in one transaction now. The unit doubles could not have shown it; the integration suite did.Rollout
chunks.embedding, its HNSW index andcreate_chunks_hnsw_index()stay for one release. The previous image keeps reading and writing them while a deployment rolls; a trigger copies what it writes into the table of that width, and this image writes the column too whenever it is declared at the width being written (legacyColumnWidth, read from the catalog per document slice and per crawl link) — the one width the previous image can read, and the one it refused every other model for. So what this image indexes is found by the previous image during the roll, and after a rollback. The trigger names no column: the previous image pins an undeclared column withALTER COLUMN … TYPE vector(<width>), which Postgres refuses for a column a trigger definition uses. The debt ledger has the entry for dropping all of it, the compatibility write included.Decisions a reviewer may want to overrule
Not in this PR
Checklist
bun run check: not green end to end on this machine. What ran on this tree:oxfmt --check(whole repo), conflict markers,lint:links,lint:manual, andturbo run lint typecheck test test:ui, where 45 of 49 tasks pass, including platform lint, typecheck and its 82,594 unit tests.@tale/platform#test:uifailed with 5 s timeouts in several suites while 49 tasks ran at once, and passes alone (630 files, 6,160 tests). Three tasks fail in workspaces this diff does not touch:@tale/cli#test(jqis not installed here),@tale/sandbox#testand@tale/sandbox-runtime-daemon#test.ruff format --checkdid not run (uvxis not installed); the diff has no Python file.bun run lint:sast(Opengrep): 0 findings on the files this PR changes, run throughtools/opengrep/run.shand again by the pre-commit hook. I did not run the whole-repository scan.services/platform/messages/{en,de,fr}/settings.yml:dimensionsHintanderrors.dimensionsInvalidreworded,sharedDbNoteremoved.de-CHoverrides none of them. The German and French are mine and have had no native review.docs/{en,de,fr}/self-hosted/configuration/data-residency.md.Test plan
bunx vitest --run --project server backend/core/knowledge backend/domains/knowledge backend/domains/websites tests/guardsinservices/platform: the routing of a width to its table and both refusals, the dense leg of both corpora reading one width, the indexer's stored state, copy and write per width, the crawler's sweep and its per-width page embed, the re-queue on save, the write door's refusal, and the migration guard ([KNOW-R11],[KNOW-R17]).bunx vitest --run --config vitest.ui.config.ts app/features/settings/data-residency: the form refuses a width outside the list and clears the width on a provider switch.bun run backend:integrationagainst a throwawaytale-dbcontainer with this branch'sservices/db/migrationsmounted, so dbmate itself applied migration 13 and the two of this PR (numbered 14 and 15 then; 15 and 16 since the rebase onto fix(platform): prevent unsafe crawl fast-path skips #4451, which took 14): 1,615 of 1,615 checks across 234 lanes, withoutITEST_REQUIRE_ALL_LANES. That run was on the tree rebased onto fix(platform): render a crawled page again only when it changed #4439; feat(platform): polish task agent UI and retry flows #4438 landed afterwards and touches none of these files. The fixtures now state a supported width (256 in place of 8). The newcheckVectorWidthslane proves, on the migrated tables:vector(1536), with and without vectors): the copy is exact, a write by the previous image is mirrored into the table of its width, deleting a chunk deletes its vectors, a vector of another width is refused by the table, and a second application changes nothing.DATA-F13added for the live flow.