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

If Sandstorm fails, don't kill the update process #1050

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

paulproteus
Copy link
Collaborator

This commit has an upside:

  • If Sandstorm crashes because e.g. mongod crashes, then we continue looking for updates.

This commit has a downside:

  • Sandstorm now has no way to signal to a process monitor (e.g. systemd) that it has failed. This means that errors like binding to a port that is already in use prevent Sandstorm don't clearly alert the admin to a problem, which is pretty sad.

So I'm not sure I actually am +1 on this change. @kentonv your feedback very much requested.

@kentonv
Copy link
Member

kentonv commented Oct 22, 2015

Did Sandstorm actually crash from things like "port already in use" before? Or did it go into an infinite loop of trying to start node and failing?

@paulproteus
Copy link
Collaborator Author

It would actually crash, which has the upside that systemd monitoring shows the process as FAILED. You can repro by setting PORT=22 on a Sandstorm on a systemd Sandstorm install.

@kentonv
Copy link
Member

kentonv commented Oct 23, 2015

For advanced sysadmins, having systemd be aware of failure may be useful, but for users like me who know nothing about systemd, it's most useful if Sandstorm tries its best to self-recover. Consider a user who launched a VM with a one-click launcher and doesn't know how to open a shell, much less use it.

That said, going into emergency update mode when the port is in-use does not seem right...

Perhaps we should hold off on this until someone has time to change it so that update checks happen more often in this mode with "type=emerg" or something, so that we can get alerts when this is happening.

@paulproteus
Copy link
Collaborator Author

paulproteus commented Oct 23, 2015 via email

@ocdtrekkie ocdtrekkie added the install-config Installation/configuration issues label Apr 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
install-config Installation/configuration issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants