diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6c13878f..802e2d6a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -68,6 +68,39 @@ therefore hold on both — nothing may assume x86, and any binary fetched during resolve an arm64 asset too. ARM is not a niche here: a NAS or a Raspberry Pi is squarely the kind of machine plMail is meant to run on. +### The second Dockerfile, and the rule it exists to keep + +`docker/postgres/Dockerfile` builds PostgreSQL with pgvector. It is not part of the published image +and CI never builds it — an operator who wants it builds it locally through +`compose.pgvector.yaml`, natively on whatever architecture they are on, so the arm64 rule above +applies to it only in the sense that nothing in it may assume x86. + +Two rules govern it, and both have already cost somebody a day: + +**pgvector is optional and must never become required.** `plmail_embed_distance()` has two bodies — +a plpgsql loop and pgvector's `<=>` — chosen by the migration, in SQL, once. Every test, every +query and every code path has to work with either. If you find yourself writing PHP that branches on +whether the extension exists, the design has gone wrong; see +[docs/internals/ai-assist.md](docs/internals/ai-assist.md). + +**Nothing may change libc under an existing data volume.** That is why the image is built `FROM +postgres:${POSTGRES_VERSION}-alpine` instead of pulling `pgvector/pgvector:pg18`, which is Debian. +musl records no collation version, so `datcollversion` is `NULL`, so Postgres's own mismatch check +never fires — the swap was tested, and 48 collation-dependent indexes in plMail's own schema (24 of +them `UNIQUE`; 84 counting the system catalogues) are +walked with the wrong comparator, returning wrong rows with an empty log. Any future change to the +database image is subject to the same rule: same base, or it does not ship. + +`OPTFLAGS=""` in that Dockerfile is load-bearing for the same class of reason. pgvector's Makefile +defaults to `-march=native`, which bakes the build machine's instruction set into `vector.so`; +built on a runner with AVX-512 and run on a host without it, the backend takes SIGILL on the first +distance and the connection closes mid-query with nothing in the log. + +The unit and browser suites deliberately run against the **stock** image, so the shipped plpgsql +path is what CI exercises. To develop against pgvector instead, add the overlay to the commands in +[docs/install/upgrading.md](docs/install/upgrading.md) § "Turning on pgvector" — and re-run the +suite without it before opening a pull request. + ## Tests Two suites: PHPUnit for unit tests (`tests/`, mirroring `src/`) and Playwright for browser end-to-end diff --git a/compose.pgvector.yaml b/compose.pgvector.yaml new file mode 100644 index 00000000..cb45ea74 --- /dev/null +++ b/compose.pgvector.yaml @@ -0,0 +1,65 @@ +# pgvector, on the database image this stack already runs. Opt in with: +# +# docker compose -f compose.yaml -f compose.pgvector.yaml up -d --wait +# +# and keep both -f flags on every later `up`, `pull` and `down`, or compose goes +# back to the stock image on the next command. +# +# WHY THIS IS AN OVERLAY AND NOT A LINE IN compose.yaml +# ──────────────────────────────────────────────────── +# pgvector is not required and must never become required. The plpgsql distance +# function in Version20260826210000 is the shipped path and stays correct; this +# only makes it faster. Folding the build into compose.yaml would make every +# `git clone && docker compose up -d` compile a C extension before the database +# could start, for a feature most installs never switch on — and would take the +# database off a pinned upstream image for people who never asked for AI at all. +# +# The layering matches compose.demo.yaml: this goes on top of compose.yaml, not +# on top of compose.prod.yaml. It touches one service and changes nothing else. +# +# WHY IT IS BUILT LOCALLY RATHER THAN PULLED +# ────────────────────────────────────────── +# `pgvector/pgvector:pg18` exists and is the wrong answer here. It is Debian and +# the shipped image is Alpine, so pulling it swaps musl for glibc UNDER AN +# EXISTING DATA VOLUME. That was tested rather than reasoned about: index scans +# return wrong rows, with an empty log, because musl records no collation +# version — `datcollversion` is NULL, Postgres's mismatch check never fires, and +# 44 collation-dependent indexes, 18 of them UNIQUE, are walked with the wrong +# comparator. docs/install/upgrading.md says "Starting the new image IS running +# it", so a libc change may never arrive by pulling a tag. +# +# docker/postgres/Dockerfile builds FROM postgres:${POSTGRES_VERSION}-alpine and +# adds three files. Same libc, same server binaries, same on-disk format — +# nothing that compares existing data changes, so there is no index to rebuild +# and no dump-and-restore to schedule. That file carries the full reasoning. +# +# The build is about 30 seconds once and then cached, and adds ~1 MB to a 304 MB +# image. + +# The one place the image is named, and the args that decide what is in it. +# +# The tag carries BOTH versions on purpose. A locally built image is not pulled +# and not digest-checked, so a tag that said only `18` would go on pointing at +# whatever was built first: bumping PGVECTOR_VERSION would rebuild the layers, +# retag nothing an already-created container refers to, and leave the operator +# looking at the old extension while `docker compose config` swore otherwise. +x-pgvector-database: &pgvector-database + image: plmail/postgres-pgvector:${POSTGRES_VERSION:-18}-${PGVECTOR_VERSION:-v0.8.6} + build: + context: ./docker/postgres + args: + # Interpolated from the same variable compose.yaml uses for the stock + # image, so POSTGRES_VERSION=19 still moves the whole stack together + # instead of compiling against 18's headers and loading into 19. + POSTGRES_VERSION: ${POSTGRES_VERSION:-18} + # v0.8.1 is the earliest tag that compiles against PostgreSQL 18 — 0.8.0 + # fails on vacuum_delay_point()'s new parameter. This is pinned three + # releases later because 0.8.3 and 0.8.4 fixed HNSW index corruption and + # a broken graph repair during vacuum, and message_embedding is vacuumed + # constantly: it hangs off `message` with ON DELETE CASCADE and mail is + # deleted in bulk. docker/postgres/Dockerfile spells that out. + PGVECTOR_VERSION: ${PGVECTOR_VERSION:-v0.8.6} + +services: + database: + <<: *pgvector-database diff --git a/docker/postgres/Dockerfile b/docker/postgres/Dockerfile new file mode 100644 index 00000000..f2cf6f0c --- /dev/null +++ b/docker/postgres/Dockerfile @@ -0,0 +1,103 @@ +# syntax=docker/dockerfile:1 + +# PostgreSQL with pgvector, built ON TOP OF the image this stack already runs. +# +# WHY THIS FILE EXISTS AT ALL, INSTEAD OF `image: pgvector/pgvector:pg18` +# ────────────────────────────────────────────────────────────────────── +# The published pgvector image is Debian, and the shipped one is Alpine. Every +# byte of PGDATA on an existing install was written by a musl-linked server, and +# swapping to a glibc one under it is a silent-data-corruption event. That was +# tested here rather than reasoned about: +# +# The swap returns wrong rows from an index scan, with an empty log. musl +# records no collation version, so `datcollversion` is NULL, so Postgres's +# mismatch check never fires. 44 collation-dependent indexes — 18 of them +# UNIQUE — are silently walked with the wrong comparator. +# +# docs/install/upgrading.md says "Starting the new image IS running it": this +# stack applies migrations on boot, so an operator who pulls a tag has already +# run it by the time anything could warn them. A libc change is therefore not +# something that may ever ride in on a `docker compose pull`. +# +# So: same base image, same musl, same PostgreSQL binaries, same locale data, +# same on-disk format. The only difference between this image and +# `postgres:${POSTGRES_VERSION}-alpine` is three added files — vector.so and its +# two extension scripts. Nothing that reads or compares existing data changes, +# so there is no index to rebuild and no dump-and-restore to schedule. +# +# WHY IT CAN BE BUILT AT ALL +# ────────────────────────── +# `postgres:18-alpine` ships the server headers (`pg_config`, and +# /usr/local/include/postgresql/server/postgres.h), which the Debian image does +# not. That is what makes an in-image PGXS build possible without assembling a +# PostgreSQL source tree. + +# Kept in step with compose's own ${POSTGRES_VERSION:-18} and passed in from +# there, so `POSTGRES_VERSION=19 docker compose ... up` still moves both the +# builder and the runtime together. Two stages, one ARG, no way to compile +# against 18's headers and load into 19. +ARG POSTGRES_VERSION=18 + +# v0.8.6 and NOT v0.8.1, which is the earliest tag that compiles against +# PostgreSQL 18 — v0.8.0 fails in src/hnswvacuum.c because vacuum_delay_point() +# gained a `bool is_analyze` parameter in 18, and in src/ivfbuild.c on +# TupleDescData.attrs. v0.8.1 is where "support for Postgres 18" landed, and it +# builds and runs here. +# +# The three releases after it are the reason this is not pinned to that: +# +# 0.8.3 fixed possible INDEX CORRUPTION with HNSW vacuuming +# 0.8.4 fixed `hnsw graph not repaired`, and inserts erroring during a vacuum +# 0.8.6 fixed a buffer overflow in the IVFFlat build on 32-bit +# +# HNSW vacuuming is not an exotic path here: message_embedding has ON DELETE +# CASCADE from `message`, and mail is deleted in bulk, so autovacuum reaches +# that index constantly. Shipping the earliest version that merely compiles +# would mean shipping two known index-corruption bugs into the one place this +# project refuses to take that risk. +ARG PGVECTOR_VERSION=v0.8.6 + +FROM postgres:${POSTGRES_VERSION}-alpine AS build + +ARG PGVECTOR_VERSION + +# build-base is gcc, musl-dev, make. No clang or llvm: PGXS would use them to +# emit LLVM bitcode for JIT inlining, which costs ~1 GB in this stage and buys +# nothing — JIT inlining does not apply to an index scan, and a missing .bc file +# makes the JIT skip inlining silently rather than fail. +# +# `git` rather than a release tarball so the tag is verifiable by name and the +# clone is one layer; --depth 1 keeps it to the one commit. +RUN set -eux; \ + apk add --no-cache build-base git; \ + git clone --depth 1 --branch "${PGVECTOR_VERSION}" https://github.com/pgvector/pgvector.git /tmp/pgvector; \ + cd /tmp/pgvector; \ + # OPTFLAGS="" is NOT optional and NOT a tuning choice. pgvector's Makefile + # defaults it to -march=native on x86_64, which bakes the BUILD machine's + # instruction set into vector.so. An image built on a runner with AVX-512 + # and pulled onto a host without it does not degrade — the backend takes + # SIGILL the first time a distance is computed, which surfaces as the + # database connection closing mid-query with nothing in the log. + make OPTFLAGS="" with_llvm=no -j"$(nproc)"; \ + make OPTFLAGS="" with_llvm=no install + +FROM postgres:${POSTGRES_VERSION}-alpine + +# Three files, copied out of the builder so the toolchain does not ship. The +# runtime image is `postgres:${POSTGRES_VERSION}-alpine` plus these and nothing +# else — which is what makes the swap safe on a live data volume. +COPY --from=build /usr/local/lib/postgresql/vector.so /usr/local/lib/postgresql/vector.so +COPY --from=build /usr/local/share/postgresql/extension/vector.control /usr/local/share/postgresql/extension/vector.control +COPY --from=build /usr/local/share/postgresql/extension/vector--*.sql /usr/local/share/postgresql/extension/ + +# Proves at BUILD time that the .so loads into this exact runtime image, rather +# than at 3am on somebody's server. A missing symbol, a headers/binary mismatch +# or an OPTFLAGS slip all fail here. +RUN set -eux; \ + initdb_dir="$(mktemp -d)"; \ + chown postgres "$initdb_dir"; \ + su postgres -c "initdb -D '$initdb_dir' -A trust --no-sync -E UTF8" >/dev/null; \ + su postgres -c "pg_ctl -D '$initdb_dir' -o '-c listen_addresses= -k /tmp' -w start" >/dev/null; \ + su postgres -c "psql -h /tmp -d postgres -v ON_ERROR_STOP=1 -c 'CREATE EXTENSION vector' -c \"SELECT '[1,2,3]'::vector\""; \ + su postgres -c "pg_ctl -D '$initdb_dir' -m immediate -w stop" >/dev/null; \ + rm -rf "$initdb_dir" diff --git a/docs/de/install/upgrading.md b/docs/de/install/upgrading.md index 2779ca0f..711021d9 100644 --- a/docs/de/install/upgrading.md +++ b/docs/de/install/upgrading.md @@ -1,4 +1,4 @@ - + # Aktualisieren plMail migriert seine eigene Datenbank beim Start. Diese eine Entscheidung prägt alles auf dieser @@ -119,6 +119,100 @@ das, was `docker compose pull` liefert. `main` nennen. Der Commit im Chip ist das Einzige, was sie unterscheidet, und er ist der Grund, warum die Bezeichnung allein nicht genügt. +## pgvector einschalten + +Die semantische Suche vergleicht zwei Vektoren mit `plmail_embed_distance()`. Das ausgelieferte +Datenbank-Image hat keine `vector`-Erweiterung, also ist diese Funktion eine plpgsql-Schleife: +korrekt, und rund 0,107 ms pro Vergleich bei 1024 Dimensionen. Über die 2.000 Kandidaten, die eine +Suche betrachtet, sind das etwa 0,63 s Rechenzeit pro semantischer Suche. + +pgvector ersetzt die Schleife durch eine SIMD-Variante. Gemessen an 74.000 Zeilen mit 1024 +Dimensionen, PostgreSQL 18.4: + +| | 20.000 Zeilen | 74.000 Zeilen | +|---|---|---| +| plpgsql-Schleife | 2,14 s | 7,70 s | +| pgvector `<=>` | 1,23 s | 4,51 s | + +**1,75× — nicht die Größenordnung, die das Wort „SIMD" nahelegt.** Die Vektoren liegen als `real[]`, +also zahlt jede Zeile ein Detoast und eine Umwandlung `real[]` → `vector`, bevor überhaupt gerechnet +wird, und diese Umwandlung ist der größte Teil der Kosten. Es sind echte 0,63 s → 0,36 s im +semantischen Zweig einer Suche, und es ist nicht das Tausendfache, das ein Index wäre — warum der +Index eine eigene, größere Aufgabe ist, steht in +[den KI-Interna](../internals/ai-assist.md). + +Das ist optional. Ohne pgvector wird nichts schlechter, und danach muss nichts neu indiziert werden. + +### Warum plMail ein eigenes Image baut, statt `pgvector/pgvector` zu ziehen + +Weil dieses Image auf Debian beruht und dieser Stack auf Alpine, und weil das eine gegen das andere +zu tauschen die libc unter einem Datenträger austauscht, auf dem die Datenbank bereits jeden Index +geschrieben hat. Das wurde getestet: Index-Scans liefern falsche Zeilen, bei leerem Protokoll. musl +hinterlegt keine Collation-Version, also ist `datcollversion` `NULL`, also greift Postgres' eigene +Prüfung nie — und jeder collation-abhängige Index, 48 in plMails eigenem Schema, davon 24 `UNIQUE`, +und 84 samt der System-Kataloge, wird +stillschweigend mit dem falschen Vergleicher durchlaufen. + +`docker/postgres/Dockerfile` baut `FROM postgres:${POSTGRES_VERSION}-alpine` und fügt pgvector +hinzu. Gleiche libc, gleiche Server-Binaries, gleiches Format auf der Platte; das Image ist 1 MB +größer und sonst identisch. Es gibt nichts neu zu bauen und keinen Dump-and-Restore einzuplanen. + +### So geht es + +Nimm ab jetzt eine Datei in jeden compose-Befehl auf: + +```bash +docker compose -f compose.yaml -f compose.pgvector.yaml pull +docker compose -f compose.yaml -f compose.pgvector.yaml up -d --wait +``` + +Der erste Lauf kompiliert pgvector — etwa 30 Sekunden, einmal, danach aus dem Cache. **Behalte beide +`-f`-Flags in jedem späteren Befehl.** `docker compose up -d` allein liest nur `compose.yaml` und +setzt das Standard-Image zurück; das ist kein Fehler und erzeugt keine Warnung. + +Prüfe, ob es gegriffen hat: + +```bash +docker compose -f compose.yaml -f compose.pgvector.yaml exec database \ + psql -U app -d app -c "SELECT extversion FROM pg_extension WHERE extname = 'vector'" +``` + +### Die Reihenfolge zählt, und genau das ist der Fallstrick + +Die Migration, die den Funktionskörper umstellt, fragt die Datenbank, ob pgvector verfügbar ist — +und sie fragt **einmal**, bei dem Start, an dem sie zuerst läuft. Machst du beides in einem Befehl, +findet sie die Erweiterung bereits vor. Aktualisierst du erst und tauschst das Datenbank-Image +nächste Woche, hat sie schon mit „nein" geantwortet, ihre Zeile ins Verzeichnis geschrieben und +schaut nie wieder nach: `migrate` meldet „already at the latest version", und die Schleife bleibt. + +Wenn du an diesem Punkt bist, stelle die Frage neu: + +```bash +docker compose -f compose.yaml -f compose.pgvector.yaml exec php \ + php bin/console doctrine:migrations:execute --down \ + 'DoctrineMigrations\Version20260901120000' --no-interaction +docker compose -f compose.yaml -f compose.pgvector.yaml exec php \ + php bin/console doctrine:migrations:migrate --no-interaction +``` + +Diese eine Migration zurückzunehmen ist von Bauart sicher: Ihr `down()` installiert die +plpgsql-Schleife, die ohnehin schon installiert ist. Es wird nichts verworfen und keine Daten +angefasst. + +### Zurück + +Lass `-f compose.pgvector.yaml` weg und fahre den Stack hoch. Die Datenbank kehrt auf demselben +Datenträger zum Standard-Image zurück — wieder dieselbe libc, also wieder nichts neu zu bauen — und +`plmail_embed_distance()` behält den Körper, den sie gerade hat. Eine Funktion, die `<=>` gegen +einen Server aufruft, der diesen Operator nicht mehr hat, scheitert zur Abfragezeit; nimm die +Migration also in derselben Sitzung zurück: + +```bash +docker compose exec php php bin/console doctrine:migrations:execute --down \ + 'DoctrineMigrations\Version20260901120000' --no-interaction +docker compose exec php php bin/console doctrine:migrations:migrate --no-interaction +``` + ## Zurückrollen Es gibt keinen Downgrade-Pfad, und der Grund steht im ersten Abschnitt dieser Seite. Die Migrationen @@ -171,6 +265,15 @@ kaputte Installation. ändert, ohne die Container neu zu erzeugen, braucht anschließend Administration → System → Worker neu starten, sonst fragen die Worker weiter nach Spalten, die es nicht mehr gibt. +**Über pgvector wird einmal entschieden, bei dem Start, der seine Migration zuerst ausführt.** Das +Datenbank-Image danach zu tauschen ändert von sich aus nichts — die zwei Befehle, die die Frage neu +stellen, stehen unter [pgvector einschalten](#pgvector-einschalten). + +**`-f compose.pgvector.yaml` wegzulassen setzt das Datenbank-Image stillschweigend zurück.** Compose +warnt nicht, wenn ein Dienst, den es neu erzeugt, aus einer Datei stammt, die du nicht mehr +übergeben hast. Ist `plmail_embed_distance()` dann noch der pgvector-Körper, scheitert jede +semantische Suche an einem fehlenden Operator. + **`POSTGRES_VERSION` ist nichts, was man während eines Upgrades nebenbei anhebt.** Das Image liefert `postgresql-client-18` mit, und `pg_dump` weigert sich, einen Server zu sichern, der neuer ist als es selbst — ein über die Client-Version hinaus angehobenes Postgres macht also `app:backup` in dem diff --git a/docs/install/upgrading.md b/docs/install/upgrading.md index 1be8e01b..3089695f 100644 --- a/docs/install/upgrading.md +++ b/docs/install/upgrading.md @@ -108,6 +108,97 @@ arguments, or a checkout. Both are legitimate; neither is what `docker compose p `main`. The commit in the chip is the only thing that distinguishes them, and it is the reason the label alone is not enough. +## Turning on pgvector + +Semantic search compares two vectors with `plmail_embed_distance()`. The shipped database image has +no `vector` extension, so that function is a plpgsql loop: correct, and about 0.107 ms per +1024-dimension comparison. Over the 2,000 candidates a search considers, that is roughly 0.63 s of +arithmetic per semantic search. + +pgvector replaces the loop with a SIMD one. Measured on 74,000 rows of 1024-dimension vectors, +PostgreSQL 18.4: + +| | 20,000 rows | 74,000 rows | +|---|---|---| +| plpgsql loop | 2.14 s | 7.70 s | +| pgvector `<=>` | 1.23 s | 4.51 s | + +**1.75×, not the order of magnitude the word "SIMD" suggests.** The vectors are stored as `real[]`, +so every row pays a detoast and a `real[]` → `vector` conversion before any arithmetic happens, and +the conversion is most of the cost. It is a real 0.63 s → 0.36 s on the semantic arm of a search, +and it is not the thousandfold that an index would be — see +[the AI internals note](../internals/ai-assist.md) for why the index is a separate, larger job. + +This is optional. Nothing degrades without it and nothing needs re-indexing after it. + +### Why plMail builds its own image instead of pulling `pgvector/pgvector` + +Because that image is Debian and this stack is Alpine, and swapping one for the other under an +existing data volume changes libc beneath a database that has already written every index. It was +tested: index scans return wrong rows, with an empty log. musl records no collation version, so +`datcollversion` is `NULL`, so Postgres's own mismatch check never fires, and every +collation-dependent index — 48 in plMail's own schema, 24 of them `UNIQUE`, and 84 counting the +system catalogues — is silently walked with +the wrong comparator. + +`docker/postgres/Dockerfile` builds `FROM postgres:${POSTGRES_VERSION}-alpine` and adds pgvector to +it. Same libc, same server binaries, same on-disk format; the image is 1 MB larger and identical in +every other respect. There is nothing to rebuild and no dump-and-restore to schedule. + +### Doing it + +Add one file to every compose command from now on: + +```bash +docker compose -f compose.yaml -f compose.pgvector.yaml pull +docker compose -f compose.yaml -f compose.pgvector.yaml up -d --wait +``` + +The first run compiles pgvector — about 30 seconds, once, then cached. **Keep both `-f` flags on +every later command.** `docker compose up -d` on its own reads `compose.yaml` alone and puts the +stock image back, which is not an error and produces no warning. + +Confirm it took: + +```bash +docker compose -f compose.yaml -f compose.pgvector.yaml exec database \ + psql -U app -d app -c "SELECT extversion FROM pg_extension WHERE extname = 'vector'" +``` + +### The order matters, and it is the one thing that bites + +The migration that switches the function body asks the database whether pgvector is available, and +it asks **once** — on the boot where it first runs. Do both in one command and it finds the +extension already there. Upgrade first and switch the database image next week, and it has already +answered "no", written its ledger row, and will never look again: `migrate` reports "already at the +latest version" and the loop stays. + +If that is where you are, re-ask the question: + +```bash +docker compose -f compose.yaml -f compose.pgvector.yaml exec php \ + php bin/console doctrine:migrations:execute --down \ + 'DoctrineMigrations\Version20260901120000' --no-interaction +docker compose -f compose.yaml -f compose.pgvector.yaml exec php \ + php bin/console doctrine:migrations:migrate --no-interaction +``` + +Reversing that one migration is safe by construction: its `down()` reinstalls the plpgsql loop, +which is what is already installed. Nothing is dropped and no data is touched. + +### Going back + +Drop the `-f compose.pgvector.yaml` and bring the stack up. The database returns to the stock image +on the same volume — same libc again, so again nothing to rebuild — and `plmail_embed_distance()` +keeps whichever body it has. A function calling `<=>` against a server that no longer has the +operator fails at query time, so reverse the migration in the same session: + +```bash +docker compose exec php php bin/console doctrine:migrations:execute --down \ + 'DoctrineMigrations\Version20260901120000' --no-interaction +docker compose exec php php bin/console doctrine:migrations:migrate --no-interaction +``` + ## Rolling back There is no downgrade path, and the reason is the first section of this page. Migrations ran on @@ -153,6 +244,14 @@ wrong. Do not read a missing chip as a broken deployment. without recreating the containers needs Admin → System → restart, or the workers keep asking for columns that are gone. +**pgvector is decided once, on the boot that first runs its migration.** Switching the database +image afterwards changes nothing on its own — see [Turning on pgvector](#turning-on-pgvector) for +the two commands that re-ask the question. + +**Dropping `-f compose.pgvector.yaml` silently reverts the database image.** Compose does not warn +that a service it is recreating came from a file you no longer passed. If `plmail_embed_distance()` +is still the pgvector body, every semantic search then fails on a missing operator. + **`POSTGRES_VERSION` is not something to bump casually during an upgrade.** The image ships `postgresql-client-18`, and `pg_dump` refuses to dump a server newer than itself — so a Postgres raised past the client version breaks `app:backup` at the moment you next need it. diff --git a/docs/internals/ai-assist.md b/docs/internals/ai-assist.md index e7a22fda..8ec20202 100644 --- a/docs/internals/ai-assist.md +++ b/docs/internals/ai-assist.md @@ -70,7 +70,7 @@ The single request lives in `ClassifyMailHandler`, on a worker, after the transa for a subset. An ambiguous answer — two category names in one reply — is discarded rather than resolved by first match. -## Vectors: no pgvector, and that is the recommendation +## Vectors: pgvector is optional, and the image is built rather than pulled The shipped image is `postgres:18-alpine`, which has no `vector` extension. Moving to `pgvector/pgvector:pg18` means musl → glibc **on an existing data volume**, and that was tested @@ -80,30 +80,66 @@ rather than reasoned about: > version, so `datcollversion` is NULL, so Postgres's mismatch check never fires. 44 > collation-dependent indexes — 18 of them UNIQUE — are silently walked with the wrong comparator. -On a stack whose upgrade documentation says *"Starting the new image IS running it"*, that is not a -hazard worth accepting for a feature nobody has asked to be mandatory. A dump-and-restore migration -is immune, because it rebuilds every index under the new libc — but that is a manual, downtime -operation, not something to trigger by pulling a tag. +That number is now 48, of which 24 are UNIQUE — 84 and 59 counting the system catalogues, which the +wrong comparator walks too — and `datcollversion` is still NULL. Both re-checked +against the current schema. On a stack whose upgrade documentation says *"Starting the new image IS +running it"*, a libc change may never ride in on a pulled tag. **That route stays closed, and a +dump-and-restore is not offered as the default path either.** + +What replaced it is `docker/postgres/Dockerfile`: a two-stage build `FROM +postgres:${POSTGRES_VERSION}-alpine` that compiles pgvector and copies three files — +`vector.so` and its two extension scripts — into a copy of the same base image. Same libc, same +server binaries, same on-disk format, 1 MB larger. Nothing that reads or compares existing data +changes, so there is no index to rebuild. Opt in with `compose.pgvector.yaml`; the base stack is +untouched and pgvector never becomes required. + +Two things in that Dockerfile are load-bearing and easy to lose: + +- **`OPTFLAGS=""`.** pgvector's Makefile defaults to `-march=native`, which bakes the *build* + machine's instruction set into `vector.so`. An image built where AVX-512 exists and run where it + does not takes SIGILL on the first distance — the connection closes mid-query with nothing in the + log. +- **The pin is v0.8.6, not v0.8.1.** v0.8.0 does not compile against PostgreSQL 18 at all + (`vacuum_delay_point()` gained a parameter; `TupleDescData.attrs` went away), and v0.8.1 is the + earliest tag that does — verified by building every tag from 0.8.0 to 0.8.6 against + `postgres:18-alpine` rather than by reading the changelog. But 0.8.3 fixed *possible index + corruption with HNSW vacuuming* and 0.8.4 fixed *`hnsw graph not repaired`*, and + `message_embedding` is `ON DELETE CASCADE` from `message` on an installation that deletes mail in + bulk, so it is vacuumed constantly. The earliest version that merely compiles would ship two known + index-corruption bugs. ### One function name, two possible bodies -The application only ever writes `plmail_embed_distance(a, b)`. The **migration** picks the body: +The application only ever writes `plmail_embed_distance(a, b)`. The **migration** picks the body, +and the choice happens in SQL, once. There is no PHP branch and no second query builder to rot, and +the same tests exercise whichever body is installed. -| | body | cost | -|---|---|---| -| normally | plpgsql dot-product loop | 0.11 ms per 768-dim row | -| if `vector` exists | `(a::vector) <=> (b::vector)` | ~24× faster, and reaches an HNSW expression index | +Measured on 74,000 rows of 1024-dimension unit vectors, PostgreSQL 18.4 on musl, single-threaded, +warm cache: -The second is `LANGUAGE sql` and a single `SELECT`, so Postgres inlines it. Detection lives in SQL, -once. There is no PHP branch and no second query builder to rot, and the same tests exercise -whichever body is installed. +| | 20,000 rows | 74,000 rows | per call | +|---|---|---|---| +| plpgsql dot-product loop | 2.14 s | 7.70 s | 0.107 ms | +| `(a::vector) <=> (b::vector)` | 1.23 s | 4.51 s | 0.061 ms | + +**1.75×, and this page previously claimed ~24×.** The correction matters because it points at where +the time actually goes. pgvector's dot product is SIMD and takes microseconds; what dominates is +that `embedding` is `real[]`, so every row pays a detoast of a 4 KB array and a fresh `real[]` → +`vector` conversion before any arithmetic starts. The conversion is the cost, not the distance. It +is still worth taking — 0.63 s → 0.36 s on the semantic arm — but it is a constant-factor win on a +scan, not the thing that removes the scan. + +`Version20260826210000` installs whichever body fits the database it first meets; +`Version20260901120000` asks again, once, for installations that gained the extension afterwards. +Neither ever requires it. ### The bound is a rule, not a hope 56 ms over 500 candidates is cheaper than the full-text pass that produces them. **11 seconds** over 100,000 rows is worse than the 9–10 s pathology the search UNION was rewritten to escape. So the distance function may only ever be evaluated over a bounded candidate set, and the search -integration has to keep that rule rather than assume it. +integration has to keep that rule rather than assume it. Nothing below changes that rule; an index +would be what finally makes it unnecessary. ### Storage @@ -116,13 +152,85 @@ property is that the answers stay correct and only the clock shows it. A table rather than a column because a 768-dimension `float4` array is 3,092 bytes, past the TOAST threshold, so every vector is stored out of line — 391 MB for 100,000 messages. That does not belong on `message`. Truncating to 384 dimensions, where the model supports it, keeps the array inline and -halves the arithmetic. +halves the arithmetic. Measured at the width the current model answers at, 74,000 rows of 1024 +dimensions is 397 MB of table, essentially all of it TOAST — which is also why the `real[]` → `vector` +conversion above dominates the distance it feeds. Vectors are normalised to unit length **on the way in**, which is what lets the distance function be a dot product with no square root per row. A zero-length or non-finite vector is refused rather than stored: one NaN in that column poisons every `ORDER BY` that touches it, and it would be blamed on the search rather than on the model that produced it. +### What it would take to reach an index, and what it would cost + +A distance function only ever scans. The reason to want pgvector is HNSW, and **no index is created +today** — not because it was forgotten, but because three separate things block it. All three were +verified against pgvector 0.8.6 on PostgreSQL 18.4, and none of them is fixed by trying harder in +the migration. + +**1. The indexable expression and the function's expression are not the same expression.** + +``` +CREATE INDEX … USING hnsw ((embedding::vector) vector_cosine_ops); +ERROR: column does not have dimensions +``` + +HNSW needs a width. Only `(embedding::vector(1024))` builds, and Postgres treats that as a different +expression from `(embedding::vector)` — a different typmod argument to `array_to_vector()`. An index +built on one is invisible to a query written with the other. Confirmed both ways with `EXPLAIN`: the +typmod-bearing query takes `Index Scan using …`, the inlined function's expression takes a `Seq +Scan` and a `Sort`, **with no error and no notice**. + +Spelling the width into the function body is not the fix. The width is whatever the configured model +returns — `count($unit)` in `EmbeddingStore`, not a setting a migration can read — and a fixed +`::vector(1024)` raises `expected 1024 dimensions, not 768` for *every* call the moment somebody +changes the model in the admin panel. A dropdown would 500 `/mail/search` for everybody. + +**2. HNSW answers `ORDER BY … LIMIT k`; the search asks a threshold.** The semantic arm is +`plmail_embed_distance(…) <= 0.45` over the most recent `SEMANTIC_CANDIDATES` messages. No +approximate-nearest-neighbour index serves a range predicate — neither HNSW nor IVFFlat has an +operator for it. Reaching an index means the query becoming a top-k ordering. + +**3. Building it is not something a boot-time migration can spend.** At 74,000 × 1024, with +`maintenance_work_mem` left at PostgreSQL's 64 MB default, which is what this stack ships: + +``` +CREATE INDEX … USING hnsw ((embedding::vector(1024)) vector_cosine_ops) WHERE dimensions = 1024; +NOTICE: hnsw graph no longer fits into maintenance_work_mem after 13928 tuples +Time: 460363.979 ms (07:40) +``` + +578 MB of index — larger than the 397 MB table, because HNSW keeps a full copy of every vector plus +its graph. At `maintenance_work_mem = 2 GB` the same build takes 1 min 54 s and is the same size. +Migrations run from the entrypoint of all six services behind one advisory lock, so that is not a +slow migration, it is a stack that does not come back for eight minutes. + +So the shape it would have to take, if someone picks this up: + +- **One partial index per width**, `WHERE dimensions = N`, which is what lets a mailbox + half-re-indexed after a model change keep both halves. +- **The width interpolated into the search SQL** rather than bound — it is already an `int` from the + model, and `$semantic->dimensions` already exists at that point — so both sides say + `::vector(N)`. +- **A top-k `ORDER BY … LIMIT`** in place of the distance threshold. +- **Built by hand, `CONCURRENTLY`, once**, by an operator who has been told the eight minutes and + the 578 MB — never from a migration. + +What that buys, measured on the same 74,000 rows, top-20 nearest: + +| | | +|---|---| +| exact scan | 3.55 s | +| HNSW index scan | 3.0 ms (12 ms cold), 2,084 buffers | + +**~1,180×**, which is the number that would justify all of the above — and which the function swap +above emphatically is not. Two caveats on it. Inserts into a table carrying the index cost 9.1 ms +per row against 0.32 ms without, measured over 2,000 rows, so a full 74,000-message backfill pays +about 11 extra minutes spread across itself. And the benchmark corpus is uniform random vectors, +where every pair is near-orthogonal — distances span 0.86–1.14 with a standard deviation of 0.031 — +so its *recall* number is meaningless and is not quoted here. The timings are sound; measuring +recall needs real embeddings. + ## Semantic search A vector hit is **another arm of the same UNION**, so it lands in `COUNT(*) OVER ()`, pages diff --git a/migrations/Version20260901120000.php b/migrations/Version20260901120000.php new file mode 100644 index 00000000..3f5c026a --- /dev/null +++ b/migrations/Version20260901120000.php @@ -0,0 +1,244 @@ +` 1.23 s + * 74,000 rows plpgsql loop 7.70 s pgvector `<=>` 4.51 s + * + * That is 1.75×, and docs/internals/ai-assist.md used to claim ~24×. The + * difference is not the arithmetic — pgvector's dot product is SIMD and takes + * microseconds — it is that `embedding` is `real[]`, so EVERY row pays a + * detoast of a 4 KB array plus a fresh `real[]` → `vector` conversion before + * the SIMD ever starts. The conversion, not the distance, is the cost. That is + * corrected in the handbook rather than left as a number nobody re-measured. + * + * 1.75× is still worth having: the search evaluates this over + * SEMANTIC_CANDIDATES = 2,000 rows, so the semantic arm drops from about 0.63 s + * to about 0.36 s with no change to any query. + * + * WHY THIS MIGRATION CREATES NO INDEX, WHICH IS THE HARD PART + * ────────────────────────────────────────────────────────── + * A distance function only ever scans. The reason to want pgvector is HNSW, and + * HNSW cannot be reached from here. Three things block it, all verified against + * pgvector 0.8.6 on PostgreSQL 18.4 rather than reasoned about: + * + * 1. AN HNSW INDEX NEEDS A WIDTH, AND THE EXPRESSION ABOVE HAS NONE. + * CREATE INDEX … USING hnsw ((embedding::vector) vector_cosine_ops) + * ERROR: column does not have dimensions + * Only `(embedding::vector(1024))` builds. Postgres treats that as a + * different expression from `(embedding::vector)` — different typmod + * argument to array_to_vector() — so the index built on one is invisible to + * a query written with the other. Confirmed both ways with EXPLAIN: the + * typmod-bearing query takes `Index Scan using …`, the inlined function's + * expression takes a Seq Scan and a Sort, with no error and no notice. That + * is the trap Version20260826210000's own comment warns about, and it is + * what the function walks into. + * + * Spelling the width into the function body instead is not a fix. The width + * is whatever the configured model returns — count($unit) in EmbeddingStore, + * not a setting anything can read here — and a fixed `::vector(1024)` raises + * `expected 1024 dimensions, not 768` for every call the moment somebody + * changes the model in the admin panel. A dropdown would 500 /mail/search + * for everybody, which is exactly the failure that migration's + * dimension-scoping exists to prevent. + * + * 2. HNSW ANSWERS `ORDER BY … LIMIT k`, AND THE SEARCH ASKS SOMETHING ELSE. + * The semantic arm is a threshold — `plmail_embed_distance(…) <= 0.45` over + * the most recent SEMANTIC_CANDIDATES messages. No approximate-nearest- + * neighbour index can serve a range predicate; neither HNSW nor IVFFlat has + * an operator for it. Reaching the index means the query becoming a top-k + * ordering, which is a change to MessageThreadRepository::buildSearchSql(). + * + * 3. IT WOULD COST MORE THAN THIS BOOT CAN SPEND. Measured at 74,000 × 1024 + * under the settings this stack actually ships (maintenance_work_mem left at + * PostgreSQL's 64 MB default): + * + * CREATE INDEX … USING hnsw 7 min 40 s 578 MB + * NOTICE: hnsw graph no longer fits into maintenance_work_mem + * after 13928 tuples + * + * (1 min 54 s at maintenance_work_mem = 2 GB, same 578 MB — the index is + * larger than the 397 MB table it indexes, because HNSW keeps a full copy of + * every vector plus its graph.) + * + * Migrations here run from the entrypoint of all six services at once, + * serialised behind one advisory lock — see MigrateCommand. A build like + * that is not a slow migration, it is a stack that does not come back for + * eight minutes, on an upgrade whose documentation says "Starting the new + * image IS running it". And a plain CREATE INDEX holds a lock that blocks + * the backfill worker and mail ingest for the whole of it. + * + * So the honest state is: the extension is available and the function uses it, + * and the index is a separate, deliberate operator step that is only worth + * taking once the search query can reach it. The exact index, its cost and the + * two changes that make it live are written down in + * docs/internals/ai-assist.md § "What it would take to reach an index", so that + * the next person to touch the search SQL finds a measured plan rather than an + * assumption. + */ +final class Version20260901120000 extends AbstractMigration +{ + public function getDescription(): string + { + return 'Use pgvector for plmail_embed_distance() where the extension is now available.'; + } + + public function up(Schema $schema): void + { + if (false === $this->hasVector()) { + // The shipped path, and the common one. Said out loud because + // "nothing happened" and "something went wrong" look identical in + // migration output otherwise, and somebody who just switched the + // database image needs to know which of the two they got. + // + // The second sentence is not a pointer at documentation, it is the + // way out of a trap. This decision is taken ONCE, when the ledger + // row is written; an operator who upgrades first and switches the + // database image afterwards has already spent it, and no later + // `migrate` will look again — it is at the latest version and says + // so. Reversing this one migration and migrating again re-asks the + // question, and reversing it is safe precisely because down() only + // reinstalls the loop that is already installed. + $this->write( + 'pgvector is not available; plmail_embed_distance() keeps the plpgsql loop. ' + . 'If you enable it later, this migration will not notice on its own — run ' + . '`doctrine:migrations:execute --down DoctrineMigrations\\Version20260901120000` ' + . 'and migrate again. See docs/install/upgrading.md § "Turning on pgvector".', + ); + + return; + } + + $this->write('pgvector is available; plmail_embed_distance() will use it.'); + + $this->addSql('CREATE EXTENSION IF NOT EXISTS vector'); + + // Byte for byte the body Version20260826210000 installs on a database + // that already had the extension, so the two paths converge on one + // definition rather than on two that drift. + // + // LANGUAGE sql and a single SELECT, so Postgres inlines it — verified + // with EXPLAIN VERBOSE, which shows the call replaced by + // `(e.embedding)::vector <=> (…)::vector` in the plan. No typmod on + // either cast, deliberately: see point 1 in the docblock. The cost of + // that is the index; the cost of the alternative is a broken search the + // day the model changes. + // + // COST 100 IS SPELLED OUT, and it is not decoration. Version20260829120000 + // raised this function's procost to 50000 because the plpgsql loop is + // 0.132 ms a call and the planner believed it was 0.25 cost units. That + // number is a lie about THIS body — an inlined SIMD dot product is not + // five hundred page reads — and on an installation that gains pgvector + // later, that migration is already in the ledger and will never run + // again to correct it. CREATE OR REPLACE happens to reset procost to + // the default, which is 100 and is the right answer, but "happens to" + // is not something the next person should have to rediscover from the + // PostgreSQL manual. Saying it makes the two migrations agree in + // writing rather than by accident. + $this->addSql(<<<'SQL' + CREATE OR REPLACE FUNCTION plmail_embed_distance(a real[], b real[]) + RETURNS double precision + LANGUAGE sql IMMUTABLE PARALLEL SAFE STRICT COST 100 AS $$ + SELECT (a::vector) <=> (b::vector) + $$ + SQL); + } + + public function down(Schema $schema): void + { + if (false === $this->hasVector()) { + return; + } + + // Back to the loop, not `DROP FUNCTION`. The function has to keep + // existing under both bodies or every search 500s between this + // migration reversing and Version20260826210000 reversing after it. + // + // The extension itself is left installed. Reversing this migration is + // "stop using pgvector", not "uninstall the operator's extension" — + // they may have put it there for something else, and DROP EXTENSION is + // not a decision a rollback gets to make on their behalf. + // + // COST 50000, and this one MUST be spelled out or reversing this + // migration reintroduces the bug Version20260829120000 exists to fix. + // That migration priced the loop at 50000 — 0.132 ms a call, against + // the 100 CREATE FUNCTION gives by default — and it is already applied, + // so it will not run again to re-price a body this reinstalls. Without + // this, `down()` would hand the planner a 1024-iteration plpgsql loop + // labelled a quarter of a page read. + // + // The literal is duplicated from that migration on purpose: a migration + // is a record of what was done, not a live reference to a constant that + // may move. If it ever does move, both files are found by grepping for + // plmail_embed_distance. + $this->addSql(<<<'SQL' + CREATE OR REPLACE FUNCTION plmail_embed_distance(a real[], b real[]) + RETURNS double precision + LANGUAGE plpgsql IMMUTABLE PARALLEL SAFE STRICT COST 50000 AS $$ + DECLARE + total double precision := 0; + width int := least(array_length(a, 1), array_length(b, 1)); + i int; + BEGIN + IF width IS NULL THEN + RETURN 2; + END IF; + + FOR i IN 1..width LOOP + total := total + (a[i]::double precision * b[i]::double precision); + END LOOP; + + RETURN 1 - total; + END; + $$ + SQL); + } + + /** + * Whether this database COULD have the extension. + * + * pg_available_extensions rather than pg_extension, matching + * Version20260826210000: the question is whether the files are on disk in + * the image, which is what an operator changes by swapping the database + * image. Whether it is installed into this particular database is + * CREATE EXTENSION IF NOT EXISTS's problem, one line further down. + */ + private function hasVector(): bool + { + return (bool) $this->connection->fetchOne( + "SELECT 1 FROM pg_available_extensions WHERE name = 'vector'", + ); + } +} diff --git a/tests/Infrastructure/Deployment/ComposeEnvironmentTest.php b/tests/Infrastructure/Deployment/ComposeEnvironmentTest.php index b05ce206..14da20eb 100644 --- a/tests/Infrastructure/Deployment/ComposeEnvironmentTest.php +++ b/tests/Infrastructure/Deployment/ComposeEnvironmentTest.php @@ -63,6 +63,25 @@ final class ComposeEnvironmentTest extends TestCase 'compose.demo.yaml', ]; + /** + * Overlays that define INFRASTRUCTURE rather than the app. + * + * They get the `.env` check and not the APP_ENV one, and the split is the + * point rather than an exemption: an overlay with no app service has no + * APP_ENV to hardcode, so putting it in the list above asserts something + * that cannot be true and fails for a reason unrelated to what it guards. + * + * compose.pgvector.yaml redefines `database` alone. It passes the `.env` + * check today for a reason that could quietly stop being true: neither + * variable it interpolates — POSTGRES_VERSION and PGVECTOR_VERSION — + * appears in `.env`, so an operator gets the compose defaults. Add either + * one there and the tag this file BUILDS and the tag it RUNS can disagree, + * which is a stack that comes up healthy on yesterday's extension. + */ + public const array INFRASTRUCTURE_COMPOSE_FILES = [ + 'compose.pgvector.yaml', + ]; + /** * DATABASE_URL is the one variable allowed to disagree, on purpose. * @@ -76,6 +95,14 @@ final class ComposeEnvironmentTest extends TestCase private const array EXEMPT = ['DATABASE_URL']; /** @return iterable */ + /** Every committed overlay an operator runs, app-defining or not. */ + public static function allComposeFiles(): iterable + { + foreach ([...self::OPERATOR_COMPOSE_FILES, ...self::INFRASTRUCTURE_COMPOSE_FILES] as $file) { + yield $file => [$file]; + } + } + public static function operatorComposeFiles(): iterable { foreach (self::OPERATOR_COMPOSE_FILES as $file) { @@ -83,7 +110,7 @@ final class ComposeEnvironmentTest extends TestCase } } - #[DataProvider('operatorComposeFiles')] + #[DataProvider('allComposeFiles')] public function testNoComposeDefaultIsOverruledByTheCommittedDotEnv(string $file): void { $env = self::committedEnvironment();