diff --git a/findings.md b/findings.md index 19ef6313..2b313035 100644 --- a/findings.md +++ b/findings.md @@ -7,14 +7,14 @@ | # | File | Line | Issue | Suggested Fix | |---|------|------|-------|---------------| | 1 | `radio.ex` | 128, 157 | N+1 queries — `list_contacts_for_user` and `list_contacts_involving_callsign` don't preload `:user` | Add `|> Repo.preload(:user)` before `Repo.all()` | -| 2 | `radio.ex` | 751-757 | Missing indexes — dedup query filters on `band`, `qso_timestamp`, `flagged_invalid` with no covering index | Add partial index on `(band, qso_timestamp) WHERE flagged_invalid = false` | -| 3 | `grid.ex` | 31-36 | `conus_points/0` recomputes ~94k elements on every call — pure function, never memoized | Memoize with module attribute `@conus_points conus_points()` | +| ~2~ | `radio.ex` | 751-757 | ~~Missing indexes — dedup query~~ | *(fixed — `contacts_dedup_idx`)* | +| ~3~ | `grid.ex` | 31-36 | ~~Memoize `conus_points/0`~~ | *(fixed — module attribute)* | | 4 | `endpoint.ex` | 7-12 | Session cookie is signed but not encrypted — payload is readable though tamper-proof | Add `encryption_salt` to `@session_options` | | 5 | `accounts.ex` | 528-538 | Fetches all tokens before deleting them — two queries where one suffices | Use `Repo.delete_all(from t in UserToken, where: t.user_id == ^user.id)` | | 6 | `profiles_file.ex` | 169-177 | Unhandled `:zlib.gunzip/1` exception — corrupt gzip files raise through cache wrapper | Wrap in `try/rescue` | | 7 | `scores_file.ex` | 515 | Direct ETS `match_delete` bypassing Cache API | Use `Cache.invalidate` or `Cache` public API | | 8 | `profiles_file.ex` | 323 | Same direct ETS access as above | Same fix | -| 9 | `path_compute.ex` | 381-394 | Six redundant list traversals — 12 passes over 9 profiles for what could be 1 `Enum.reduce` | Single pass with `Enum.reduce` | +| ~9~ | `path_compute.ex` | 381-394 | ~~Six redundant list traversals~~ | *(fixed — single `Enum.reduce`)* | | 10 | `hf_muf.ex` | 51 | `sec_i_factor(@ref_distance_km)` recomputed on every call — constant input | Precompute with module attribute | | 11 | `config/config.exs` | 163 | `EXLA.Backend` configured for all envs but `nx`/`exla` are dev/test-only deps | Guard with `if Mix.env() in [:dev, :test]` | @@ -29,7 +29,7 @@ | 16 | `path_compute.ex` | 356-357 | Eager `Repo.get(Station, ...)` instead of preloading assoc | Preload `:station` upstream | | 17 | `radio.ex` | 1203 | Hardcoded cache key `{ContactMapController, :gzipped_payload}` fragile to module rename | Extract to a named key in `ContactMapController` | | 18 | `user.ex` | — | Missing `has_many :contacts` and `has_many :beacons` associations | Add for convenience (not a bug, test callers use raw queries) | -| 19 | `radio.ex` | 337 | Missing partial indexes for enrichment queries | Create filtered indexes on `(qso_timestamp) WHERE weather_status IN ('pending','failed')` | +| ~19~ | `radio.ex` | 337 | ~~Missing enrichment query partial indexes~~ | *(fixed — 5 `qso_timestamp` partial indexes added)* | --- @@ -65,14 +65,7 @@ | 4 | `Credo.Check.Readability.Specs` disabled | Consider re-enabling for production codebase | | 5 | `Credo.Check.Warning.LeakyEnvironment` disabled | Re-enable to catch accidental env var logging | -### Performance Hotspots - -| # | Area | Impact | -|---|------|--------| -| 1 | `grid.ex:31` — `conus_points()` rebuilds ~94k items per call | **Highest impact fix** — memoize with `@conus_points` | -| 2 | `radio.ex` dedup queries — missing indexes | Growing table, will degrade over time | -| 3 | Enrichment queries — missing partial indexes | Each `contacts_needing_enrichment` call scans entire table | -| 4 | `path_compute.ex:381-394` — 12 list traversals for 9-element profile lists | Small today, but unnecessary overhead in hot path | +~~Performance Hotspots~~ (all fixed) ### Security (Remaining) diff --git a/lib/microwaveprop/application.ex b/lib/microwaveprop/application.ex index 14d8f879..c02f4302 100644 --- a/lib/microwaveprop/application.ex +++ b/lib/microwaveprop/application.ex @@ -81,13 +81,8 @@ defmodule Microwaveprop.Application do # the health check will catch schema mismatches. Task.start(fn -> try do - case Microwaveprop.Release.migrate() do - results when is_list(results) -> - Logger.info("Database migrations completed (#{length(results)} repos)") - - other -> - Logger.warning("Database migrations returned: #{inspect(other)}") - end + results = Microwaveprop.Release.migrate() + Logger.info("Database migrations completed (#{length(results)} repos)") rescue e -> Logger.error("Database migrations failed: #{Exception.format(:error, e, __STACKTRACE__)}") diff --git a/lib/microwaveprop/propagation/grid.ex b/lib/microwaveprop/propagation/grid.ex index 4e7a0285..485ea155 100644 --- a/lib/microwaveprop/propagation/grid.ex +++ b/lib/microwaveprop/propagation/grid.ex @@ -26,13 +26,24 @@ defmodule Microwaveprop.Propagation.Grid do @hrdps_lon_min -141.0 @hrdps_lon_max -52.0 + @conus_points for( + lat <- + Enum.map( + 0..round((@lat_max - @lat_min) / @step), + fn i -> Float.round(@lat_min + i * @step, 3) end + ), + lon <- + Enum.map( + 0..round((@lon_max - @lon_min) / @step), + fn i -> Float.round(@lon_min + i * @step, 3) end + ), + do: {lat, lon} + ) + @doc "Returns all grid points as `{lat, lon}` tuples covering CONUS at 0.125 degree spacing." @spec conus_points() :: [{float(), float()}] def conus_points do - for lat <- float_range(@lat_min, @lat_max, @step), - lon <- float_range(@lon_min, @lon_max, @step) do - {Float.round(lat, 3), Float.round(lon, 3)} - end + @conus_points end @doc "Returns the grid step size in degrees." diff --git a/lib/microwaveprop/propagation/path_compute.ex b/lib/microwaveprop/propagation/path_compute.ex index 296b4961..4bb28dba 100644 --- a/lib/microwaveprop/propagation/path_compute.ex +++ b/lib/microwaveprop/propagation/path_compute.ex @@ -378,8 +378,19 @@ defmodule Microwaveprop.Propagation.PathCompute do defp build_scoring([], _src, _dst, _now, _band_config, _native_duct), do: {nil, nil} defp build_scoring(profiles, src, dst, now, band_config, native_duct) do - temps = profiles |> Enum.map(& &1.surface_temp_c) |> Enum.reject(&is_nil/1) - dewpoints = profiles |> Enum.map(& &1.surface_dewpoint_c) |> Enum.reject(&is_nil/1) + {temps, dewpoints, pressures, gradients, bl_depths, pwats} = + Enum.reduce(profiles, {[], [], [], [], [], []}, fn p, + {ts, ds, ps, gs, bs, ws} -> + { + if(p.surface_temp_c != nil, do: [p.surface_temp_c | ts], else: ts), + if(p.surface_dewpoint_c != nil, do: [p.surface_dewpoint_c | ds], else: ds), + if(p.surface_pressure_mb != nil, do: [p.surface_pressure_mb | ps], else: ps), + if(p.min_refractivity_gradient != nil, do: [p.min_refractivity_gradient | gs], + else: gs), + if(p.hpbl_m != nil, do: [p.hpbl_m | bs], else: bs), + if(p.pwat_mm != nil, do: [p.pwat_mm | ws], else: ws) + } + end) if temps == [] or dewpoints == [] do {nil, nil} @@ -387,11 +398,6 @@ defmodule Microwaveprop.Propagation.PathCompute do avg_temp_c = Enum.sum(temps) / length(temps) avg_dewpoint_c = Enum.sum(dewpoints) / length(dewpoints) - pressures = profiles |> Enum.map(& &1.surface_pressure_mb) |> Enum.reject(&is_nil/1) - gradients = profiles |> Enum.map(& &1.min_refractivity_gradient) |> Enum.reject(&is_nil/1) - bl_depths = profiles |> Enum.map(& &1.hpbl_m) |> Enum.reject(&is_nil/1) - pwats = profiles |> Enum.map(& &1.pwat_mm) |> Enum.reject(&is_nil/1) - conditions = %{ abs_humidity: Scorer.absolute_humidity(avg_temp_c, avg_dewpoint_c), temp_f: Scorer.c_to_f(avg_temp_c), diff --git a/lib/microwaveprop/radio.ex b/lib/microwaveprop/radio.ex index 26664f50..210d84d3 100644 --- a/lib/microwaveprop/radio.ex +++ b/lib/microwaveprop/radio.ex @@ -733,7 +733,7 @@ defmodule Microwaveprop.Radio do {:error, %{changeset | action: :insert}} end rescue - e in Ecto.ConstraintError -> + _e in Ecto.ConstraintError -> # Unique constraint violation from a concurrent insert — the # dedup check at `insert_validated_contact` raced with another # process. Re-check and return the existing contact. diff --git a/priv/repo/migrations/20260529203624_add_enrichment_query_qso_timestamp_indexes.exs b/priv/repo/migrations/20260529203624_add_enrichment_query_qso_timestamp_indexes.exs new file mode 100644 index 00000000..9459e799 --- /dev/null +++ b/priv/repo/migrations/20260529203624_add_enrichment_query_qso_timestamp_indexes.exs @@ -0,0 +1,25 @@ +defmodule Microwaveprop.Repo.Migrations.AddEnrichmentQueryQsoTimestampIndexes do + use Ecto.Migration + + def change do + create index(:contacts, [:qso_timestamp], + where: "weather_status IN ('pending', 'failed') AND pos1 IS NOT NULL", + name: :contacts_weather_enrichment_qso_idx) + + create index(:contacts, [:qso_timestamp], + where: "hrrr_status IN ('pending', 'failed') AND pos1 IS NOT NULL", + name: :contacts_hrrr_enrichment_qso_idx) + + create index(:contacts, [:qso_timestamp], + where: "terrain_status IN ('pending', 'failed') AND pos1 IS NOT NULL", + name: :contacts_terrain_enrichment_qso_idx) + + create index(:contacts, [:qso_timestamp], + where: "iemre_status IN ('pending', 'failed') AND pos1 IS NOT NULL", + name: :contacts_iemre_enrichment_qso_idx) + + create index(:contacts, [:qso_timestamp], + where: "radar_status IN ('pending', 'failed') AND pos1 IS NOT NULL", + name: :contacts_radar_enrichment_qso_idx) + end +end