Persistent retries #8
Conversation
this maintains backwards compatability via the `child_spec`
This enables fwupdates to resume even after a reboot
ConnorRigby
force-pushed
the
persistent-retries
branch
from
abfbdf1 to
c3d2b35
Compare
| defstruct fwup_public_keys: [], | ||
| fwup_devpath: "/dev/mmcblk0", | ||
| handle_fwup_message: nil, | ||
| update_available: nil | ||
| update_available: nil, | ||
| journal_location: "/tmp/" |
There was a problem hiding this comment.
| journal_location: "/tmp/" | |
| journal_location: "/data/nerves_hub_link.partial" |
I'm just making up the journal location so other names work for me too. What I'd like to suggest are:
- Putting it on a persistent filesystem so that it can survive a reboot
- Only having one file to make it easy to figure out where to find it and easy to clean up
I haven't gotten to it yet, but I'm guessing that the use of one file might have other ramifications. I do like the simplicity and limiting the number of files that it can create.
There was a problem hiding this comment.
yeah i only defaulted it to /tmp for the test suite since /data can't be written to on our host machines. I agree on keeping a single file, but i also wanted this to be a directory because there needs to be some way of identifying a partial download as a particular firmware after a reboot, which i was going to use the Filename for.
I also considered putting a header in the partial file, but am still thinking about that.
| def save_chunk(%__MODULE__{fd: fd} = journal, data) when is_binary(data) do | ||
| hash = :crypto.hash(:sha256, data) | ||
| length = byte_size(data) | ||
| journal_entry = IO.iodata_to_binary([<<length::32>>, hash, data]) |
There was a problem hiding this comment.
:file.write/2 takes iodata, so you can pass the list:
| journal_entry = IO.iodata_to_binary([<<length::32>>, hash, data]) | |
| journal_entry = [<<length::32>>, hash, data] |
There was a problem hiding this comment.
whoops, you're right, good catch
| @type t :: %__MODULE__{ | ||
| fd: :file.fd(), | ||
| content_length: non_neg_integer(), | ||
| chunks: [binary()] |
There was a problem hiding this comment.
I'm a little worried about firmware images that exceed available memory. I haven't read the code completely, but keeping a chunk list makes me think they're being cached.
How about a setup where the firmware download process starts by pulling chunks from the journal and feeding them to fwup. When it runs out of journal chunks, it makes an http request for the rest?
There was a problem hiding this comment.
Yeah i think something along those lines will be required as well. I haven't thought of exactly how yet, but i like your idea.
|
I had a couple other thoughts on this:
Btw, there's a current issue with |
| @@ -183,6 +183,11 @@ defmodule NervesHubLinkCommon.UpdateManager do | |||
|
|
|||
| @spec maybe_update_firmware(UpdateAvailable.t(), State.t()) :: | |||
| State.download_started() | State.download_rescheduled() | State.t() | |||
| defp maybe_update_firmware(%UpdateAvailable{update_available: false}, %State{} = state) do | |||
There was a problem hiding this comment.
Should it also bail early if the :update_available key doesn't exist? Isn't that a possible case prior to your NH fix? Like would this affect people running NH servers and who aren't syncing with upstream regularly?
No description provided.