This puts a lot of state into a new `LockedStream` that was formerly
guarded more coarsely by the recording lock or more granularly by
`RecordingToInsert` locks. And it restructures `LockedDatabase::flush`
to avoid holding this lock while performing SQLite operations.
Streamers write only into a recent frames buffer in RAM, which the
directory pool workers asynchronously write to disk approximately once
per GOP, where formerly there was a write call per frame.
Before, streamers could fall behind due to slow sample file directory
disk I/O calls or if writing got stuck in a retry loop, for example if
out of disk space. If Moonfire falls too far behind in receiving,
cameras tend to disconnect or (sadly) mis-frame messages because they do
not handle this case well. This removes a confusing potential cause of
such errors.
In the new model, streamers never wait for disk I/O. Frames are buffered
up to a limit. Recordings are aborted if the I/O pool falls too far
behind. There's no retry loop.
This roughly halves CPU usage as thread hand-offs and syscalls are less
common. It also enables several desirable future changes:
* audio recording (which radically increases the number of frames and
thus would otherwise significantly increase CPU usage).
* "monitor mode" (allowing live view without recording to disk),
* further CPU reductions for live view (by skipping all SQLite ops and
disk I/O).
In the new model, streamers also rarely wait for SQLite database
operations either—now only on video parameter changes. That might go
away in an upcoming change too.
The "syncer" (which synced files/dirs and kept track of what has to be
flushed into the SQLite database when) has mostly gone away too. The
directory pool workers handle the syncing now, and then they notify
a new "flusher" for the latter portion.
I'd like to go further still and have the database as a whole be owned
by a dedicated thread, the only one that uses the SQLite connection.
This prevents the tokio threads from blocking on SQLite operations.
I tried out doing so before making the streamers less dependent on the
db, but the extra thread handoffs were awful for performance.
Now, all IO happens on each directory's two-thread pool with threads
named `dir-<PATH>`. Formerly, IO could happen on the per-stream writer
threads, per-directory reader threads, and occasional on even the tokio
worker threads.
The recording path uses more CPU than before due to extra thread
hand-offs. That will change in the next commit, which will radically
change the approach to writing.
Currently all errors from retina are `Unknown`, and these massive
backtraces are obnoxious when a camera fails on every attempt, with
only a second between.
This centralizes a major source of `.unwrap()` throughout the code,
and one that would otherwise grow with upcoming changes. The new
error message should be more clear.
HTTP requests were only returning the error message to the caller, not
logging locally. In most cases the problem could be understood
client-side, but there are some exceptions. E.g. if Moonfire returns
a 403 on WebSocket update, even in the Chrome debug tools's network
tab the HTTP response body seems to be unavailable. And in general,
it's nice to have more context server-side.
Logging a `response::Body` isn't practical (it could be a stream), so
convert all the web stuff to use `base::Error` err returns.
Convert the `METHOD_NOT_ALLOWED` paths to return `Ok` for now. This is a
bit lame but punts on having some way of plumbing an explicit/overridden
status code in `base::Error`, as no gRPC error kind cleanly maps to
that.
Also convert `db::auth`, rather than making up an error kind in the web
layer.
This is also a small step toward getting rid of `failure::Error`.
Sessions' last use updates weren't getting persisted to the database
because the update statement wasn't passing through the hash.
Also address a TODO of tracing in tests by using the same tracing
setup as in production.
I think this is a big improvement in readability.
I removed the `lnav` config, which is a little sad, but I don't think it
supports this structured logging format well. Still seems worthwhile on
balance.
This gives much better information to the UI layer, getting rid of a
whole troubleshooting guide entry. See #119#132#218#219
I also restructured the code in anticipation of a new WebSocket event
stream (#40).
This stops using parking_lot entirely. Since Rust 1.62, the std
implementations on Linux are direct futexes, not the boxed pthread
mutexes they used to be. No real reason to use parking_lot anymore, so
shed this dependency.
error[E0106]: missing lifetime specifier
--> base/time.rs:26:68
|
26 | fn fixed_len_num<'a>(len: usize) -> impl FnMut(&'a str) -> IResult<&'a str, i32> {
| ^ expected named lifetime parameter
|
= help: this function's return type contains a borrowed value with an elided lifetime, but the lifetime cannot be derived from the arguments
help: consider using the `'a` lifetime
I did a full `cargo upgrade` and fixed what it broke:
* a couple things for the latest protobuf 3.0 alphas
(note alphas don't promise API stability)
* new minimum supported Rust version
This should have some other nice effects: parking_lot now uses inline
assembler, tokio has gotten faster, etc.
After a frustrating search for a suitable channel to use for shutdown
(tokio::sync::watch::Receiver and
futures::future::Shared<tokio::sync::oneshot::Receiver> didn't look
quite right) in which I rethought my life decisions, I finally just made
my own (server/base/shutdown.rs). We can easily poll it or wait for it
in async or sync contexts. Most importantly, it's convenient; not that
it really matters here, but it's also efficient.
We now do a slightly better job of propagating a "graceful" shutdown
signal, and this channel will give us tools to improve it over time.
* Shut down even when writer or syncer operations are stuck. Fixes#117
* Not done yet: streamers should instantly shut down without waiting for
a connection attempt or frame or something. I'll probably
implement that when removing --rtsp-library=ffmpeg. The code should be
cleaner then.
* Not done yet: fix a couple places that sleep for up to a second when
they could shut down immediately. I just need to do the plumbing for
mock clocks to work.
I also implemented an immediate shutdown mode, activated by a second
signal. I think this will mitigate the streamer wait situation.
* API change: in update signals, allow setting a start time relative
to now. This is an accuracy improvement in the case where the client
has been retrying an initial request for a while. Kind of an obscure
corner case but easy enough to address. And use a more convenient
enum representation.
* in update signals, choose `now` before acquiring the database lock.
If lock acquisition takes a long time, this more accurately reflects
the time the caller intended.
* in general, make Time and Duration (de)serializable and use them
in json types. This makes the types more self-describing, with
better debug printing on both the server side and on the client
library (in moonfire-playground). To make this work, base has to
import serde which initially seemed like poor layering to me, but
serde seems to be imported in some pretty foundational Rust crates
for this reason. I'll go with it.
I also enforced some invariants in the signals code, fixing a couple
bugs. The signals code is more complex than I'd like, but hopefully
is working now.
I think this has a minor behavior change: permission denied replies
change to HTTP 403 where they were HTTP 401. The new behavior seems
more correct, as these errors can occur when authentication has
succeeded but the session in question is not authorized for the given
operation. The UI currently doesn't care about this distinction.
Scott Lamb
Jared Wolff
Greg V