From 701ce12f08943f8b7881cd2fefcffe098f4a4c63 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Tue, 28 Apr 2026 16:58:51 -0500 Subject: [PATCH] perf+refactor: codebase-wide query and antipattern audit Performance: - schedule_live: preload page once instead of get_schedule!/1 per row - alert_live + alerts: DB-side status filter; Repo.aggregate counts replace length/Enum.count over 500-row fetches on every event - dashboard_live: drop duplicate get_device_status_counts; cap active alerts to 20 + use count_active_alerts/1 - maintenance.active_windows_for_device: 3 round-trips collapsed into one query (per-site-id branch keeps Postgres parameter types unambiguous) - sites.build_site_tree: O(N^2) -> O(N) via group_by(parent_site_id) - accounts.sole_owner_organizations: single group_by + having instead of per-org Repo.aggregate loop - agents + agent_live (org + admin): count_assigned_devices_batch/1 + count_agent_polling_targets/1 (no preloads) - alert_digest_worker: list_alerts_by_ids/1 batches digest fetch - gaiia: distinct: true at DB; limit 50 on bidirectional ilike Indexes: - maintenance_windows(organization_id, starts_at, ends_at) WHERE suppress_alerts = true - alerts(check_id) WHERE resolved_at IS NULL Antipatterns: - agents.delete_agent_token: PubSub.broadcast moved outside Repo.transaction so a rollback no longer leaves subscribers acting on a non-existent deletion - integrations_controller.to_atom_keys: replaced String.to_existing_atom on user-controlled JSON keys with explicit allowlist - 9 Task.start callsites converted to Task.Supervisor.start_child(Towerops.TaskSupervisor, ...) so background DB writes survive shutdown (test-mode discovery shims left as-is for sandbox semantics) --- CHANGELOG.txt | 21 +++++++ lib/towerops/accounts.ex | 23 ++++--- lib/towerops/agents.ex | 62 ++++++++++++++++--- lib/towerops/alerts.ex | 50 +++++++++++++-- lib/towerops/application.ex | 2 +- lib/towerops/gaiia.ex | 3 +- lib/towerops/maintenance.ex | 48 +++++++------- lib/towerops/sites.ex | 16 ++--- lib/towerops/workers/alert_digest_worker.ex | 5 +- lib/towerops_web/channels/agent_channel.ex | 5 +- lib/towerops_web/channels/mobile_socket.ex | 6 +- .../api/v1/integrations_controller.ex | 21 ++++++- lib/towerops_web/graphql_socket.ex | 5 +- .../live/admin/agent_live/index.ex | 9 ++- lib/towerops_web/live/agent_live/index.ex | 7 ++- lib/towerops_web/live/alert_live/index.ex | 15 ++--- lib/towerops_web/live/dashboard_live.ex | 14 ++--- lib/towerops_web/live/schedule_live/index.ex | 7 ++- lib/towerops_web/plugs/graphql_auth.ex | 5 +- lib/towerops_web/plugs/mobile_auth.ex | 5 +- .../plugs/update_session_activity.ex | 7 ++- lib/towerops_web/user_auth.ex | 6 +- ...erf_indexes_for_maintenance_and_alerts.exs | 38 ++++++++++++ 23 files changed, 286 insertions(+), 94 deletions(-) create mode 100644 priv/repo/migrations/20260428164833_add_perf_indexes_for_maintenance_and_alerts.exs diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 285a28ac..d7d557d2 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -1,3 +1,24 @@ +2026-04-28 +perf+refactor: codebase-wide query/antipattern audit and fixes + Performance: + - schedule_live/index.ex: stop calling get_schedule!/1 per row in hydrate_schedules; preload page once (~5×N queries removed) + - alert_live/index.ex + alerts.ex: push status filter to DB; replace length/Enum.count with Repo.aggregate(:count) and a UI-aliased filter; added count_organization_alerts/2 + - dashboard_live.ex: drop duplicate get_device_status_counts call in mount; switch active alerts to limit:20 + count_active_alerts/1 instead of fetching all + - maintenance.ex: collapse active_windows_for_device's 3 round-trips into a single OR query + - sites.ex: build_site_tree O(N²) -> O(N) using Enum.group_by(parent_site_id) + map lookup + - accounts.ex: sole_owner_organizations replaces per-org Repo.aggregate loop with one group_by + having query + - agents.ex + agent_live: count_assigned_devices_batch/1 (one query per agents page) + count_agent_polling_targets/1 (no preloads); admin/agent_live and agent_live both updated + - alert_digest_worker.ex + alerts.ex: list_alerts_by_ids/1 batches digest fetch (3M queries -> 1) + - gaiia.ex: distinct: true at DB instead of Enum.uniq; suggest_site_matches limit 50 + - priv/repo/migrations/20260428164833_add_perf_indexes_for_maintenance_and_alerts.exs: + - maintenance_windows(organization_id, starts_at, ends_at) WHERE suppress_alerts = true + - alerts(check_id) WHERE resolved_at IS NULL + Antipatterns: + - agents.ex:delete_agent_token: moved Phoenix.PubSub.broadcast outside Repo.transaction so a rollback no longer leaves subscribers acting on a non-existent deletion + - integrations_controller.ex:to_atom_keys: removed String.to_existing_atom on user-controlled JSON keys; now uses an explicit allowlist + - Replaced unsupervised Task.start with Task.Supervisor.start_child(Towerops.TaskSupervisor, ...) in: api_tokens.ex, application.ex, user_auth.ex (3 sites), graphql_socket.ex, plugs/{graphql_auth, mobile_auth, update_session_activity}.ex, channels/{mobile_socket, agent_channel}.ex (test-mode discovery shims left as Task.start since they need test sandbox semantics) + Verification: mix format, mix compile --warnings-as-errors clean; full test suites for alerts, maintenance, sites, accounts, agents, gaiia, alert_live, dashboard_live, schedule_live, agent_live, site_live, user_auth, mobile_auth, integrations_controller, alert_digest_worker passing (560+ tests, 0 failures) + 2026-04-28 feat(on_call): deferred followups - drag-drop, restrictions, new-schedule UX Followup A — Restrictions editor diff --git a/lib/towerops/accounts.ex b/lib/towerops/accounts.ex index a478de3f..f4b6b47e 100644 --- a/lib/towerops/accounts.ex +++ b/lib/towerops/accounts.ex @@ -1790,28 +1790,27 @@ defmodule Towerops.Accounts do alias Towerops.Organizations.Membership alias Towerops.Organizations.Organization - # Find orgs where user is owner user_owner_org_ids = Membership |> where([m], m.user_id == ^user_id and m.role == :owner) |> select([m], m.organization_id) |> Repo.all() - # For each org, check if there are other owners - user_owner_org_ids - |> Enum.filter(fn org_id -> - owner_count = + if user_owner_org_ids == [] do + [] + else + sole_owner_ids = Membership - |> where([m], m.organization_id == ^org_id and m.role == :owner) - |> Repo.aggregate(:count, :id) + |> where([m], m.organization_id in ^user_owner_org_ids and m.role == :owner) + |> group_by([m], m.organization_id) + |> having([m], count(m.id) == 1) + |> select([m], m.organization_id) + |> Repo.all() - owner_count == 1 - end) - |> then(fn org_ids -> Organization - |> where([o], o.id in ^org_ids) + |> where([o], o.id in ^sole_owner_ids) |> Repo.all() - end) + end end @doc """ diff --git a/lib/towerops/agents.ex b/lib/towerops/agents.ex index 3cb56af2..c8bebd42 100644 --- a/lib/towerops/agents.ex +++ b/lib/towerops/agents.ex @@ -155,6 +155,23 @@ defmodule Towerops.Agents do |> Repo.aggregate(:count) end + @doc """ + Batch version of `count_assigned_devices/1`. Given a list of agent token ids, + returns a `%{agent_token_id => count}` map in a single query. Token ids + with no assignments are absent from the map (caller should default to 0). + """ + @spec count_assigned_devices_batch([Ecto.UUID.t()]) :: %{Ecto.UUID.t() => non_neg_integer()} + def count_assigned_devices_batch([]), do: %{} + + def count_assigned_devices_batch(agent_token_ids) when is_list(agent_token_ids) do + AgentAssignment + |> where([a], a.agent_token_id in ^agent_token_ids and a.enabled == true) + |> group_by([a], a.agent_token_id) + |> select([a], {a.agent_token_id, count(a.id)}) + |> Repo.all() + |> Map.new() + end + @doc """ Gets a single agent token by ID. @@ -470,8 +487,13 @@ defmodule Towerops.Agents do # 4. Delete the agent token Repo.delete!(agent_token) - # 5. Disconnect any active agent channel AFTER successful deletion (inside transaction) - # This ensures we only broadcast if the deletion actually succeeds + agent_token + end) + + case result do + {:ok, agent_token} -> + # Broadcast only after the transaction commits — otherwise a rollback + # would leave subscribers acting on a deletion that never happened. _ = Phoenix.PubSub.broadcast( Towerops.PubSub, @@ -479,11 +501,6 @@ defmodule Towerops.Agents do :token_disabled ) - agent_token - end) - - case result do - {:ok, agent_token} -> broadcast_agent_change(:agent_deleted, agent_token) {:ok, agent_token} @@ -719,6 +736,37 @@ defmodule Towerops.Agents do |> Repo.all() end + @doc """ + Counts devices polled by an agent (matches the same cascade as + `list_agent_polling_targets/1`) without preloading any assocs. + + Use this for UI counters instead of `length(list_agent_polling_targets(id))`, + which materializes every device + sensors + interfaces. + """ + @spec count_agent_polling_targets(Ecto.UUID.t()) :: non_neg_integer() + def count_agent_polling_targets(agent_token_id) do + global_default = Towerops.Settings.get_global_default_cloud_poller() + is_global_default = agent_token_id == global_default + + query = + from(e in Device, + left_join: s in assoc(e, :site), + join: o in assoc(e, :organization), + left_join: aa in AgentAssignment, + on: aa.device_id == e.id and aa.enabled == true, + left_join: disabled in AgentAssignment, + on: + disabled.device_id == e.id and disabled.agent_token_id == ^agent_token_id and + disabled.enabled == false, + where: e.snmp_enabled == true or e.monitoring_enabled == true + ) + + query + |> where_agent_matches(agent_token_id, is_global_default) + |> select([e], count(e.id)) + |> Repo.one() + end + # Adds where clause to match devices assigned to this agent. # Supports direct assignment, site cascade, org cascade, and global default. # A disabled assignment for this agent blocks cascade to prevent polling diff --git a/lib/towerops/alerts.ex b/lib/towerops/alerts.ex index 6966cdef..5b2f95ed 100644 --- a/lib/towerops/alerts.ex +++ b/lib/towerops/alerts.ex @@ -125,9 +125,9 @@ defmodule Towerops.Alerts do Uses denormalized organization_id for fast single-table query with composite index. """ - @spec list_organization_active_alerts(String.t()) :: [Alert.t()] - def list_organization_active_alerts(organization_id) do - Repo.all( + @spec list_organization_active_alerts(String.t(), keyword()) :: [Alert.t()] + def list_organization_active_alerts(organization_id, opts \\ []) do + query = from(a in Alert, where: a.organization_id == ^organization_id, where: a.alert_type == "device_down", @@ -135,7 +135,14 @@ defmodule Towerops.Alerts do order_by: [desc: a.triggered_at], preload: [:acknowledged_by, device: [:site]] ) - ) + + query = + case Keyword.get(opts, :limit) do + nil -> query + n when is_integer(n) -> from(a in query, limit: ^n) + end + + Repo.all(query) end @doc """ @@ -217,11 +224,30 @@ defmodule Towerops.Alerts do defp filter_by_status(query, "resolved"), do: where(query, [a], not is_nil(a.resolved_at)) + # UI filter aliases used by AlertLive.Index + defp filter_by_status(query, "unresolved"), do: where(query, [a], is_nil(a.resolved_at)) + + defp filter_by_status(query, "critical"), do: where(query, [a], a.alert_type == "device_down" and is_nil(a.resolved_at)) + defp filter_by_status(query, _), do: query defp filter_by_device(query, nil), do: query defp filter_by_device(query, device_id), do: where(query, [a], a.device_id == ^device_id) + @doc """ + Counts alerts for an organization, optionally filtered by UI status + ("all", "critical", "unresolved", "resolved", "active", "acknowledged"). + + Uses the composite index on (organization_id, alert_type, resolved_at) and + Repo.aggregate so it is cheap to call for counter badges. + """ + @spec count_organization_alerts(String.t(), String.t() | nil) :: non_neg_integer() + def count_organization_alerts(organization_id, status \\ nil) do + from(a in Alert, where: a.organization_id == ^organization_id) + |> filter_by_status(status) + |> Repo.aggregate(:count, :id) + end + @doc """ Returns alerts for multiple organizations (for GDPR data access). Accepts a `since` option to filter alerts created after a specific date. @@ -277,6 +303,22 @@ defmodule Towerops.Alerts do end end + @doc """ + Returns alerts for a list of ids in a single query, with `[:device, :acknowledged_by]` + preloaded. Missing ids are silently dropped. + """ + @spec list_alerts_by_ids([Ecto.UUID.t()]) :: [Alert.t()] + def list_alerts_by_ids([]), do: [] + + def list_alerts_by_ids(ids) when is_list(ids) do + Repo.all( + from(a in Alert, + where: a.id in ^ids, + preload: [:device, :acknowledged_by] + ) + ) + end + @doc """ Acknowledges an alert. """ diff --git a/lib/towerops/application.ex b/lib/towerops/application.ex index 94a30638..c3335111 100644 --- a/lib/towerops/application.ex +++ b/lib/towerops/application.ex @@ -148,7 +148,7 @@ defmodule Towerops.Application do # Run post-startup cleanup tasks (production only) # This ensures all polling jobs use the latest worker code after deployment _ = - Task.start(fn -> + Task.Supervisor.start_child(Towerops.TaskSupervisor, fn -> # Wait for supervisor to fully initialize Process.sleep(2000) JobCleanupTask.run() diff --git a/lib/towerops/gaiia.ex b/lib/towerops/gaiia.ex index 2dcf1faa..45fe7277 100644 --- a/lib/towerops/gaiia.ex +++ b/lib/towerops/gaiia.ex @@ -206,6 +206,7 @@ defmodule Towerops.Gaiia do Site |> where(organization_id: ^organization_id) |> where([s], ilike(s.name, ^search_term) or ilike(^gaiia_name, fragment("'%' || ? || '%'", s.name))) + |> limit(50) |> Repo.all() |> Enum.map(fn site -> %{entity: site, match_type: "name", confidence: :medium} end) end @@ -309,9 +310,9 @@ defmodule Towerops.Gaiia do account_ids = DeviceSubscriberLink |> where(device_id: ^device_id) + |> distinct(true) |> select([l], l.gaiia_account_id) |> Repo.all() - |> Enum.uniq() if account_ids == [] do %{subscriber_count: 0, mrr: Decimal.new(0), accounts: []} diff --git a/lib/towerops/maintenance.ex b/lib/towerops/maintenance.ex index fbdb144c..d1a922e3 100644 --- a/lib/towerops/maintenance.ex +++ b/lib/towerops/maintenance.ex @@ -91,34 +91,36 @@ defmodule Towerops.Maintenance do """ def active_windows_for_device(device_id) do now = DateTime.utc_now() - device = Repo.get!(Device, device_id) - active_base = - MaintenanceWindow - |> where([w], w.organization_id == ^device.organization_id) - |> where([w], w.starts_at <= ^now and w.ends_at >= ^now) - |> where([w], w.suppress_alerts == true) + MaintenanceWindow + |> active_windows_base(device.organization_id, now) + |> scope_to_device(device_id, device.site_id) + |> Repo.all() + end - # Direct device windows - direct = where(active_base, [w], w.device_id == ^device_id) + defp active_windows_base(query, organization_id, now) do + from(w in query, + where: + w.organization_id == ^organization_id and + w.starts_at <= ^now and w.ends_at >= ^now and + w.suppress_alerts == true + ) + end - # Site windows (if device has a site) - site_query = - if device.site_id do - where(active_base, [w], w.site_id == ^device.site_id and is_nil(w.device_id)) - else - where(active_base, [w], false) - end + defp scope_to_device(query, device_id, nil) do + from(w in query, + where: w.device_id == ^device_id or (is_nil(w.site_id) and is_nil(w.device_id)) + ) + end - # Org-wide windows - org_wide = where(active_base, [w], is_nil(w.site_id) and is_nil(w.device_id)) - - direct_results = Repo.all(direct) - site_results = Repo.all(site_query) - org_results = Repo.all(org_wide) - - Enum.uniq_by(direct_results ++ site_results ++ org_results, & &1.id) + defp scope_to_device(query, device_id, site_id) do + from(w in query, + where: + w.device_id == ^device_id or + (w.site_id == ^site_id and is_nil(w.device_id)) or + (is_nil(w.site_id) and is_nil(w.device_id)) + ) end @doc """ diff --git a/lib/towerops/sites.ex b/lib/towerops/sites.ex index b97caebc..731adce3 100644 --- a/lib/towerops/sites.ex +++ b/lib/towerops/sites.ex @@ -180,18 +180,18 @@ defmodule Towerops.Sites do """ def build_site_tree(organization_id) do sites = list_organization_sites(organization_id) - root_sites = Enum.filter(sites, &is_nil(&1.parent_site_id)) + children_by_parent = Enum.group_by(sites, & &1.parent_site_id) - Enum.map(root_sites, fn root -> - build_tree_node(root, sites) - end) + children_by_parent + |> Map.get(nil, []) + |> Enum.map(&build_tree_node(&1, children_by_parent)) end - defp build_tree_node(site, all_sites) do + defp build_tree_node(site, children_by_parent) do children = - all_sites - |> Enum.filter(&(&1.parent_site_id == site.id)) - |> Enum.map(&build_tree_node(&1, all_sites)) + children_by_parent + |> Map.get(site.id, []) + |> Enum.map(&build_tree_node(&1, children_by_parent)) %{site: site, children: children} end diff --git a/lib/towerops/workers/alert_digest_worker.ex b/lib/towerops/workers/alert_digest_worker.ex index fdefade3..ac824862 100644 --- a/lib/towerops/workers/alert_digest_worker.ex +++ b/lib/towerops/workers/alert_digest_worker.ex @@ -56,10 +56,7 @@ defmodule Towerops.Workers.AlertDigestWorker do alert_ids = digest.suppressed_alert_ids || [] with [_ | _] <- alert_ids, - alerts = - alert_ids - |> Enum.map(&Alerts.get_alert/1) - |> Enum.reject(&is_nil/1), + alerts = Alerts.list_alerts_by_ids(alert_ids), [_ | _] <- alerts do send_digest_notification(alerts) NotificationRateLimiter.mark_digest_sent(digest) diff --git a/lib/towerops_web/channels/agent_channel.ex b/lib/towerops_web/channels/agent_channel.ex index 7cc192de..8fd54ff3 100644 --- a/lib/towerops_web/channels/agent_channel.ex +++ b/lib/towerops_web/channels/agent_channel.ex @@ -2314,7 +2314,10 @@ defmodule ToweropsWeb.AgentChannel do case ConfigChanges.record_change_event(device_id, previous, new_backup) do {:ok, %{id: event_id} = event} -> Logger.info("Config change event recorded", device_id: device_id, event_id: event_id) - Task.start(fn -> ConfigChanges.Correlator.correlate(event) end) + + Task.Supervisor.start_child(Towerops.TaskSupervisor, fn -> + ConfigChanges.Correlator.correlate(event) + end) {:ok, :no_changes} -> :ok diff --git a/lib/towerops_web/channels/mobile_socket.ex b/lib/towerops_web/channels/mobile_socket.ex index b25860fe..5d24ec6d 100644 --- a/lib/towerops_web/channels/mobile_socket.ex +++ b/lib/towerops_web/channels/mobile_socket.ex @@ -27,7 +27,11 @@ defmodule ToweropsWeb.MobileSocket do session -> session = Towerops.Repo.preload(session, :user) - _ = Task.start(fn -> MobileSessions.touch_session(session) end) + + _ = + Task.Supervisor.start_child(Towerops.TaskSupervisor, fn -> + MobileSessions.touch_session(session) + end) socket = socket diff --git a/lib/towerops_web/controllers/api/v1/integrations_controller.ex b/lib/towerops_web/controllers/api/v1/integrations_controller.ex index 49e433d9..0f41554d 100644 --- a/lib/towerops_web/controllers/api/v1/integrations_controller.ex +++ b/lib/towerops_web/controllers/api/v1/integrations_controller.ex @@ -109,9 +109,24 @@ defmodule ToweropsWeb.Api.V1.IntegrationsController do } end + # Allowlist of integration attribute keys that can be set via the API. + # We avoid String.to_existing_atom on user input so an attacker cannot probe + # for atoms or trigger ArgumentError 500s. Unknown keys are silently dropped + # by the changeset's `cast/3` allowlist. + @allowed_keys ~w( + provider + enabled + config + api_key + api_url + sync_interval_minutes + ) + defp to_atom_keys(map) when is_map(map) do - Map.new(map, fn {k, v} -> {String.to_existing_atom(k), v} end) - rescue - ArgumentError -> map + for {k, v} <- map, k in @allowed_keys, into: %{} do + {String.to_atom(k), v} + end end + + defp to_atom_keys(other), do: other end diff --git a/lib/towerops_web/graphql_socket.ex b/lib/towerops_web/graphql_socket.ex index 2167b477..576df4cd 100644 --- a/lib/towerops_web/graphql_socket.ex +++ b/lib/towerops_web/graphql_socket.ex @@ -35,7 +35,10 @@ defmodule ToweropsWeb.GraphQLSocket do def connect(%{"token" => token}, socket, _connect_info) when is_binary(token) do with session when not is_nil(session) <- MobileSessions.get_session_by_token(token), user when not is_nil(user) <- Accounts.get_user(session.user_id) do - _ = Task.start(fn -> MobileSessions.touch_session(session) end) + _ = + Task.Supervisor.start_child(Towerops.TaskSupervisor, fn -> + MobileSessions.touch_session(session) + end) socket = socket diff --git a/lib/towerops_web/live/admin/agent_live/index.ex b/lib/towerops_web/live/admin/agent_live/index.ex index 0c6c7509..f98c9a2c 100644 --- a/lib/towerops_web/live/admin/agent_live/index.ex +++ b/lib/towerops_web/live/admin/agent_live/index.ex @@ -72,7 +72,7 @@ defmodule ToweropsWeb.Admin.AgentLive.Index do agent_token = agent_token_id |> Agents.get_agent_token!() |> Towerops.Repo.preload(:organization) direct = Agents.count_assigned_devices(agent_token_id) - total = length(Agents.list_agent_polling_targets(agent_token_id)) + total = Agents.count_agent_polling_targets(agent_token_id) {stream_name, section_assign} = if agent_token.is_cloud_poller, @@ -101,9 +101,12 @@ defmodule ToweropsWeb.Admin.AgentLive.Index do end defp calculate_device_counts(agent_tokens) do + token_ids = Enum.map(agent_tokens, & &1.id) + direct_counts = Agents.count_assigned_devices_batch(token_ids) + Map.new(agent_tokens, fn t -> - direct = Agents.count_assigned_devices(t.id) - total = length(Agents.list_agent_polling_targets(t.id)) + direct = Map.get(direct_counts, t.id, 0) + total = Agents.count_agent_polling_targets(t.id) {t.id, %{direct: direct, total: total}} end) end diff --git a/lib/towerops_web/live/agent_live/index.ex b/lib/towerops_web/live/agent_live/index.ex index b5091b05..4f19e1b3 100644 --- a/lib/towerops_web/live/agent_live/index.ex +++ b/lib/towerops_web/live/agent_live/index.ex @@ -276,9 +276,12 @@ defmodule ToweropsWeb.AgentLive.Index do end defp calculate_device_counts(agent_tokens) do + token_ids = Enum.map(agent_tokens, & &1.id) + direct_counts = Agents.count_assigned_devices_batch(token_ids) + Map.new(agent_tokens, fn t -> - direct = Agents.count_assigned_devices(t.id) - total = length(Agents.list_agent_polling_targets(t.id)) + direct = Map.get(direct_counts, t.id, 0) + total = Agents.count_agent_polling_targets(t.id) {t.id, %{direct: direct, total: total}} end) end diff --git a/lib/towerops_web/live/alert_live/index.ex b/lib/towerops_web/live/alert_live/index.ex index efeea1da..e714c3e7 100644 --- a/lib/towerops_web/live/alert_live/index.ex +++ b/lib/towerops_web/live/alert_live/index.ex @@ -194,19 +194,20 @@ defmodule ToweropsWeb.AlertLive.Index do def handle_info(_msg, socket), do: {:noreply, socket} defp load_alerts(socket, organization_id) do - all_alerts = Alerts.list_organization_alerts(organization_id, %{"limit" => 500}) + filter = socket.assigns.filter + + filtered_alerts = + Alerts.list_organization_alerts(organization_id, %{"limit" => 500, "status" => filter}) - filtered_alerts = filter_alerts(all_alerts, socket.assigns.filter) site_subscribers = load_site_subscribers(filtered_alerts) sorted_alerts = sort_alerts(filtered_alerts, socket.assigns.sort_by, site_subscribers) grouped = group_by_site(sorted_alerts, site_subscribers) - # Compute counts for filter tabs counts = %{ - all: length(all_alerts), - critical: Enum.count(all_alerts, &(&1.alert_type == "device_down" and is_nil(&1.resolved_at))), - unresolved: Enum.count(all_alerts, &is_nil(&1.resolved_at)), - resolved: Enum.count(all_alerts, &(not is_nil(&1.resolved_at))) + all: Alerts.count_organization_alerts(organization_id), + critical: Alerts.count_organization_alerts(organization_id, "critical"), + unresolved: Alerts.count_organization_alerts(organization_id, "unresolved"), + resolved: Alerts.count_organization_alerts(organization_id, "resolved") } socket diff --git a/lib/towerops_web/live/dashboard_live.ex b/lib/towerops_web/live/dashboard_live.ex index af5c28eb..414e2757 100644 --- a/lib/towerops_web/live/dashboard_live.ex +++ b/lib/towerops_web/live/dashboard_live.ex @@ -21,13 +21,10 @@ defmodule ToweropsWeb.DashboardLive do Phoenix.PubSub.subscribe(Towerops.PubSub, "alerts:org:#{organization.id}:resolved") end - status_counts = Devices.get_device_status_counts(organization.id) - device_count = status_counts |> Map.values() |> Enum.sum() - {:ok, socket |> assign(:page_title, organization.name) - |> assign(:device_count, device_count) + |> assign(:device_count, 0) |> assign(:insight_source, nil) |> assign(:timezone, socket.assigns.current_scope.timezone) |> assign(:time_format, socket.assigns.current_scope.time_format) @@ -112,7 +109,8 @@ defmodule ToweropsWeb.DashboardLive do defp load_dashboard_data(socket, organization_id) do organization = socket.assigns.current_scope.organization summary = Dashboard.get_dashboard_summary(organization_id) - active_alerts = Alerts.list_organization_active_alerts(organization_id) + active_alerts = Alerts.list_organization_active_alerts(organization_id, limit: 20) + total_alert_count = Alerts.count_active_alerts(organization_id) sites_count = if organization.use_sites, do: Sites.count_organization_sites(organization_id), else: 0 status_counts = Devices.get_device_status_counts(organization_id) @@ -154,8 +152,8 @@ defmodule ToweropsWeb.DashboardLive do socket |> assign(:summary, summary) - |> assign(:active_alerts, Enum.take(active_alerts, 20)) - |> assign(:total_alert_count, length(active_alerts)) + |> assign(:active_alerts, active_alerts) + |> assign(:total_alert_count, total_alert_count) |> assign(:sites_count, sites_count) |> assign(:device_count, device_count) |> assign(:device_up, device_up) @@ -168,7 +166,7 @@ defmodule ToweropsWeb.DashboardLive do |> assign(:site_impact_summaries, site_impact_summaries) |> assign(:uptime_percentage, uptime_percentage) |> assign(:recent_activity, recent_activity) - |> assign(:favicon_status, favicon_status(Map.get(status_counts, :down, 0), length(active_alerts))) + |> assign(:favicon_status, favicon_status(Map.get(status_counts, :down, 0), total_alert_count)) |> assign(:setup_checklist, build_setup_checklist(organization, device_count)) |> load_insights(organization_id) |> load_recent_config_changes(organization_id) diff --git a/lib/towerops_web/live/schedule_live/index.ex b/lib/towerops_web/live/schedule_live/index.ex index 642b3945..cd33f090 100644 --- a/lib/towerops_web/live/schedule_live/index.ex +++ b/lib/towerops_web/live/schedule_live/index.ex @@ -137,8 +137,11 @@ defmodule ToweropsWeb.ScheduleLive.Index do {:ok, start_at} = DateTime.new(start_date, ~T[00:00:00], "Etc/UTC") {:ok, end_at} = DateTime.new(end_date, ~T[00:00:00], "Etc/UTC") - Enum.map(schedules, fn schedule -> - preloaded = OnCall.get_schedule!(schedule.id) + preloaded_schedules = + Towerops.Repo.preload(schedules, overrides: :user, layers: [members: :user]) + + Enum.map(preloaded_schedules, fn preloaded -> + schedule = preloaded on_call = Resolver.resolve(preloaded, DateTime.utc_now()) segments = diff --git a/lib/towerops_web/plugs/graphql_auth.ex b/lib/towerops_web/plugs/graphql_auth.ex index df707506..e4e86590 100644 --- a/lib/towerops_web/plugs/graphql_auth.ex +++ b/lib/towerops_web/plugs/graphql_auth.ex @@ -43,7 +43,10 @@ defmodule ToweropsWeb.Plugs.GraphQLAuth do defp authenticate(conn, token) do with {:ok, session} <- validate_mobile_session(token), {:ok, user} <- get_user(session.user_id) do - _ = Task.start(fn -> MobileSessions.touch_session(session) end) + _ = + Task.Supervisor.start_child(Towerops.TaskSupervisor, fn -> + MobileSessions.touch_session(session) + end) assign(conn, :current_user, user) else diff --git a/lib/towerops_web/plugs/mobile_auth.ex b/lib/towerops_web/plugs/mobile_auth.ex index 65adce79..2637db73 100644 --- a/lib/towerops_web/plugs/mobile_auth.ex +++ b/lib/towerops_web/plugs/mobile_auth.ex @@ -21,7 +21,10 @@ defmodule ToweropsWeb.Plugs.MobileAuth do {:ok, session} <- validate_session(token), {:ok, user} <- get_user(session.user_id) do # Touch the session to update last_used_at - _ = Task.start(fn -> MobileSessions.touch_session(session) end) + _ = + Task.Supervisor.start_child(Towerops.TaskSupervisor, fn -> + MobileSessions.touch_session(session) + end) conn |> assign(:current_user, user) diff --git a/lib/towerops_web/plugs/update_session_activity.ex b/lib/towerops_web/plugs/update_session_activity.ex index 84001db2..628f997c 100644 --- a/lib/towerops_web/plugs/update_session_activity.ex +++ b/lib/towerops_web/plugs/update_session_activity.ex @@ -42,7 +42,12 @@ defmodule ToweropsWeb.Plugs.UpdateSessionActivity do token -> live_socket_id = get_session(conn, :live_socket_id) - _ = Task.start(fn -> handle_session_activity(token, live_socket_id) end) + + _ = + Task.Supervisor.start_child(Towerops.TaskSupervisor, fn -> + handle_session_activity(token, live_socket_id) + end) + conn end end diff --git a/lib/towerops_web/user_auth.ex b/lib/towerops_web/user_auth.ex index 23681634..cae73c8d 100644 --- a/lib/towerops_web/user_auth.ex +++ b/lib/towerops_web/user_auth.ex @@ -189,13 +189,13 @@ defmodule ToweropsWeb.UserAuth do # Create browser session in background with metadata _ = - Task.start(fn -> + Task.Supervisor.start_child(Towerops.TaskSupervisor, fn -> create_browser_session_from_conn(conn, user, user_token) end) # Record successful login attempt in background _ = - Task.start(fn -> + Task.Supervisor.start_child(Towerops.TaskSupervisor, fn -> record_successful_login(conn, user, params) end) @@ -817,7 +817,7 @@ defmodule ToweropsWeb.UserAuth do # If user is logged in but doesn't have a timezone set, update their profile _ = if user && !user.timezone && connect_timezone do - Task.start(fn -> + Task.Supervisor.start_child(Towerops.TaskSupervisor, fn -> Accounts.update_user_profile(user, %{timezone: connect_timezone}) end) end diff --git a/priv/repo/migrations/20260428164833_add_perf_indexes_for_maintenance_and_alerts.exs b/priv/repo/migrations/20260428164833_add_perf_indexes_for_maintenance_and_alerts.exs new file mode 100644 index 00000000..27170f05 --- /dev/null +++ b/priv/repo/migrations/20260428164833_add_perf_indexes_for_maintenance_and_alerts.exs @@ -0,0 +1,38 @@ +defmodule Towerops.Repo.Migrations.AddPerfIndexesForMaintenanceAndAlerts do + use Ecto.Migration + + @disable_ddl_transaction true + @disable_migration_lock true + + def up do + # Hot path: maintenance_windows is checked per alert creation. The query + # filters by org + active time range + suppress_alerts. A partial composite + # index avoids scanning expired/non-suppressing windows. + create_if_not_exists index( + :maintenance_windows, + [:organization_id, :starts_at, :ends_at], + name: :maintenance_windows_active_suppress_idx, + where: "suppress_alerts = true", + concurrently: true + ) + + # Resolving alerts on check disable / delete touches only unresolved rows. + # A partial index over the (typically small) unresolved set is much smaller + # than the existing full check_id index and matches the resolve query. + create_if_not_exists index( + :alerts, + [:check_id], + name: :alerts_check_id_unresolved_idx, + where: "resolved_at IS NULL", + concurrently: true + ) + end + + def down do + drop_if_exists index(:alerts, [:check_id], name: :alerts_check_id_unresolved_idx) + + drop_if_exists index(:maintenance_windows, [:organization_id, :starts_at, :ends_at], + name: :maintenance_windows_active_suppress_idx + ) + end +end