Hi, I've finally integrated this patch, even though I don't really understand all intents behind it, and I'm almost certain that this still has some subtle issues. I'd like to revamp the way how blocking (e.g. threaded) destinations work, and move threading and synchronization into the core, rather than having to implement them in all plugins. Currently both mongodb and the sql destinations have a similar, but slightly different implementations of the threading part, and I'm afraid the same bug that this patch fixes affects mongodb too (even though that doesn't support flush-lines yet). Also, I think we'll have a bit more similar blocking style destination drivers, and I wouldn't want to duplicate code into them (one is already available in the SMTP plugin, which I guess is structured similarly, but I do have some further ideas, like SNMP trap sender), but since this fixes a concrete bug, and I'd like to push 3.3 out of the door, I wouldn't want to start that refactorization now. This is the patch ID of the commit that integrates both this and the follow-up patch into a single commit: commit 3d8c32601151343506be61a3f5d0af9888da785b Author: Balazs Scheidler <bazsi@balabit.hu> Date: Thu Aug 11 09:34:05 2011 +0200 [sql] send a signal to the database thread only in case when the queue is syncronised because in other cases the database thread wake up unnecessary many times in case of threaded mode And the sql stucked if the explicit commit is turned on and flush_lines and log_iw_size and fetch_limit and log_fifo_size have the same value. The reason of the stuck was that the queue become full but the database thread can't pop any messages form the queue. So it can be occurred that the reader finished the io_job and suspended and the database thread wait for the signal with 0 read message. Signed-off-by: Viktor Juhasz <jviktor@balabit.hu> On Mon, 2011-06-27 at 12:42 +0200, Juhasz Viktor wrote:
Hi,
Sorry the previous patch is not correct because it causes deadlock in case of no multithread. The correction of the patch is the following:
[afsql] Fix afsql deadlock which is caused the previous patch (send a signal to the database thread only in case when the queue is syncronised) In case of no multithread the push tail call the push notify and it causes deadlock of sql
Signed-off-by: Viktor Juhasz (jviktor@balabit.hu) diff --git a/modules/afsql/afsql.c b/modules/afsql/afsql.c index bbdd1d2..27fff99 100644 --- a/modules/afsql/afsql.c +++ b/modules/afsql/afsql.c @@ -1173,8 +1173,10 @@ afsql_dd_queue(LogPipe *s, LogMessage *msg, const LogPathOptions *path_options,
g_mutex_lock(self->db_thread_mutex); queue_was_empty = log_queue_get_length(self->queue) == 0; + g_mutex_unlock(self->db_thread_mutex); log_queue_push_tail(self->queue, msg, path_options);
+ g_mutex_lock(self->db_thread_mutex); if (queue_was_empty && !self->db_thread_suspended) { log_queue_set_parallel_push(self->queue, 1, afsql_dd_queue_notify, self, NULL);
Juhasz Viktor wrote:
------------------------------------------------------------------------
______________________________________________________________________________ Member info: https://lists.balabit.hu/mailman/listinfo/syslog-ng Documentation: http://www.balabit.com/support/documentation/?product=syslog-ng FAQ: http://www.balabit.com/wiki/syslog-ng-faq
______________________________________________________________________________ Member info: https://lists.balabit.hu/mailman/listinfo/syslog-ng Documentation: http://www.balabit.com/support/documentation/?product=syslog-ng FAQ: http://www.balabit.com/wiki/syslog-ng-faq
-- Bazsi