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]