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

Make SIGINT be more of a "graceful shutdown" than it is now #554

Closed
bruceg opened this issue Jun 23, 2021 · 9 comments · May be fixed by #833
Closed

Make SIGINT be more of a "graceful shutdown" than it is now #554

bruceg opened this issue Jun 23, 2021 · 9 comments · May be fixed by #833
Labels
discuss way forward is unclear; needs discussion of approach to take and why effort-high issue is likely to require >20h of effort, perhaps much more enhancement issue is a request for a feature, and not a defect impact-low low importance

Comments

@bruceg
Copy link

bruceg commented Jun 23, 2021

When running unison -repeat watch -watch, unison will continuously look for new changes (via the watcher) and synchronize them. This all works fine, but I would like a way to interrupt this process between synchronization batches. Currently, the only way I am aware of (using the text UI) is to wait for unison to report "Synchronization complete" and then quickly hit Control-C before it can be notified of changes. I know unison can be interrupted during synchronization, but I would prefer to avoid leaving temporary files around or having to resolve issues with partial synchronizations or leftover lock files.

Is there another way of stopping unison -repeat other than killing it? If not, could some mechanism be added?

@gdt gdt added discuss way forward is unclear; needs discussion of approach to take and why effort-high issue is likely to require >20h of effort, perhaps much more enhancement issue is a request for a feature, and not a defect impact-low low importance labels Jun 23, 2021
@gdt
Copy link
Collaborator

gdt commented Jun 23, 2021

I would ask: when you run that, and decide to stop unison because you need to reboot a computer or something, what do you do? I wonder if this is really "there is no graceful shutdown in repeat mode" and not particularly about wanting a pause between batches.

@bruceg
Copy link
Author

bruceg commented Jun 23, 2021

It is the former (no graceful shutdown). My current process is to watch the text output and quickly hit Control-C when I see the Synchronization complete … or Nothing to do line, as there's usually a second or two pause before it starts the loop again.

@bruceg
Copy link
Author

bruceg commented Jun 23, 2021

I had two thoughts for implementation, though I am not familiar with the internals so I cannot comment on the feasibility of either. First would be to use some non-interrupt signal (say SIGHUP) to set an internal flag that is checked before and after synchronizations to trigger a graceful shutdown. The other would be to check for a configurable shutdown file to do similar. Doing a graceful shutdown during a synchronization strikes me as a much more difficult problem to solve.

@gdt gdt changed the title Interrupting unison -repeat Add graceful shutdown to unison -repeat Jun 23, 2021
@gdt
Copy link
Collaborator

gdt commented Jun 23, 2021

I guess the other question is about graceful shutdown of a sync that doesn't have -repeat. Really, SIGINT should lead to things being cleaned up and left in a state where if reran nothing bad will happen.

I also wonder if the command-line could read from stdin, and if it sees EOF enter graceful shutdown at the end of the current sync. Or take q\n etc.

@bruceg
Copy link
Author

bruceg commented Jun 23, 2021

Right, reading something (like EOF) from stdin would work as well and is a good idea, at least for an interactive stop. It's a little harder to set that up to trigger automatically from another program, but it could still be done with a named pipe on stdin. My other suggestions are harder for interactive mode, so this latter one is likely better for the common case.

I agree SIGINT should be safe as well, but I have run into two situations where interrupting unison causes problems.

  1. The common one is where it is interrupted while synchronizing a file that is modified before the next run. It then shows up as an update-update conflict requiring manual intervention. While it is technically safe, it is annoying at best, particularly in combination with batch mode.
  2. I have had a number of instances where interrupting unison led to a corrupt database file. Now, I think most, if not all, of these have happened to me when one end or the other also experienced power failures, so that may well be an entirely different problem, but it has left me nervous about interrupting.

There is also the nuisance of having temporary files scattered around after exit, but that can happen even during normal synchronization (ie files modified during synchronization), so that is definitely unrelated and minor in any case.

@gdt
Copy link
Collaborator

gdt commented Jun 23, 2021

If you can create a repro recipe for an interrupted update with problems, please open a new issue. Unison arguably should be doing write/fsync/rename sorts of things, although that raises lots of other issues.

@tleedjarv
Copy link
Contributor

Some thoughts:

  • Interrupting unison at any time is supposed to be safe, even if it comes with some cruft left behind and other potential inconveniences that @bruceg mentioned. But there is currently a situation with watcher mode that makes interrupting not safe even between the sync runs (that is, after "Synchronization complete"). See fsmonitor immediate events considered harmful #525
  • I think being able to interactively do Control-C and get immediate effect is a good thing and should remain possible. But it can be tweaked and enhanced a bit. For example, one SIGINT could be a "graceful shutdown" signal. Several SIGINTs in sequence could trigger "immediate shutdown".

Really, SIGINT should lead to things being cleaned up and left in a state where if reran nothing bad will happen.

Agree. This mostly is the case as well, at least the "if reran nothing bad will happen" part. Things being cleaned up is relative, normally the tmp files are cleaned up by the next run. With an immediate (forced) shutdown there will be some inconveniences that were mentioned above but nothing directly dangerous. Solving this is probably a big task. Doing the entire sync atomically is impossible. The only alternative that comes to mind right now is some kind of synced archive checkpoints or WAL-style recording.

Being able to distinguish between a "graceful" SIGINT and "stop NOW!" SIGINT is a good compromise of functionality and imlpementation simplicity.

@gdt gdt changed the title Add graceful shutdown to unison -repeat Make SIGINT be more of a "graceful shutdown" than it is now Jun 27, 2021
@gdt
Copy link
Collaborator

gdt commented Jun 27, 2021

I have retitled this issue and hereby set scope:

  • This issue is about SIGINT doing more/better cleanup/shutdown while still being quite fast (e.g., 3s while not waiting for network input would be ok)
  • This issue is not about anything that happens currently where SIGINT leads to anything more than the annoyance of temp files that are not cleaned up until a future run. If anyone finds that, please open a separate issue.

@gdt
Copy link
Collaborator

gdt commented Mar 19, 2023

I think this issue has been addressed. If there is anything wrong still (SIGINT leads to conflicts, etc.), feel free to open a new issue with a reproduction recipe with 2.53.1. (Note also SIGUSR2 feature, but that's different.)

@gdt gdt closed this as completed Mar 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss way forward is unclear; needs discussion of approach to take and why effort-high issue is likely to require >20h of effort, perhaps much more enhancement issue is a request for a feature, and not a defect impact-low low importance
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants