fix(etl): playlist_seen PK, ErrNoRows on soft-deletes, attribute dispatch errors - #310
Merged
Merged
Conversation
… dispatch errors
Follow-ups from the overnight prod-clone validation run (38,834 chain
blocks, 69 deep diff cycles against the read replica).
1. playlist_seen PK was wrong. Our 0005 migration declared
`PRIMARY KEY (is_current, user_id, playlist_id, seen_at)` but prod's
schema has `(user_id, playlist_id, seen_at)`. The handler's ON CONFLICT
target that names the prod columns gets rejected with 42P10 against
the old in-house PK; the version that names is_current too (which
the previous PR had) silently mismatches prod. Fix both sides:
- Update 0005 to declare the prod-correct PK on fresh databases.
- Add migration 0027 to DROP+ADD the PK on databases that already
ran the broken 0005.
- Drop is_current from the ON CONFLICT spec in notification.go.
2. loadCurrentTrackRow / loadCurrentPlaylistRow propagated raw
pgx.ErrNoRows when the target was soft-deleted (is_delete = true)
between validation and update. The validation (trackExists /
playlistExists) accepts is_delete=true rows; these row-loaders filter
them out. Race manifested as bursts of "no rows in result set" at
indexer.go:425 during high-Repost / high-Follow surge windows. Wrap
ErrNoRows as ValidationError so the dispatcher logs at WARN, not ERROR.
3. getUserHandle in user_update.go had the same shape — propagated raw
ErrNoRows in the rare race window between userExists and the handle
lookup. Treat ErrNoRows as empty handle (callers compare against the
new handle).
4. The dispatch-error log line at indexer.go:425 had no context fields,
so when one fired we could see "no rows in result set" but had no clue
which handler emitted it. Add entity_type / action / entity_id /
user_id / hash to the log so future occurrences are self-attributing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Merged
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.
Summary
Follow-ups from the overnight prod-clone validation run (38,834 chain blocks, 69 deep diff cycles against the prod read replica). PR #308's fixes held up cleanly — these are the remaining two real findings + one observability gap, all surgical.
Changes
1.
playlist_seenprimary key mismatch with prodOur
0005_notification_tables.up.sqldeclared the PK as(is_current, user_id, playlist_id, seen_at). Prod's schema has(user_id, playlist_id, seen_at)— nois_current. This caused an asymmetric bug:ON CONFLICT (is_current, user_id, playlist_id, seen_at)matched our in-house PK but silently mismatched prod.ON CONFLICT (user_id, playlist_id, seen_at)matches prod but breaks against our in-house PK with42P10 no unique or exclusion constraint matching.Observed firing 7 times in the 11h validation run. Fix:
0005_notification_tables.up.sql: declare the prod-correct PK on fresh databases.0027_fix_playlist_seen_pkey.up.sql(new): DROP+ADD the PK on databases that already ran the broken0005.notification.go:100: dropis_currentfrom the ON CONFLICT target.2.
loadCurrentTrackRow/loadCurrentPlaylistRowpropagated rawpgx.ErrNoRowson soft-deleted rowsvalidateTrackUpdate/validatePlaylistUpdateusetrackExists/playlistExistswhich acceptis_delete = truerows. The row-loadersloadCurrentTrackRowandloadCurrentPlaylistRowfilteris_delete = false— so a track that was soft-deleted between validation and update returns rawpgx.ErrNoRows, which the dispatcher then logs at ERROR level asno rows in result set.Observed firing 14 times during the 11h run, clustered with
Track/Repost "already exists"rejections during follow surges — strongly correlated with high-activity windows where soft-deletes and updates race.Wrap
ErrNoRowsin both row-loaders as aNewValidationError("X N is deleted or does not exist")so the dispatcher logs at WARN, not ERROR.3.
getUserHandlerace-window ErrNoRowsSame shape as #2 —
validateUserUpdatechecksuserExists, but if the user disappears in the narrow window beforegetUserHandleruns, the rawErrNoRowsleaks out. Fix: treatErrNoRowsas empty handle (callers compare against the new handle).4. Attribute dispatch errors to a handler — observability fix
The dispatch-error log line at
indexer.go:425had no context fields, so when one fired we could seeno rows in result setbut had no clue which handler emitted it. Made #2 hard to attribute — only discovered via correlation analysis after multi-hour runs.Add
entity_type / action / entity_id / user_id / hashto the log so future occurrences are self-attributing. Mirrors the fields already on the WARN line for ValidationErrors.Test plan
go build ./...+go vet ./...cleango test ./pkg/etl/...against fresh Postgres: all green (including the previously-affectedTestPlaylistSeenView_Success)TestPlaylistSeenView_Successnow exercises the new PK on the test side and passesTestPlaylistUpdate_Success(which exercisesloadCurrentPlaylistRow) still passesOpen follow-ups (out of scope, called out in run findings)
is_albumcheck producessave_type='album'where prod writes'playlist'. Intentional, but downstream consumers should know.🤖 Generated with Claude Code