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

better to fail startup than auto-kill other running instances #324

Closed
jonassmedegaard opened this issue Mar 14, 2022 · 5 comments · Fixed by #378
Closed

better to fail startup than auto-kill other running instances #324

jonassmedegaard opened this issue Mar 14, 2022 · 5 comments · Fixed by #378

Comments

@jonassmedegaard
Copy link

I stumbled upon #167 and that sems bad design to me: Normal behavior of daemons is to simply refuse to start if a pidfile exist and the referenced PID is alive.

It seems you wanted the forceful startup specifically for certain deployment scripts. I urge you to consider removing this functionality from atomic-server and instead handle such unusual forceful treatment of pre-existing processes e.g. by having your deployment script do a killall atomic-server && sleep 2 || true before starting atomic-server.

Reason I stumbled upon #167 was that I wondered why heim was used. If you insist that forceful startup should be implemented in atomic-server itself, then at least consider moving to a more lightweight library for that - e.g. sysinfo. I have zero experience programming in Rust, so only mention that alternative because others seem to recommend it, both at stackoverflow and users.rust-lang.org.

@jonassmedegaard
Copy link
Author

Here is an excellent explanation on why it is better to leave daemon handling to external tools: https://stackoverflow.com/a/61681452

I.e. the recommendation there goes even further: Don't create a PID file at all - leave that to external tools as well!

@joepio
Copy link
Member

joepio commented Mar 14, 2022

Thanks for pointing this out, @jonassmedegaard

It's definitely true that external daemon management is superior to the ad-hoc solution I came up with. It's also not even used for the deployment script any more (I now use systemd for that), it's now only there for convenience if you don't want to do the hassle of setting up systemd or something similar. If you open a terminal and run atomic-server, and later forget which terminal you've used to open it, the process becomes kind of hard to find and kill. Auto-close would help in this situation. I find it quite useful during development, too.

I.e. the recommendation there goes even further: Don't create a PID file at all - leave that to external tools as well!

So what should the behavior be? I think this would result in confusing error messages, like Could not acquire lock on DB file, another process is using it. Then the user needs to find this other process, which often involves doing port lookups and other difficult stuff. This is what I started out with, and it was a pretty bad UX.

If I keep the PID file, I could have: PID file found, atomic-server is running elsewhere. Please close the existing process.

Personally, I'd still rather have it would close the process, instead of having to then do this manually, but I'm pretty sure there are reasons why auto-closing is bad.

So two questions:

  • What are the downsides of auto-closing?
  • What (cross-platform compatible) alternative would give a similar (or better) UX?

@jonassmedegaard
Copy link
Author

If you [try to handle daemonizing yourself, and do it wrong], the process becomes kind of hard to find and kill.

Yes, handling background processes correctly is hard. Very hard. Might even be insecure - depending on how you fail to do it exactly correct.

So don't do it yourself.

Just please pretty please don't do it yourself.

@joepio
Copy link
Member

joepio commented Mar 14, 2022

Ok, no real arguments still, but I know that I can trust your opinion on this. I'll remove the feature.

@jonassmedegaard
Copy link
Author

jonassmedegaard commented Mar 14, 2022

What are the downsides of auto-closing?

Same downsides as killing someone sitting on your favorite seat in the bus: a) Might be a friend. b) Might be a stranger that is not an enemy.

What (cross-platform compatible) alternative would give a similar (or better) UX?

Daemon handling is inherently platform-specific.

The non-cross-platform alternative providing better UX is to use the platform-specific tools as recommended (same link as I referenced previously).

If you feel like reinventing the wheel of systemd then here is a starting point: http://software.clapper.org/daemonize/ (but that doesn't solve cross-platform nor UX - systemd also provides APIs for improved UX).

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

Successfully merging a pull request may close this issue.

2 participants