> On Jul 15, 2020, at 1:09 AM, Robin Sommer <ro...@corelight.com> wrote:
>
> On Thu, Jul 09, 2020 at 18:19 -0700, Bob Murphy wrote:
>
>> Proposed Solution: Add a new optional API for writing a batch all at once,
>> while
>> still supporting older log writers that don't need to write batches.
>
> That sounds good to me, a PR with the proposed API would be great.
That’s sounds great. I wanted to bounce the ideas around with people who know
more about Zeek than i do before going into detail on a proposed API.
>
>> a. For non-batching log writers, change the "false" status to just mean
>> "There was an error writing a log record". The log writing system will then
>> report those failures to other Zeek components such as plug-ins, so they
>> can
>> monitor a log writer's health, and make more sophisticated decisions about
>> whether a log writer can continue running or needs to be shut down.
>
> Not quite sure what this would look like. Right now we just shut down
> the thread on error, right? Can you elaborate how "report those
> failures to other Zeek components" and "make more sophisticated
> decisions" would look like?
Yes, right now, any writer error just shuts down the entire thread.
That’s a good solution for destinations like a disk, because if a write fails,
something really bad has probably happened. But Seth Hall pointed out that some
log destinations can recover, and it’s not a good solution for those.
Here are a couple of examples:
1. A writer might send log records to a network destination. If the connection
is temporarily congested, it would start working again when the congestion
clears.
2. The logs go to another computer that’s hung, and everything would work again
if somebody rebooted it.
Seth's idea was to report the failures to a plugin that could be configured by
an administrator. A plugin for a writer that goes to disk could shut down the
writer on the first failure, like Zeek does now. And plugins for other writers
could approach the examples above with a little more intelligence:
1. The plugin for the network destination writer could decide to shut down the
writer only after no records have been successfully sent for a minimum of ten
minutes.
2. The plugin for the remote-computer writer could alert an administrator to
reboot the other computer. After that, the writer would successfully resume
sending logs.
> Could we just change the boolean result into a tri-state (1) all good;
> (2) recoverable error, and (3) fatal error? Here, (2) would mean that
> the writer failed with an individual write, but remains prepared to
> receive further messages for output. We could the also implicitly
> treat a current "false" as (3), so that existing writers wouldn't even
> notice the difference (at the source code level at least).
I don’t think that would work, because the member function in question returns
a bool. To change that return value to represent more than two states, we’d
have to do one of two things:
1. Change that bool to some other type.
If we did that, existing writers wouldn’t compile any more.
2. Use casts or a union to store and retrieve values other than 0 and 1 in that
bool, and hope those values will be preserved across the function return and
into the code that needs to analyze them.
We can’t count on values other than 0 or 1 being preserved, because the bool
type in C++ is a little weird, and some behaviors are implementation-dependent.
I wrote a test program using a pointer to store 0x0F into a bool, and other
than looking at it in a debugger, everything I did to read the value out of
that bool turned it into 0x01, including assigning it to another bool or an
int. The only thing that saw 0x0F in there was taking a pointer to the bool,
casting it to a pointer to char or uint8_t, and dereferencing that pointer.
>
>> b. Batching log writers will have a new API anyway, so that will let log
>> writers report more detail about write failures, including suggestions
>> about
>> possible ways to recover.
>
> Similar question here: how would these "suggestions" look like?
For batching, I was thinking of having a way to send back a std::vector of
structs that would be something like this:
struct failure_info {
uint32_t index_in_batch;
uint16_t failure_type;
uint16_t recovery_suggestion;
};
The values of failure_type would be an enumeration indicating things like
“fatal, shut down the writer”, “log record exceeds protocol limit”, “unable to
send packet”, “unable to write to disk”, etc. Using a fixed-size struct member
that’s larger than the enum would allow extra values to be added in the future.
recovery_suggestion would be a similar enum-in-larger-type, and let the writer
convey more information, based on what it knows about the log destination. That
could indicate things like, “the network connection has entirely dropped and no
recovery is possible”, “the network connection is busy, try again later”, “this
log record is too large for the protocol, but re-sending it might succeed if
it’s truncated or split up”, etc.
- Bob
_______________________________________________
Zeek-Dev mailing list
Zeek-Dev@zeek.org
http://mailman.icsi.berkeley.edu/mailman/listinfo/zeek-dev
