[syslog-ng] [PATCH] afsql: Fix sql stuck problema

[Balazs Scheidler]

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 at 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 at 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 at 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