fix: declare unique_constraint on audit_log pkey to swallow Ecto.ConstraintError (OPS-4587)#47
Closed
palantir-valiot[bot] wants to merge 1 commit into
Closed
Conversation
…t_log" <> "_pkey") in changelog_changeset Prevents Ecto.ConstraintError on "audit_logs_pkey" (unique_constraint) from log_changes/5 during update_and_log/insert_and_log/etc when the sequence yields a colliding id for the audit row (observed in prod under high load / sequence rewind scenarios). - Added regression test exercising the collision path inside the update_and_log transaction (rewinds the id_seq and asserts the user op succeeds while the audit insert is turned into a best-effort error). - Bumped version 1.0.3 -> 1.0.4 and added CHANGELOG entry. - Upgraded benchee + credo (and transitive) within allowed ranges per deps-freshness rule. - mix format clean; compiles clean. Closes OPS-4587
Member
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
Declare
unique_constraint(:id, name: "<table>_pkey")insidechangelog_changeset/1(lib/ecto_trail/ecto_trail.ex:574) so that a colliding primary-key insert on the audit log (theaudit_log_pkey/audit_logs_pkeyunique constraint) duringlog_changes/5(called fromupdate_and_log,insert_and_log, etc.) is turned into a changeset error instead of raisingEcto.ConstraintError. The audit row is best-effort; the user transaction continues to succeed. Added a regression test that forces a sequence collision inside the same tx.Also bumped version 1.0.3 → 1.0.4, added CHANGELOG entry, ran
mix format, upgraded dev/test deps within ranges (benchee, credo + transitives), andmix compileis clean.Why
Observed in prod (eliot-lamosa-gto-prod):
Triage (OPS-4587): NOTIFY+FIX, high severity, code_bug in first-party package. The root cause is the absence of a
unique_constraint/3declaration for the dynamically-named audit table's pkey when building the Changelog changeset.Fixes the exact failure mode described in the stack trace and the Linear issue.
Changes
lib/ecto_trail/ecto_trail.ex: compute table name from config (default "audit_log"), derive"<table>_pkey", and callChangeset.unique_constraint(:id, name: pkey)inchangelog_changeset/1.test/unit/ecto_trail_test.exs: new test underupdate_and_log/3that:*_id_seqviaALTER SEQUENCE ... RESTART WITH.mix.exs:@version "1.0.4".CHANGELOG.md: new## [1.0.4]entry.mix.lock: refreshed benchee 1.5.0→1.5.1, credo 1.7.18→1.7.19 and their transitive deep_merge/statistex (per "keep dependencies fresh" rule).Test plan
mix format(clean; any long lines auto-fixed)mix compile(clean; only pre-existing redundant clause warning on map_custom_ecto_type)mix hex.outdated+mix deps.update benchee credo(upgraded within allowed ranges; lockfile diff reviewed)mix test— requires a running Postgres (TestRepo + sandbox + ALTER SEQUENCE in the new test). The agent image has no Postgres, no docker, and no sudo. The regression test is written to go red for "constraint error when attempting to insert" before the fix and green after. CI on the PR will execute it against the real DB.git diff --statand full diff vs origin/main (no debug prints, no empty hunks, no scope creep, follows existing patterns, no new comments per "DO NOT ADD ANY COMMENTS").Closes OPS-4587