Skip to content

Commit

Permalink
Apply backpressure to downloader on slow updates (#202)
Browse files Browse the repository at this point in the history
It's possible for the firmware download to be faster than fwup can write
to eMMC or MicroSD. This causes the message queue to the update manager
to balloon as data starts queuing up in route to fwup. On a device
without much extra memory, this can be a cause of the following error:

```
04:14:26.957 [warn] __vm_enough_memory: pid: 87, comm: erts_sched_1, not enough memory for the allocation
```

This commit makes the asynchronous downloader updates synchronous so
fwup can push back when the writes are time consuming. The backpressure
results in the TCP receive buffers filling and eventually the download
slowing.
  • Loading branch information
fhunleth authored Jun 24, 2024
1 parent c90486a commit c6f10af
Showing 1 changed file with 31 additions and 18 deletions.
49 changes: 31 additions & 18 deletions lib/nerves_hub_link/update_manager.ex
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,19 @@ defmodule NervesHubLink.UpdateManager do
GenServer.call(manager, :currently_downloading_uuid)
end

# Private API for reporting download progress. This wraps a GenServer.call so
# that it can apply backpressure to the downloader if applying the update is
# slow.
defp report_download(manager, message) do
# 60 seconds is arbitrary, but currently matches the `fwup` library's
# default timeout. Having fwup take longer than 5 seconds to perform a
# write operation seems remote except for perhaps an exceptionally well
# compressed delta update. The consequences of crashing here because fwup
# doesn't have enough time are severe, though, since they prevent an
# update.
GenServer.call(manager, {:download, message}, 60_000)
end

@doc false
@spec child_spec(FwupConfig.t()) :: Supervisor.child_spec()
def child_spec(%FwupConfig{} = args) do
Expand Down Expand Up @@ -113,6 +126,23 @@ defmodule NervesHubLink.UpdateManager do
{:reply, state.status, state}
end

# messages from Downloader
def handle_call({:download, :complete}, _from, state) do
Logger.info("[NervesHubLink] Firmware Download complete")
{:reply, :ok, %State{state | download: nil}}
end

def handle_call({:download, {:error, reason}}, _from, state) do
Logger.error("[NervesHubLink] Nonfatal HTTP download error: #{inspect(reason)}")
{:reply, :ok, state}
end

# Data from the downloader is sent to fwup
def handle_call({:download, {:data, data}}, _from, state) do
_ = Fwup.Stream.send_chunk(state.fwup, data)
{:reply, :ok, state}
end

@impl GenServer
def handle_info({:update_reschedule, response, fwup_public_keys}, state) do
{:noreply,
Expand Down Expand Up @@ -144,23 +174,6 @@ defmodule NervesHubLink.UpdateManager do
end
end

# messages from Downloader
def handle_info({:download, :complete}, state) do
Logger.info("[NervesHubLink] Firmware Download complete")
{:noreply, %State{state | download: nil}}
end

def handle_info({:download, {:error, reason}}, state) do
Logger.error("[NervesHubLink] Nonfatal HTTP download error: #{inspect(reason)}")
{:noreply, state}
end

# Data from the downloader is sent to fwup
def handle_info({:download, {:data, data}}, state) do
_ = Fwup.Stream.send_chunk(state.fwup, data)
{:noreply, state}
end

@spec maybe_update_firmware(UpdateInfo.t(), [binary()], State.t()) :: State.t()
defp maybe_update_firmware(
%UpdateInfo{} = _update_info,
Expand Down Expand Up @@ -215,7 +228,7 @@ defmodule NervesHubLink.UpdateManager do
@spec start_fwup_stream(UpdateInfo.t(), [binary()], State.t()) :: State.t()
defp start_fwup_stream(%UpdateInfo{} = update_info, [] = fwup_public_keys, state) do
pid = self()
fun = &send(pid, {:download, &1})
fun = &report_download(pid, &1)
{:ok, download} = Downloader.start_download(update_info.firmware_url, fun)

{:ok, fwup} =
Expand Down

0 comments on commit c6f10af

Please sign in to comment.