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

Automatically starting NervesHubLink.Supervisor race condition #11

Closed
coop opened this issue Feb 25, 2020 · 3 comments
Closed

Automatically starting NervesHubLink.Supervisor race condition #11

coop opened this issue Feb 25, 2020 · 3 comments

Comments

@coop
Copy link

coop commented Feb 25, 2020

In the switch from NervesHub to NervesHubLink the NervesHubLink.Supervisor is automatically started on boot instead of "whenever I want to start it". This change has introduced a race condition in my code because my NervesHubLink.Client relies on a process that may not have been started yet. Obviously I can catch the exception and reschedule the update but I'm wondering why the change?

This change also takes away the ability to conditionally include the supervisor which is something I do in development all the time when I'm testing without pushing releases.

@jjcarstens
Copy link
Collaborator

A few things:

  1. I'm definitely unaware of all the use-cases and totally missed this one that you might be relying on
  2. When I think of hardware uptime and reliability, ability to update firmware at any time to remove bad firmware is really high priority which means trying to do everything possible to keep that connection alive and independent of application logic. i.e. If you manage the NervesHub supervisor in your app, but its totally borked and can't start, now it cannot connect to NervesHub to get not-broken firmware. One of the simplest elixir mechanisms is to put that in its own app with separate supervision and start it independent of the app process, like via shoehorn. Or even just having the application outside of your application might be enough.
  3. A race condition is bad and we shouldn't allow to break this process. Either handling in your app, or adding a bit to nerves_hub_link to recover from errors and keep trying to get updates
  4. Ability to reduce noise in tests is a great thing. We should fix that and reduce trying to connect to something externally during tests

@jjcarstens jjcarstens added the Hacktoberfest Would be especially useful to get done during Hacktoberfest! label Sep 30, 2020
@jjcarstens
Copy link
Collaborator

I think one way we could address this issue is by allowing the library to start its own supervision, or be included manually in external supervision. Much like how nerves_ssh handles this deduction

@mobileoverlord mobileoverlord removed the Hacktoberfest Would be especially useful to get done during Hacktoberfest! label Dec 28, 2020
@joshk
Copy link
Contributor

joshk commented May 15, 2024

With the connect: false config option you can control when and where the children are added and started.

It's not perfect by any means, and I'd be happy to look into this if this feature is still wanted, but as its been 3.5 years since any activity on this issue, I think its best to close it for the time being.

@joshk joshk closed this as completed May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants