deps: pin pkg/etl v1.6.4 and backfill users + save/repost types - #1011
Merged
Conversation
users_pkey is (user_id, txhash), which admits a second is_current row under a different txhash. Five users have one, spread from 2025-06 to 2026-06. Five rows, but the blast radius is not five rows: anything joining an entity to its owner's wallet fans out and silently duplicates every entity those users own — 18 extra tracks and 787 extra follows on a production clone. This is the backfill half of pkg/etl migration 0035, which adds users_current_uniq_idx to stop it recurring. The delete lives here rather than in that migration because ETL migrations run automatically at indexer start, so shipping it there would make a `go get` silently remove rows from this database. The index cannot be created while duplicates exist, so this has to run first, and it does: ddl migrations run in the pre-roll `migrate` Job that every serving Deployment depends on, while the ETL's run at indexer start after that Job completes. Verified both orders against a fixture — backfill then index succeeds; index first fails with "could not create unique index ... Key (user_id)=(98311147) is duplicated" rather than repairing anything, which is the signal that this migration has not run. Deleted rather than demoted: users keeps no versioned history, so demoting would leave a category of row nothing reads. No foreign key references users, and its triggers are INSERT or INSERT OR UPDATE, so a delete fires neither. Winner is the highest blocknumber, which also keeps is_deactivated = true for user 666149592. Re-running is a no-op. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit a20666a)
An album is a playlist with is_album = true. The indexer briefly derived a separate 'album' save_type/repost_type by reading playlists.is_album at index time; it no longer does (OpenAudio/go-openaudio#428). This backfills the rows written while that was live: 670 saves and 528 reposts, all first appearing on 2026-05-28. Two hazards this has to avoid: * on_save/on_repost fire on AFTER INSERT OR UPDATE, and the notification group_id embeds the type ('save:<id>:type:<save_type>'). A plain UPDATE would mint a second favourite/repost notification per row under a new group_id, so those two triggers are disabled for the backfill. trg_saves/trg_reposts stay enabled so the search indexer still sees the rows change. * save_type/repost_type are part of the primary key. Verified against prod data that no (user_id, item_id, txhash) has both a 'playlist' and an 'album' row, so this is a straight UPDATE with no conflict handling. Aggregate counts are unaffected either way: handle_save's delta is transition-aware and evaluates to 0 when is_delete does not change. Each table gets its own transaction to keep the ACCESS EXCLUSIVE lock taken by ALTER TABLE ... DISABLE TRIGGER as short as possible. Re-running is a no-op once no 'album' rows remain. The 'album' label is left in the savetype/reposttype enums since Postgres cannot drop an enum value without rebuilding the type. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit 737918c)
This was referenced Aug 4, 2026
Moves off the 2026-07-27 pseudo-version onto the real tag. Three changes come
with it, all pkg/etl-only, so the root module pin is untouched:
* album saves and reposts record save_type/repost_type 'playlist' rather
than deriving 'album' from the mutable playlists.is_album at index time
(OpenAudio/go-openaudio#428). The rows already written as 'album' are
backfilled by ddl 0236 in this PR.
* one current row per user is enforced via users_current_uniq_idx, and the
fifteen DISTINCT ON dedupe subqueries in genesis-writer collapse to plain
joins (#425). The duplicates that index cannot coexist with are removed by
ddl 0237 in this PR, which runs first.
* that index migration no longer deletes anything itself (#433), so no ETL
migration modifies row data.
Also rewrites the comment above the ETL config. It described the migrations
as additive without saying why that matters; it now records the line being
held — a module bump reaches this database automatically, so data repair
belongs in ddl/ where it goes through review and the pre-roll migrate Job —
and the corollary that 0035 fails if ddl 0237 has not run, which the Job's
ordering is what prevents.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
rickyrombo
force-pushed
the
deps/etl-bump-and-backfills
branch
from
August 5, 2026 00:18
c552a8e to
bdc41d9
Compare
rickyrombo
changed the title
rickyrombo
marked this pull request as ready for review
August 5, 2026 00:18
0236 as written would abort against production data. It justified a blind
UPDATE by checking saves_pkey (user_id, save_item_id, save_type, txhash),
where nothing collides. But pkg/etl 0030 also added
saves_current_uniq_idx ON saves (user_id, save_item_id, save_type) WHERE is_current
reposts_current_uniq_idx ON reposts (user_id, repost_item_id, repost_type) WHERE is_current
which carry no txhash. Two current rows for one (user, item) are legal today
precisely because one is 'album' and one is 'playlist', so collapsing the
type makes them duplicates: 43 pairs in saves, 34 in reposts. The UPDATE
would have raised a unique violation, failed the pre-roll migrate Job, and
blocked the deploy — taking 0237 with it, since psql exits on the first
error and 0237 sorts after 0236.
So resolve the pairs first, then retype the rest. The loser of each pair is
demoted (is_current = false), which these tables already use for superseded
rows, unlike users. Winner is the highest blocknumber: of the 43 save pairs
38 agree on is_delete and 5 disagree, and in every disagreeing pair the
blocknumbers differ with the later row holding the correct state. Zero pairs
both disagree and tie, so the choice is always decidable; the created_at and
type tiebreaks only make it deterministic. Reposts are the same shape.
This is also a net aggregate correction — reconcile_aggregates counts these
rows with count(*) across both types, so each pair was double-counting
aggregate_playlist.save_count / repost_count.
Verified against a fixture carrying the real partial unique indexes and real
colliding rows from a production clone: applies cleanly, leaves no 'album'
rows and no duplicate current pairs, preserves the 2026 unfavourite for user
9014, fires on_save/on_repost zero times, re-enables both triggers, and is a
no-op on a second run.
Also from review:
* guard the DISABLE/ENABLE TRIGGER statements on the trigger existing.
pg_migrate.sh applies migrations/ before functions/, so on a database
bootstrapped from ddl/ alone they do not exist yet.
* correct the lock level in 0236's comment: ALTER TABLE ... DISABLE TRIGGER
takes ShareRowExclusiveLock, which blocks writes but not reads.
* narrow 0237 to the offending user_ids and compare txhash under the C
collation so its tiebreak cannot vary by database collation.
* drop the claim that ETL migrations are all additive DDL — 0026 drops a
constraint and 0027 recreates playlist_seen's primary key. They do not
touch row data, which is the property that actually matters here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CI could not have caught 0236's unique violation. The four partial unique indexes from pkg/etl migration 0030 exist in every deployed database, because the ETL module runs its own migrations at indexer start — but they are absent from sql/01_schema.sql, so the constraint that 0236 actually had to satisfy simply did not exist under test. The gap cannot close on its own. `make test-schema` brings up a database from this dump, applies only ddl/ migrations, and dumps the result. ETL-created objects are never introduced anywhere in that loop, so regenerating the file reproduces the same omission. Adding them once is enough, for the same reason: regeneration starts from this file, so they carry forward. users_current_uniq_idx is deliberately excluded. pkg/etl 0035 has not run in production, and ddl 0237 is specified to run before it does — seeding it would make the test schema describe a state that does not exist yet, and would hide whether 0237 works on the schema it will actually meet. The definitions are copied verbatim from a production clone rather than from the migration source, so they match what deployments really have. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`make test-schema` seeds a database from sql/01_schema.sql, runs `make
migrate`, and dumps the result. Only ddl/ migrations ran in that loop, so
objects created by the vendored pkg/etl migrations could never enter the
dump no matter how often it was regenerated. That is why the four partial
unique indexes from pkg/etl 0030 were absent under test while present in
every deployment — and why ddl 0236 could be written against the wrong
constraint and still pass CI.
The `migrate` subcommand now applies the ETL migrations after the ddl ones.
They are idempotent and tracked separately in etl_db_migrations, so the
indexer applying them again at startup is a no-op. Useful side effect: a ddl
migration that must precede an ETL one (0237 before 0035) is now sequenced
within a single process rather than across two pods.
Two things had to be fixed for that to work at all:
* the ETL migrator uses lib/pq, which defaults to sslmode=require, while
api's connection strings rely on pgx's laxer default. Against the compose
database it failed with "pq: SSL is not enabled on the server", so the
target's writeDbUrl now carries sslmode=disable.
* the recipe chained with ';' and never set errexit, so that failure was
swallowed: `make migrate` exited 1 and the target went on to dump a
schema missing everything and print "Done". Now `set -e`.
Regenerating with both fixed shows the seed was stale in two independent
ways. It gained the 14 etl_ tables, etl_db_migrations and the fifth unique
index as expected — but the migration tracker also went from 185 rows to
192, picking up five ddl migrations whose objects the dump had never
contained: 0232_new_chain_queue, both 0233s, 0234 and 0235. Two of the
remaining entries are this branch's own 0236 and 0237; the rest are
refreshed md5s for two functions and a view.
Nothing was removed — the object inventory is purely additive (tables
164→179, indexes 201→265, views 16→18, functions 90→92, sequences 23→37,
zero dropped).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
rickyrombo
force-pushed
the
deps/etl-bump-and-backfills
branch
from
August 5, 2026 00:56
b79f8f2 to
4939feb
Compare
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
1 participant
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.
Supersedes #1008, #1009 and #1010, which were the same work split three ways.
Why one PR
These three changes are only correct together, in one deploy. Split, each one alone breaks something:
0035createsusers_current_uniq_idx, fails on the existing duplicates,RunMigrationserrors and the indexer won't start0236(album)albumfromis_albumand undoes the backfill0237(users)As one PR the deploy is atomic, and the ordering inside it is guaranteed by existing machinery:
bridge migrateruns as a pre-roll Job that every serving DeploymentDependsOn(serving pods getrunMigrations=false), so both ddl migrations complete before the indexer starts and runs the ETL's.Contents
0237_users_one_current_row_backfill— deletes 5 duplicateis_currentrows fromusers. Small count, large blast radius: joins from an entity to its owner's wallet fan out, measured at +18 tracks and +787 follows on a production clone. Must precede the ETL index.0236_saves_reposts_album_to_playlist— 670 saves and 528 reposts written asalbumcollapse toplaylist.on_save/on_repostare disabled for the update (their notificationgroup_idembeds the type, so a plain UPDATE would mint duplicate favourite notifications);trg_saves/trg_repostsstay enabled so the search indexer sees the change.deps: pin pkg/etl v1.6.4— brings OpenAudio/go-openaudio#428 (album type), #425 (theusersinvariant + genesis-writer join simplification) and #433 (0035no longer deletes anything).The delete moved out of the ETL migration
v1.6.3's0035deleted the duplicates itself. Since ETL migrations run automatically at indexer start, that made ago getable to remove rows from this database. #433 split it: the index stays in the ETL migration, the repair moved to0237here.v1.6.4ships zeroDELETEstatements — verified against the resolved module, not just the tag.The comment above the ETL config now records that line, and its corollary: an ETL migration can depend on a ddl one having run, and
0035fails loudly if0237hasn't.Verified
pkg/etl@v1.6.4contains0035withCREATE UNIQUE INDEXand 0DELETEstatements.violations 0,indisvalid = t); index→backfill fails withcould not create unique index … Key (user_id)=(98311147) is duplicated, which is the intended signal that0237hasn't run.0236fires zeroon_save/on_reposttriggers against a fixture with the real wiring, and exactly onepg_notifyper updated row.go build ./...andgo vet ./indexer/clean.users; its triggers are INSERT / INSERT OR UPDATE, so the delete fires neither.pkg/etl/v1.6.4did not moveopenaudio/go-openaudio:stable— still the 2026-07-30v1.8.2digest, so no node-operator rollout.Not established
The cause of the duplicate
usersrows. Both indexer create paths reject an existing user, so a single writer can't produce them; a second writer can, since check-then-act isn't atomic across transactions. Three of five pairs put a bare-hextxhashnext to a0x-prefixed one, which fits but doesn't prove it. The index will surface it if it recurs.🤖 Generated with Claude Code