Now that we run callbacks on the main loop, we can move callback support
for Counter and Gauge instances directly into Zeek and don't need to patch
prometheus-cpp anymore.
For terminated threads, the totals would go down once the threads are
removed, which isn't great. Move tracking of sent in and sent out
messages from callback to explicit `Inc()` calls.
Also fixes total_messages_in_metric being initialized twice rather
than total_messages_out_metric.
For uint64_t, std::numeric_limits<T>::has_infinity is false and infinity()
actually returns 0. Use uint64_t's max() instead. We could cast to double
and use the double infinity, but this seems reasonable, too.
This was found while trying to provoke some pending messages and being
confused why all but the "inf" bucket increased.
The buckets are specified as lower-equal (changed from lower-than now),
which means we shouldn't break: The larger "le" bucket contains all previous
buckets, too. The "inf" bucket represents the current number of threads.
For example, with a total of 10 threads, 5 threads with 0 messages pending,
another 4 threads with 50 messages, and on with 2000 messages, the metrics
would end end up as follows:
pending_buckets{le=1} = 5
pending_buckets{le=10} = 5
pending_buckets{le=100} = 9
pending_buckets{le=1000} = 9
pending_buckets{le=10000} = 10
pending_buckets{le=inf} = 10
This might be strange initially, but aligns with the Prometheus
histogram approach (though we're using gauges here).
The core.file-analyzer-violation test showed that it's possible to
create new threads (log writers) when Zeek is in the process of
terminating. This can result in the IO manager's deconstructor
deleting IO sources for threads that are still running.
This is sort of a scripting issue, so for now log a reporter warning
when it happens to have a bit of a bread-crumb what might be
going on. In the future it might make sense to plug APIs with
zeek_is_terminating().
This largely copies over Spicy's `.clang-format` configuration file. The
one place where we deviate is header include order since Zeek depends on
headers being included in a certain order.
This commit refactors the SendEvent call and moves it from the Input
ReaderBackend to to MsgThread. This allows all other types of threads
to access this functionality.
This necessitated a few more changes. Most importantly, one of the
ValueToVal methods was moved over to SerialTypes. Whereit arguably
belongs - there was nothing that was input-framework specific in
that method - and the functionality could come in useful in a number
of cases.
threading::Manager is currently never deleted, but if that ever changes,
deleting the stored HeartbeatTimer pointer would be a double-free
since TimerMgr owns it.
The Zeek code base has very inconsistent #includes. Many sources
included a few headers, and those headers included other headers, and
in the end, nearly everything is included everywhere, so missing
#includes were never noticed. Another side effect was a lot of header
bloat which slows down the build.
First step to fix it: in each source file, its own header should be
included first to verify that each header's includes are correct, and
none is missing.
After adding the missing #includes, I replaced lots of #includes
inside headers with class forward declarations. In most headers,
object pointers are never referenced, so declaring the function
prototypes with forward-declared classes is just fine.
This patch speeds up the build by 19%, because each compilation unit
gets smaller. Here are the "time" numbers for a fresh build (with a
warm page cache but without ccache):
Before this patch:
3144.94user 161.63system 3:02.87elapsed 1808%CPU (0avgtext+0avgdata 2168608maxresident)k
760inputs+12008400outputs (1511major+57747204minor)pagefaults 0swaps
After this patch:
2565.17user 141.83system 2:25.46elapsed 1860%CPU (0avgtext+0avgdata 1489076maxresident)k
72576inputs+9130920outputs (1667major+49400430minor)pagefaults 0swaps
- threading::Manager is no longer an IOSource.
- threading::MsgThread is now an IOSource. This allows threads themselves to signal when they have data to process instead of continually checking each of the threads on every loop pass.
- Make the thread heartbeat timer an actual timer and let it fire as necessary instead of checking to see if it should fire
For input threads that get joined during run-time instead of being
signalled to stop at termination-time as typical (e.g. an error occurs
or process exits w/ non-zero status) messages could remain in the
thread's queue and leak.
This patches threads to ensure they enter the proper "finished"
state so that the thread manager can attempt to fully process and
empty out their queues before joining them.
A bunch of infrastructure work to move IOSource, IOSourceRegistry (now
iosource::Manager) and PktSrc/PktDumper code into iosource/, and over
to a plugin structure.
Other IOSources aren't touched yet, they are still in src/*.
It compiles and does something with a small trace, but that's all I've
tested so far. There are quite certainly a number of problems left, as
well as various TODOs and cleanup; and nothing's cast in stone yet.
Will continue to work on this.
A thread that is done/killed should signify that the thread manager has
some processing to do -- it needs to process any messages in its out
queue, join the thread, and delete it. Otherwise the thread manager
may reach a state where it makes no progress in processing the last
remaining done/killed thread.
This fix also fixes the deadlock issue without putting any
new strain into the main packet processing path.
Instead of occasionally returning true in MaybeReady sometime,
we occasionally process threads if time_mgr time is not running.
If time_mgr time is running, we have heartbeat messages that will
trigger processing in any case -- processing always checks the
exact state of the Queues.
This fix probably also means that we can remove the communication
loads from all input framework tests and run them all simultaneously.
Now it should work. However - this commit changes a basic assumption
of the threading queue. This basic assumption is, that nothing can
be read out of the out-queue of a dead thread. I think that reading
out of the queue of a dead thread makes perfect sense (when the thread
shuts down, pushes the rest of its work on the queue and says bye,
and wants the main thread to pick it up afterwards) - however, I
guess one can be of a differing opinion here.
In any case, it makes stuff a bit easier to understand - in my opinion.
It took me a while to find out why the messages disappear in thin
air and never arrive in the main thread ;)
PrepareStop() is now SignalStop() and just signals a thread that it
should terminate. After that's called, WaitForStop() (formerly Stop())
wait for it to actually finish processing.
When stopping writers during operation, we now no longer wait for them
to finish.
Once a BasicThread leaves its run() method, a thread is now marked for
cleaning up, and the ThreadMgr will soon join it to release the OS
resources.
Also, adding a function Log::remove_stream() that remove a logging
stream, stopping all writer threads that are associated with it.
Note, however, that removing a *filter* from a stream still doesn't
clean up any threads. The problem is that because of the output paths
potentially being created dynamically it's unclear if the writer
thread will still be needed in the future. We could add clean writers
up with timeouts, but that doesn't sound great either. So for now, the
only way to sure clean up logging threads is to remove the entire
stream.
Also note that cleanup doesn't work with input threads yet, which
don't seem to terminate (at least in the case I tried).
If a thread command fails (like the input framework not finding a
file), that now (1) no longer hangs Bro, and (2) even allows for
propagating error messages back before the thread is stops.
(Actually, the thread doesn't really "stop"; the thread manager keeps
threads around independent of their success; but it no longer polls
them for input.)
Closes#858.
