Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] Explicitly expire child processes of a Task.Supervisor #1873

Merged
merged 7 commits into from
Sep 27, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions lib/screens/screens_by_alert/memcache.ex
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ defmodule Screens.ScreensByAlert.Memcache do
```
"""
alias Screens.ScreensByAlert.Memcache.TaskSupervisor
alias Screens.Util

require Logger

Expand Down Expand Up @@ -56,10 +57,16 @@ defmodule Screens.ScreensByAlert.Memcache do

# To avoid bottlenecks and unnecessarily blocking the caller, run in a separate task process
_ =
Task.Supervisor.start_child(TaskSupervisor, fn ->
update_screens_last_updated_key(screen_id, now)
Enum.each(alert_ids, &update_alert_key(&1, screen_id, now))
end)
Task.Supervisor.start_child(
TaskSupervisor,
Util.fn_with_timeout(
fn ->
update_screens_last_updated_key(screen_id, now)
Enum.each(alert_ids, &update_alert_key(&1, screen_id, now))
end,
10_000
)
)

:ok
end
Expand Down
13 changes: 10 additions & 3 deletions lib/screens/screens_by_alert/self_refresh_runner.ex
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ defmodule Screens.ScreensByAlert.SelfRefreshRunner do

alias Screens.Config.Screen
alias Screens.ScreensByAlert
alias Screens.Util

# (Not a real module--just a name assigned to the Task.Supervisor process that supervises each simulated data request run)
alias Screens.ScreensByAlert.SelfRefreshRunner.TaskSupervisor
Expand Down Expand Up @@ -70,9 +71,15 @@ defmodule Screens.ScreensByAlert.SelfRefreshRunner do
# process from going down if an exception is raised while running
# ScreenData.by_screen_id/1 for some screen.
Enum.each(screen_ids_to_refresh, fn screen_id ->
Task.Supervisor.start_child(TaskSupervisor, fn ->
@screen_data_fn.(screen_id, skip_serialize: true)
end)
Task.Supervisor.start_child(
TaskSupervisor,
Util.fn_with_timeout(
fn ->
@screen_data_fn.(screen_id, skip_serialize: true)
end,
10_000
)
)
end)

{:noreply, state}
Expand Down
12 changes: 12 additions & 0 deletions lib/screens/util.ex
Original file line number Diff line number Diff line change
Expand Up @@ -218,4 +218,16 @@ defmodule Screens.Util do
def translate_carriage_occupancy_status(:full), do: :crowded
def translate_carriage_occupancy_status(:not_accepting_passengers), do: :closed
def translate_carriage_occupancy_status(_), do: nil

@doc """
Adds a timeout to a function. Mainly used for child processes of a Task.Supervisor
which don't come with a timeout by default.
"""
@spec fn_with_timeout((() -> val), non_neg_integer()) :: (() -> val) when val: any()
def fn_with_timeout(fun, timeout) do
fn ->
_ = :timer.exit_after(timeout, :normal)
hannahpurcell marked this conversation as resolved.
Show resolved Hide resolved
fun.()
end
end
end
Loading