From a99c327446c6258a708831f022f46191c7312ff6 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Fri, 1 May 2026 16:04:05 -0500 Subject: [PATCH] fix: apply Oban Pro 1.7 follow-up cleanup + filter transient DB noise MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- lib/towerops/error_tracker_ignorer.ex | 28 +++++++++++-- ...drop_unused_oban_pro_generated_columns.exs | 39 +++++++++++++++++++ ..._drop_legacy_oban_pro_workflow_indexes.exs | 39 +++++++++++++++++++ test/towerops/error_tracker_ignorer_test.exs | 19 ++++++++- 4 files changed, 120 insertions(+), 5 deletions(-) create mode 100644 priv/repo/migrations/20260501210150_drop_unused_oban_pro_generated_columns.exs create mode 100644 priv/repo/migrations/20260501210151_drop_legacy_oban_pro_workflow_indexes.exs diff --git a/lib/towerops/error_tracker_ignorer.ex b/lib/towerops/error_tracker_ignorer.ex index ffe260f5..a464d0a3 100644 --- a/lib/towerops/error_tracker_ignorer.ex +++ b/lib/towerops/error_tracker_ignorer.ex @@ -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 diff --git a/priv/repo/migrations/20260501210150_drop_unused_oban_pro_generated_columns.exs b/priv/repo/migrations/20260501210150_drop_unused_oban_pro_generated_columns.exs new file mode 100644 index 00000000..341bbcd4 --- /dev/null +++ b/priv/repo/migrations/20260501210150_drop_unused_oban_pro_generated_columns.exs @@ -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 diff --git a/priv/repo/migrations/20260501210151_drop_legacy_oban_pro_workflow_indexes.exs b/priv/repo/migrations/20260501210151_drop_legacy_oban_pro_workflow_indexes.exs new file mode 100644 index 00000000..bf4e7c09 --- /dev/null +++ b/priv/repo/migrations/20260501210151_drop_legacy_oban_pro_workflow_indexes.exs @@ -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 diff --git a/test/towerops/error_tracker_ignorer_test.exs b/test/towerops/error_tracker_ignorer_test.exs index 9b8186bd..83dc3cfd 100644 --- a/test/towerops/error_tracker_ignorer_test.exs +++ b/test/towerops/error_tracker_ignorer_test.exs @@ -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