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