[syslog-ng] [RFC]: LogThreadedDestDriver - making threaded destinations better

[Gergely Nagy]

Hi!

There have been a number of fixes going into afsql or afmongodb (or
both) recently, fixing bugs that affected both, for the simple reason
that when I made afmongodb, I copied parts of the architecture from
afsql. Now there is the SMTP destination aswell, suffering from the same
issues, and a Redis destination is in the works aswell.

Therefore, it is about time to rethink how these drivers that all look
very similar under the hood could be rebased onto a common foundation,
something that abstracts away all the common stuff, such as handling the
worker thread, and the queue method - so all the affected drivers need
to do, is implement their own tasks, and skip all the copy-paste-ish
boilerplate they have now.

To this end, we had a short brainstorming with Bazsi, and I'll try to
summarize what I remember (with a few later ideas added).

The goal is to create a new class, based on LogDestDriver, for all those
destination drivers that use a worker thread to do their blocking
operations. This new class, which for the time being shall be named
LogThreadedDestDriver shall implement the following things:

* It must handle the setup, starting and stopping of the worker thread,
  with a module-supplied worker function, and that function shall be
  assisted by class functions to do the dirty work.
* The queue handling must be safe and free of races (which pretty much
  means we have to find a way to wrap log_queue_pop_head into a
  synchronous call)
* The class must be prepared to work not only with single messages, but
  in bulk aswell (think SQL's transactions, or the upcoming MongoDB bulk
  insert)
* The class must implement its own queue method, downstream modules
  should not need to care about that.
* We must be able to wake up the worker thread, even if there is nothing
  to handle: this is needed for a clean shutdown.

And that's pretty much it.

The class would have callbacks for the following things:

* A pair of callbacks to call during the driver's _init() and _deinit()
  functions. These can either be implemented as callbacks, and the
  driver will then use it's own init/deinit and set it in its parent,
  and call the module-set callbacks from those.

  Or, the class can provide _init/_deinit methods, and leave it up to
  the module to implement its own methods, and call the class methods
  itself.

  The former makes drivers that do not require extra init/deinit
  simpler, the latter has less callbacks and looks neater. When
  implementing, I'll probably go with the latter.

* An 'do-the-work' callback, which is called whenever work needs to be
  performed, and an error callback to fire whenever the do-the-work
  failed.

  I'm not entirely sure how best to handle the transaction case here,
  but that's something that needs to be done around here.

  One possibility is to allow two kinds of do-the-work callbacks: one
  for the simple case where no transactions are done (afsmtp and current
  afmongodb) and one that supports transactions/bulk inserts (afsql and
  future afmongodb and redis).

Off the top of my head, this should be enough to move the boring parts
of the current threaded destination drivers into a common class. Once
the transaction case is figured out, all of our drivers can be
reimplemented on top of this class, reducing copied and slightly altered
code by a HUGE margin, and finally making it so that we have a single
place to fix possible bugs in, instead of 3 slightly different copies of
the same thing.

As a side effect, time_reopen and stored/dropped counter handling would
get moved from individual drivers into this base class, along with
flush_timeout and flush_lines (if those still exist in 3.4) and a couple
of other things.

I plan to have a Proof of Concept patch ready by friday night, we'll see
how it goes!

(And shortly after, a redis destination will follow)

-- 
|8]