Skip to content

Commit

Permalink
perf: reduce API request volume on DUPs
Browse files Browse the repository at this point in the history
Previously DUPs made one routes-serving-stop API request for every stop
ID configured for every section. This may be inefficient given the API
supports filtering by many stop IDs in one request, and suspiciously,
DUPs configured with more stop IDs seem to encounter the "Failed to get
section data" crash more often.

This enhances our routes-serving-stop fetching to support more than one
stop ID, and passes in the entire list of stop IDs for each DUP section.
  • Loading branch information
digitalcora committed Sep 4, 2024
1 parent ecd0415 commit 28f32d6
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 76 deletions.
10 changes: 5 additions & 5 deletions lib/screens/routes/route.ex
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ defmodule Screens.Routes.Route do
end
end

@doc "Fetches routes that serve the given stop."
@spec serving_stop(Stop.id()) :: {:ok, [t()]} | :error
def serving_stop(stop_id, get_json_fn \\ &V3Api.get_json/2) do
case get_json_fn.("routes", %{"filter[stop]" => stop_id}) do
@doc "Fetches routes that serve the given stops."
@spec serving_stops([Stop.id()]) :: {:ok, [t()]} | :error
def serving_stops(stop_ids, get_json_fn \\ &V3Api.get_json/2) do
case get_json_fn.("routes", %{"filter[stop]" => Enum.join(stop_ids, ",")}) do
{:ok, %{"data" => data}} ->
{:ok, Enum.map(data, fn route -> Parser.parse_route(route) end)}

Expand All @@ -72,7 +72,7 @@ defmodule Screens.Routes.Route do
end

@doc """
Similar to `serving_stop` but also determines whether each route has any scheduled service at
Similar to `serving_stops` but also determines whether each route has any scheduled service at
the given stop on the current day. Only route IDs and the `active?` flag are returned.
"""
@spec serving_stop_with_active(Stop.id()) ::
Expand Down
23 changes: 10 additions & 13 deletions lib/screens/v2/candidate_generator/dup/departures.ex
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@ defmodule Screens.V2.CandidateGenerator.Dup.Departures do
fetch_departures_fn \\ &Departure.fetch/2,
fetch_alerts_fn \\ &Alert.fetch_or_empty_list/1,
fetch_schedules_fn \\ &Screens.Schedules.Schedule.fetch/2,
fetch_routes_serving_stop_fn \\ &Screens.Routes.Route.serving_stop/1,
fetch_routes_serving_stops_fn \\ &Screens.Routes.Route.serving_stops/1,
fetch_vehicles_fn \\ &Screens.Vehicles.Vehicle.by_route_and_direction/2
) do
primary_departures_instances =
primary_sections
|> get_sections_data(
fetch_departures_fn,
fetch_alerts_fn,
fetch_routes_serving_stop_fn,
fetch_routes_serving_stops_fn,
now
)
|> sections_data_to_departure_instances(
Expand Down Expand Up @@ -64,7 +64,7 @@ defmodule Screens.V2.CandidateGenerator.Dup.Departures do
|> get_sections_data(
fetch_departures_fn,
fetch_alerts_fn,
fetch_routes_serving_stop_fn,
fetch_routes_serving_stops_fn,
now
)
|> sections_data_to_departure_instances(
Expand Down Expand Up @@ -208,7 +208,7 @@ defmodule Screens.V2.CandidateGenerator.Dup.Departures do
sections,
fetch_departures_fn,
fetch_alerts_fn,
fetch_routes_serving_stop_fn,
fetch_routes_serving_stops_fn,
now
) do
Screens.Telemetry.span(
Expand All @@ -222,7 +222,7 @@ defmodule Screens.V2.CandidateGenerator.Dup.Departures do
&1,
fetch_departures_fn,
fetch_alerts_fn,
fetch_routes_serving_stop_fn,
fetch_routes_serving_stops_fn,
now,
ctx
),
Expand All @@ -249,15 +249,15 @@ defmodule Screens.V2.CandidateGenerator.Dup.Departures do
section,
fetch_departures_fn,
fetch_alerts_fn,
fetch_routes_serving_stop_fn,
fetch_routes_serving_stops_fn,
now,
ctx
) do
Screens.Telemetry.span(
[:screens, :v2, :candidate_generator, :dup, :departures, :get_section_data],
ctx,
fn ->
routes = get_routes_serving_section(params, fetch_routes_serving_stop_fn)
routes = get_routes_serving_section(params, fetch_routes_serving_stops_fn)
# DUP sections will always show no more than one mode.
# For subway, each route will have its own section.
# If the stop is served by two different subway/light rail routes, route_ids must be populated for each section
Expand Down Expand Up @@ -634,16 +634,13 @@ defmodule Screens.V2.CandidateGenerator.Dup.Departures do

defp get_routes_serving_section(
%{route_ids: route_ids, stop_ids: stop_ids},
fetch_routes_serving_stop_fn
fetch_routes_serving_stops_fn
) do
routes =
stop_ids
|> Enum.map(&fetch_routes_serving_stop_fn.(&1))
|> Enum.flat_map(fn
case fetch_routes_serving_stops_fn.(stop_ids) do
{:ok, routes} -> routes
:error -> []
end)
|> Enum.uniq()
end

if route_ids == [] do
routes
Expand Down
10 changes: 5 additions & 5 deletions lib/screens/v2/candidate_generator/pre_fare.ex
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,10 @@ defmodule Screens.V2.CandidateGenerator.PreFare do
def audio_only_instances(
widgets,
config,
routes_serving_stop_fn \\ &Route.serving_stop/1
routes_serving_stops_fn \\ &Route.serving_stops/1
) do
[
fn -> content_summary_instances(widgets, config, routes_serving_stop_fn) end,
fn -> content_summary_instances(widgets, config, routes_serving_stops_fn) end,
fn -> alerts_intro_instances(widgets, config) end,
fn -> alerts_outro_instances(widgets, config) end
]
Expand Down Expand Up @@ -133,9 +133,9 @@ defmodule Screens.V2.CandidateGenerator.PreFare do
[%ShuttleBusInfoWidget{screen: config, now: now}]
end

defp content_summary_instances(widgets, config, routes_serving_stop_fn) do
config.app_params.content_summary.parent_station_id
|> routes_serving_stop_fn.()
defp content_summary_instances(widgets, config, routes_serving_stops_fn) do
[config.app_params.content_summary.parent_station_id]
|> routes_serving_stops_fn.()
|> case do
{:ok, routes_at_station} ->
subway_lines_at_station =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ defmodule Screens.V2.CandidateGenerator.Widgets.ElevatorClosures do
end

defp routes_serving_stop(stop_id) do
case Route.serving_stop(stop_id) do
case Route.serving_stops([stop_id]) do
{:ok, routes} -> routes
:error -> []
end
Expand Down
Loading

0 comments on commit 28f32d6

Please sign in to comment.