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

[Gergely Nagy]

For the record, my current work on the LogThreadedDestDriver is
available from the feature/3.4/threaded-destdriver branch of my repo:
  https://github.com/algernon/syslog-ng/commits/feature/3.4/threaded-destdriver

Balazs Scheidler <bazsi at balabit.hu> writes:


>> 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.
>
> I would prefer OOP-style virtual methods over an additional layer of
> callbacks. As long as the init function can simply be invoked by the
> subclass.

I went with a mix, because that allowed me to have the bulk of the code
in the class easily, and didn't require the converted drivers to go to
great lengths if they wanted to override a particular function.

>> * 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).
>
> Probably separate methods for start_txn and commit_txn and a txn_size or
> something. start/commit would do nothing by default. txn_size would
> probably be the same as flush_lines.

As a first step, I'd refactor the code still keeping flush_lines and the
slightly broken queue_pop_head(), convert whatever can be to
LogThreadedDestDriver, and go from there.

Fixing the queue_pop_head() situation and flush_lines would be an
entirely separate project for me, primarly so I don't loose track O:)

> Also, we should get rid of flush_timeout() and close transactions
> whenever the input queue empties, just like Patrick suggested in an
> earlier thread. Also the BalaBit syslog-ng team has decided the same for
> their 4.2 version. Their code is available on git.balabit.hu, so we can
> borrow their implementation.

Thanks, I'll take a look!


>> 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)
>> 
>
> This sounds like a plan :)

If you have a few spare moments, and could have a look at my
feature/3.4/threaded-destdriver branch, I'd appreciate any feedback.

(You might wish to revert the top patch, "WIP: $something", because that
does not compile. I put it there so I can work it over the long weekend,
but I was busy with other things and didn't get around to it.)

-- 
|8]