* Remove the pid file if runtime errors occur
* Clean up error handling and fix pid file creation
The pid file was being created before evaluating the args, now it may
happen that incorrect args or --help was passed: in that event, the pid
file remains created. This was also fixed, besides some refactoring.
* Deter other processes from using the same data dir
For more information, see #167
* Don't lock `pid_file`
Windows has mandatory locking so second instance won't be able to read
the PID of the other process. We'll just keep the file descriptor/handle
open
This is very useful because it removes the need for user intervention in
the event save on termination fails. Say the save operation fails due to
'some bad daemon' changing the directory's perms. Now skyd reports this
error while trying to save upon termination. Our sysadmin now fixes the
perms issue. The previous design would force the sysadmin to _somehow_
foreground skyd and hit enter. That is silly. The new design just
attempts to do a save operation every 10 seconds. So in case the issue
is fixed, the save operation will recover on its own.
Why not exponential backoff?
That's because the issue can be fixed some long time later and we may
have reached a large backoff value so the save that could have succeeded
would have to wait for a long duration before it can do anything
meaningful.
This also fixes a bug that caused BGSAVE errors to be reported as info
class log entries.
* Explicitly fsync and relax CPU on snap busy-loop
This commit also switches to using global `VERSION` and `URL` statics
than defining it per-crate.
* Add changelog entry and bump up version
* Optimize `dbtest` macro and rm redundant allocs
* Upgrade deps
This is a silly optimization but can be significantly faster for larger
action names. Also, since there are (should be) no UTF-8 characters in
the first argument, this is absolutely fine and sensible.
* Stop accepting writes if snapshotting fails
This is an important consideration: if BGSAVE fails and poisons the
database, snapshotting can and should too. But this is debatable in some
parts. For example, users may configure snapshots to be on a network
file system (symlinked maybe) and this can fail.
Now in some cases, this failure 'may be acceptable'. This commit adds a
way to customize this behavior through the `failsafe` key in the
snapshots section of the cfg file and through the --stop-write-on-fail
option passed to `skyd` on startup. However, BGSAVE remains unchanged:
it will always poison the database if it fails. If the user doesn't want
this, they can simply disable BGSAVE.
* Add changelog
* Create a new file on writing to flock-ed file
This fix is a very important one in two ways. Say we have an user A.
They go ahead and launch skyd. skyd creates a data.bin file. Now A just
deletes the data.bin file for fun. Funny enough, this never causes flock
to error!
Why? Well because the descriptor/handle is still valid and was just
unlinked from the current directory. But this might seem silly since
the user exits with a 'successfully saved notice' only to find that the
file never existed and all of their data was lost. That's bad.
There's a hidden problem in our current approach too, apart from this.
Our writing process begins by truncating the old file and then writing
to it by placing the cursor at 0. Nice, but what if this operation just
crashes. So we lost the current data AND the old data. Not good.
This commit does a better thing: it creates a new temporary file, locks
it before writing and then flushes the current data to the temporary
file. Once that succeeds, it replaces the old data.bin file with the
newly created file.
This solves both the problems mentioned here for us:
1. No more of the silly error
2. If BGSAVE crashes in between, we can be sure that at least the last
data.bin file is in proper shape and not half truncated or so.
This commit further moves the background services into their
own module(s) for easy management.
* Fix CI scripts
Fixes:
1. Our custom runner (drone/.ci.yml) was modified to kill the skyd
process once done since this pipeline is not ephemeral.
2. GHA for some reason ignores any error in the test step and proceeds
to kill the skyd process without erroring. Since GHA runners are
ephemeral, we don't need to do this manually.
What we did in the old implementation was pure over-engineering.
We relied on CoreDB's `Drop` impl to terminate the background services.
Now this is absolutely unreliable due to the nature of async functions.
We also relied on the bgsave scheduler to release the lock upon exit
which is also unreliable because we left the service to the mercy of the
runtime. We spawned the task and didn't hold as much as a `JoinHandle`
to it. That's bad because the runtime can just abort these tasks which
may result in the lock never being released. Even though it is designed
to release the lock on Drop, the destructor may however not be called at
all.
This commit fixes all those issues by simplifying the entire impl to
use Terminator. Now the background save and snapshot services run
independently, in their own tasks. Whenever the user passes a SIGINT,
we tell everyone to quit. The listeners understand that this is the
last query they'll process and the background save tasks exit almost
immediately. But what if some data was modified by this last query...?
No worries, that is completely handled by main(). The lock that BGSAVE
leaves is immediately (almost) returned to main and main will attempt
to flush the data almost immediately. That's how we maintain reliability
This commit ensures that BGSAVE is optimistic in doing what it is doing:
If BGSAVE fails once, it will immediately poison the table. Now let's
say that some amazing sysadmin managed to SSH into the server and was
able to fix the storage issue; BGSAVE would be able to succeed.
The current implementation was flawed: firstly it prevented that and
secondly even if it succeeded in running BGSAVE, the server would refuse
to accept writes. This commit fixes this behavior.
Sayan Nandan