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 all 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
22 changes: 18 additions & 4 deletions lib/screens/ol_crowding/dynamic_supervisor.ex
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
defmodule Screens.OlCrowding.DynamicSupervisor do
@moduledoc false

require Logger

use DynamicSupervisor
alias Screens.OlCrowding.Logger
alias Screens.OlCrowding.LogCrowdingInfo

def start_link(init_arg) do
DynamicSupervisor.start_link(__MODULE__, init_arg, name: __MODULE__)
Expand All @@ -27,9 +29,9 @@ defmodule Screens.OlCrowding.DynamicSupervisor do
fetch_params
) do
spec = %{
id: Logger,
id: LogCrowdingInfo,
start:
{Logger, :start_link,
{LogCrowdingInfo, :start_link,
[
%{
original_crowding_levels: original_crowding_levels,
Expand All @@ -48,6 +50,18 @@ defmodule Screens.OlCrowding.DynamicSupervisor do
restart: :transient
}

DynamicSupervisor.start_child(__MODULE__, spec)
case DynamicSupervisor.start_child(__MODULE__, spec) do
{:ok, child_pid} ->
_ = :timer.exit_after(10_000, child_pid, :kill)

{:ok, child_pid, _} ->
_ = :timer.exit_after(10_000, child_pid, :kill)

{:error, error} ->
Logger.error("crowding_dyn_supervisor_process_error #{inspect(error)}")

_ ->
Logger.warn("Something went wrong with starting the crowding dynamic supervisor process")
end
end
end
2 changes: 1 addition & 1 deletion lib/screens/ol_crowding/logger.ex
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
defmodule Screens.OlCrowding.Logger do
defmodule Screens.OlCrowding.LogCrowdingInfo do
@moduledoc false

require Logger
Expand Down
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, :kill)
fun.()
end
end
end
Loading