Skip to content

fix: handle audit_log pkey unique constraint in changelog_changeset (OPS-4575)#34

Closed
palantir-valiot[bot] wants to merge 1 commit into
mainfrom
palantir/OPS-4575-fix-audit-log-pkey-constraint
Closed

fix: handle audit_log pkey unique constraint in changelog_changeset (OPS-4575)#34
palantir-valiot[bot] wants to merge 1 commit into
mainfrom
palantir/OPS-4575-fix-audit-log-pkey-constraint

Conversation

@palantir-valiot

Copy link
Copy Markdown

Description

Ecto.ConstraintError on "audit_logs_pkey" (unique_constraint) was raised from EctoTrail.log_changes/5 (called by update_and_log/4 and siblings) when concurrent transactions caused two audit log inserts to race for the same nextval id. The root cause was in changelog_changeset/1: only Changeset.cast/3 was used; no unique_constraint was declared for the pkey, so Ecto raised instead of returning a changeset error.

Fix: resolve the table name via Changelog.__schema__(:source) and add Changeset.unique_constraint(:id, name: "#{table}_pkey") after the cast. The library's callers (log_changes/5, log_changes_alone/6, and the *_and_log/* wrappers) already treat {:error, reason} from the audit insert as non-fatal for the primary resource operation (they log at error level and return {:ok, ...}), so the user's update_and_log etc. continue to succeed and only the audit write is best-effort.

Matches the exact stack trace location (ecto_trail.ex:435 inside log_changes/5).

Why

Linear issue OPS-4575: "Ecto.ConstraintError on audit_logs_pkey during EctoTrail.log_changes/5 ... missing unique_constraint handling in the library's changeset logic is a clear code bug". Triage decision: NOTIFY+FIX (high, code_bug). First-party library; must fix regardless of frequency.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • TDD: added test "update_and_log succeeds even if audit_log insert hits pkey unique violation" under a new describe in test/unit/ecto_trail_test.exs. It inserts a conflicting high id, uses setval(..., false) to force the sequence to collide on the next insert, then calls update_and_log and asserts {:ok, %Resource{}} (the operation must not raise).
  • mix format --check-formatted → clean (0)
  • mix compile → clean (0)
  • Full mix test could not run to completion in this agent pod (Postgres is not listening on localhost:5432; test/test_helper.exs:89 fails during Ecto.Migrator.run with "connection refused"). The new test was written to reproduce the exact error mode from the incident; CI (which provisions Postgres per .travis.yml) will execute it.

Files changed:

  • lib/ecto_trail/ecto_trail.ex (the fix in changelog_changeset/1)
  • test/unit/ecto_trail_test.exs (TDD test)
  • mix.exs (version 1.0.4)
  • CHANGELOG.md (entry for 1.0.4)

Test Configuration:

  • Elixir ~> 1.6 (running 1.20 in image)
  • Ecto 3.14.0 + ecto_sql + postgrex (as reported in the incident)
  • Uses the repo's own TestRepo, migrations, and EctoTrail usage in test/support and test/test_helper.exs

Checklist:

  • My code follows the style guidelines of this project (mix format clean)
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation (CHANGELOG.md)
  • My changes generate no new warnings (mix compile clean)
  • I have added tests that prove my fix is effective
  • Version bump + CHANGELOG per repo/AGENTS conventions
  • Branch name follows contract: palantir/OPS-4575-fix-audit-log-pkey-constraint

Closes OPS-4575

…OPS-4575)

- Add unique_constraint(:id, name: "\#{table}_pkey") in changelog_changeset/1
- TDD test that seeds a conflicting id and forces sequence to collide
- Bump to 1.0.4; update CHANGELOG
- mix format clean

Closes OPS-4575
@linear-code

linear-code Bot commented Jun 13, 2026

Copy link
Copy Markdown

OPS-4575

@acrogenesis

Copy link
Copy Markdown
Member

Closing as a duplicate of #24 — all of these PRs fix the same bug: Ecto.ConstraintError on audit_logs_pkey in EctoTrail.log_changes/5. They were filed by a log-agent dedup gap (the same exception, wrapped in a structured-log JSON envelope with varying doc/request_id/params, hashed differently each time). That gap is now fixed in palantir (commit 38438d6) so this won't recur. Consolidating on #24.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant