fix: apply Oban Pro 1.7 follow-up cleanup + filter transient DB noise

The 1.7 upgrade migration added the new oban_workflows tracking but left
behind legacy state from earlier versions that the upgrade guide
strongly recommends removing
(https://oban.pro/docs/pro/v1-7.html):

  - The uniq_key / partition_key GENERATED ALWAYS columns. The Smart
    engine no longer reads them; uniqueness and partition lookups now
    use expression indexes on meta. Keeping the columns imposes write
    overhead on every oban_jobs insert/update and was contributing to
    intermittent transaction-rollback errors.
  - The oban_jobs_*_old workflow/chain indexes. v1.7 renamed the
    originals and built expression-based replacements; the _old indexes
    are dead weight that still slow down writes.

Also extend ErrorTrackerIgnorer to drop transient
DBConnection.ConnectionError noise ("connection is closed",
"transaction rolling back"). These come from the Repo's intentional
queue-target / disconnect-on-error cycling that recovers from stale SSL
connections — Ecto retries automatically, so logging adds noise without
value.
This commit is contained in:
Graham McIntire 2026-05-01 16:04:05 -05:00
parent 1804a0a26a
commit a99c327446
4 changed files with 120 additions and 5 deletions

View file

@ -1,6 +1,7 @@
defmodule Towerops.ErrorTrackerIgnorer do
@moduledoc """
Drops benign errors from ErrorTracker that occur during K8s pod shutdown.
Drops benign errors from ErrorTracker that occur during K8s pod shutdown
or transient DB connection cycling.
Matches the same patterns as `Towerops.HoneybadgerFilter` and
`Towerops.LoggerFilters.drop_shutdown_errors/2`.
@ -9,8 +10,8 @@ defmodule Towerops.ErrorTrackerIgnorer do
@behaviour ErrorTracker.Ignorer
@impl true
def ignore?(%ErrorTracker.Error{reason: reason}, _context) do
shutdown_error?(reason)
def ignore?(%ErrorTracker.Error{reason: reason} = error, _context) do
shutdown_error?(reason) or transient_db_error?(error)
end
defp shutdown_error?(reason) when is_binary(reason) do
@ -21,4 +22,25 @@ defmodule Towerops.ErrorTrackerIgnorer do
end
defp shutdown_error?(_), do: false
# Transient DB connection drops surface as DBConnection.ConnectionError. The
# Repo's aggressive idle/queue cycling (queue_target/queue_interval,
# disconnect_on_error_codes) intentionally tears down stale SSL connections,
# which can interrupt queries mid-flight. Common reasons we don't want to
# log: "connection is closed..." and "transaction rolling back". The query
# is retried by the caller; logging adds noise without value.
@transient_db_phrases [
"connection is closed",
"transaction rolling back"
]
defp transient_db_error?(%ErrorTracker.Error{reason: reason, kind: kind}) do
binary_reason = to_string(reason || "")
binary_kind = to_string(kind || "")
String.contains?(binary_kind, "DBConnection.ConnectionError") and
Enum.any?(@transient_db_phrases, &String.contains?(binary_reason, &1))
end
defp transient_db_error?(_), do: false
end

View file

@ -0,0 +1,39 @@
defmodule Towerops.Repo.Migrations.DropUnusedObanProGeneratedColumns do
use Ecto.Migration
# Per the Oban Pro 1.7 upgrade guide
# (https://oban.pro/docs/pro/v1-7.html), the `uniq_key` and `partition_key`
# GENERATED ALWAYS columns are no longer used by the Smart engine — uniqueness
# and partition lookups are now served by expression indexes on `meta`. The
# columns still impose write overhead on every INSERT/UPDATE to `oban_jobs`
# and can interact badly with Smart engine batches, so the guide strongly
# recommends dropping them.
def up do
alter table(:oban_jobs) do
remove_if_exists :uniq_key, :text
remove_if_exists :partition_key, :text
end
end
def down do
execute """
ALTER TABLE oban_jobs
ADD COLUMN IF NOT EXISTS uniq_key text
GENERATED ALWAYS AS (
CASE
WHEN meta->'uniq_bmp' @> oban_state_to_bit(state) THEN meta->>'uniq_key'
END
) STORED
"""
execute """
ALTER TABLE oban_jobs
ADD COLUMN IF NOT EXISTS partition_key text
GENERATED ALWAYS AS (
CASE
WHEN meta ? 'partition_key' THEN meta->>'partition_key'
END
) STORED
"""
end
end

View file

@ -0,0 +1,39 @@
defmodule Towerops.Repo.Migrations.DropLegacyObanProWorkflowIndexes do
use Ecto.Migration
@disable_ddl_transaction true
@disable_migration_lock true
# Drop the legacy `_old` workflow/chain indexes left behind by the Oban Pro
# v1.7 migration. v1.7 renamed the originals to `_old` and built new
# expression-based replacements; the `_old` indexes are now dead weight that
# still slow down writes to `oban_jobs`.
#
# Concurrent drops require the migration to run outside a DDL transaction
# and outside the migration lock.
def up do
execute "DROP INDEX CONCURRENTLY IF EXISTS oban_jobs_workflow_index_old"
execute "DROP INDEX CONCURRENTLY IF EXISTS oban_jobs_sup_workflow_index_old"
execute "DROP INDEX CONCURRENTLY IF EXISTS oban_jobs_chain_index_old"
end
def down do
execute """
CREATE INDEX CONCURRENTLY IF NOT EXISTS oban_jobs_workflow_index_old
ON oban_jobs ((meta->>'workflow_id'), state, (meta->>'on_hold'), (meta->>'name'))
WHERE meta ? 'workflow_id'
"""
execute """
CREATE INDEX CONCURRENTLY IF NOT EXISTS oban_jobs_sup_workflow_index_old
ON oban_jobs ((meta->>'sup_workflow_id'), state, (meta->>'on_hold'))
WHERE meta ? 'sup_workflow_id'
"""
execute """
CREATE INDEX CONCURRENTLY IF NOT EXISTS oban_jobs_chain_index_old
ON oban_jobs (state, (meta->>'chain_id'), (meta->>'on_hold'))
WHERE meta ? 'chain_id'
"""
end
end

View file

@ -3,9 +3,9 @@ defmodule Towerops.ErrorTrackerIgnorerTest do
alias Towerops.ErrorTrackerIgnorer
defp build_error(reason) do
defp build_error(reason, kind \\ "ErlangError") do
%ErrorTracker.Error{
kind: "ErlangError",
kind: kind,
reason: reason,
source_line: "-",
source_function: "-",
@ -37,9 +37,24 @@ defmodule Towerops.ErrorTrackerIgnorerTest do
assert ErrorTrackerIgnorer.ignore?(error, %{})
end
test "ignores transient DBConnection.ConnectionError closures" do
error =
build_error(
"connection is closed because of an error, disconnect or timeout",
"Elixir.DBConnection.ConnectionError"
)
assert ErrorTrackerIgnorer.ignore?(error, %{})
end
test "does not ignore regular errors" do
error = build_error("something went wrong")
refute ErrorTrackerIgnorer.ignore?(error, %{})
end
test "does not ignore other DBConnection errors" do
error = build_error("ownership timeout", "Elixir.DBConnection.ConnectionError")
refute ErrorTrackerIgnorer.ignore?(error, %{})
end
end
end