* 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.
The size part of the metaline is absolutely redundant as we're doing
double the work while reading the size and then the real thing.
Since sizes won't have escape codes, we can freely read upto the LF
If Parser::will_cursor_give_char is set to not error if a char matches
or the next line is empty, return Ok(bool). If this_if_nothing_ahead is
set to false, then return a NotEnough error if no more chars are
available.
The newly added test explains why
It is likely that we'll change the HashMap implementation in the future,
hence its best to hide away the HashMap to make sure we can easily
replace it.
The previous logic was heavily flawed; it only had to check if the path
was a dir and isn't the remote snapshot directory.
Similarly, the file name parsing should only kick in if the item is a
file
This commit adds changes so that the main process almost immediately
acquires a lock on the data file when runtime is dropped. This is just
an added precaution to try and ensure that no other process does
something silly with the data file.
The descriptor is cloned for this using `FileLock::try_clone`
8e46e62 added a block_on_process_exit function that kept on sending
`notify_one()`s in a loop until the services terminated. This was
pointless as the `Drop` impl would do it for us anyways.
(What was I thinking?)
So, in main(), we're spawning an async task that lets the DB run as long
as we don't pass a ctrl_c (or some bad panic occurs). Once the ctrl_c
is received, we start terminating all workers. `block_on` returns DB
which should be the only one holding an atomic reference to the shared
field. We assert this right after dropping `runtime`.
Finally, the ECONNRESET suppression match was fixed to remove an
unreachable branch by adding conditional compilation
This commit ensures that the workers exit before attempting a flush_db
operation. Only after block_on_process_exit finishes we return `db`.
Now we run a simple flush_db operation knowing that the lock has been
released.
To block on process termination, we introduce a new function
block_on_process_exit that does the same thing as CoreDB's Drop
implementation.
Windows is the most ingenious OS in the world where filenames can
conflict with shell commands. That's right, con is an I/O device on
Windows and cannot be used for a filename! This is why we were having
checkout errors on Windows!
Vive la POSIX!
We have introduced a trait `BufferedSocketStream` that is a 'dummy'
trait and is implemented for both `SslStream<TcpStream>` and
`TcpStream`. So, the generic `Connection` object accepts any type that
implements the `BufferedSocketStream` trait (and hence should also
implement `AsyncWrite`)
This commit does a LOT! It migrates the `queryengine::execute_simple`,
`CoreDB::execute_query` and the kvengine functions to use generic
connections.
The object dbnet::Con was removed because it isn't needed anymore.
The listeners were also upgraded to use the generic connection handler
The trait `Con` and `ConOps` were renamed to `ProtocolConnectionExt`
and `ProtocolConnection`.
This naming scheme clearly explains that the Ext version 'augments' the
non-Ext impl. This is the very case here: ProtocolConnection provides
the basic funtions needed for interfacing with net I/O while the Ext
trait enables high-level interaction with the protocol and ultimately
queries.
A generic `ConnectionHandler` object was added that will replace the
SSL and non-SSL handler objects, again reducing redundancy.
Dummy execute functions were added to CoreDB and queryengine.