Re: [Sqlgrey-users] [SPAM] Re: Patches for SQLgrey

[Kenneth M. <kt...@ri...>]

On Tue, Aug 18, 2009 at 04:48:59PM +1200, Michal Ludvig wrote:
> Michal Ludvig wrote:
> 
> > Eventually we could rework the whole cleaning process. Instead of doing
> > (potentially massive) cleanups in fixed time periods there could be a
> > minor cleanup on every, say, 1000th call. No need to count the calls,
> > simple 'if(int(rand(1000))==0) { .. cleanup .. }' should be sufficient.
> 
> Actually... attached is a patch that implements it. I'm running it with
> cleanup_chance=10 (ie 1:10 chance to perform cleanup) on one of my
> light-loaded servers and it seems to do the job. So far I've seen 13
> cleanups in 150 greylist checks which is quite on track.
> 
> 
> BTW How about dropping the async cleanup path completely? Given the
> scary comment in sqlgrey.conf I don't believe anyone dares to use it anyway.
> 
> Michal

I would rather get the async cleanup path working. This will allow
non-locking backends to continue to process incoming mail connections
without blocking during the cleanup. A synchronous cleanup process is
pretty painful on a busy mail system, and will get more painful the
busier it gets.

Regards,
Ken

> Index: sqlgrey
> ===================================================================
> RCS file: /cvsroot/sqlgrey/sqlgrey/sqlgrey,v
> retrieving revision 1.110
> diff -u -r1.110 sqlgrey
> --- sqlgrey	17 Aug 2009 13:15:03 -0000	1.110
> +++ sqlgrey	18 Aug 2009 03:13:20 -0000
> @@ -86,6 +86,8 @@
>  $dflt{discrimination} 	  = 0;
>  $dflt{discrimination_add_rulenr} = 0;
>  
> +$dflt{cleanup_chance}     = 1000;	# Cleanup the DB roughly in 1:1000 calls.
> +
>  $dflt{log} = { # note values here are not used
>  	      'grey' => 2,
>  	      'whitelist' => 2,
> @@ -1608,16 +1610,13 @@
>  }
>  
>  ## Choose the actual cleanup method
> -sub start_cleanup {
> +sub try_cleanup {
>      my $self = shift;
>  
> -    if ($dflt{dont_db_clean}) {
> -            $self->mylog('conf', 2, "This host has db-cleaning disabled");
> -            return;
> -    }
> +	return if (int(rand($dflt{cleanup_chance})) != 0);
>  
>      if ($self->{sqlgrey}{clean_method} eq 'sync') {
> -        $self->cleanup();
> +		$self->cleanup();
>      } else {
>          $self->fork_cleanup();
>      }
> @@ -2309,31 +2308,8 @@
>      # we need the value of now() in the database
>      $self->update_dbnow();
>  
> -    # If !defined last_dbclean, reload value from DB
> -    if (!defined $self->{sqlgrey}{last_dbclean}) {
> -        $self->{sqlgrey}{last_dbclean} = $self->getconfig('last_dbclean');
> -
> -        # if last_dbclean not found in db then write it.
> -        if (!defined $self->{sqlgrey}{last_dbclean}) {
> -	     # 0 will force a cleanup (unless db_cleandelay is really huge)
> -             $self->setconfig('last_dbclean',0);
> -             $self->{sqlgrey}{last_dbclean} = 0;
> -        }
> -    }
> -
> -    # Is it time for cleanups ?
> -    my $current_time = time();
> -    if ($current_time > ($self->{sqlgrey}{last_dbclean} + $self->{sqlgrey}{db_cleandelay})) {
> -            # updateconfig() returns affected_rows
> -            if ($self->updateconfig('last_dbclean',$current_time,$self->{sqlgrey}{last_dbclean})) {
> -                  # If affected_rows > 0, its my job to clean the db
> -                  $self->{sqlgrey}{last_dbclean} = $current_time;
> -                  $self->start_cleanup();
> -            } else {
> -                  #If affected_rows == 0, then someone already cleaned db
> -                  $self->{sqlgrey}{last_dbclean} = undef; #make sqlgrey reload time from db on next pass
> -            }
> -    }
> +    # See if we get a chance to clean up the DB a little
> +    $self->try_cleanup();
>  
>      # domain scale awl check
>      if ($self->is_in_domain_awl($sender_domain, $cltid)) {
> @@ -2572,7 +2548,6 @@
>              awl_age         => $dflt{awl_age},
>  	    # How many from match a domain/IP before a switch to domain AWL
>  	    domain_level    => $dflt{group_domain_level},
> -            last_dbclean    => undef, # triggers reload from db
>  	    db_cleandelay   => $dflt{db_cleandelay}, # between table cleanups (seconds)
>  
>              db_prepare_cache => $dflt{db_prepare_cache},
> Index: etc/sqlgrey.conf
> ===================================================================
> RCS file: /cvsroot/sqlgrey/sqlgrey/etc/sqlgrey.conf,v
> retrieving revision 1.19
> diff -u -r1.19 sqlgrey.conf
> --- etc/sqlgrey.conf	17 Aug 2009 12:43:11 -0000	1.19
> +++ etc/sqlgrey.conf	18 Aug 2009 03:13:21 -0000
> @@ -124,13 +124,14 @@
>  # db_prepare_cache = 0 # use prepared statements cache
>                         # BEWARE: memory leaks have been reported
>  		       # when it is active
> -# db_cleandelay = 1800 # in seconds, how much time between database cleanups
>  # clean_method = sync # sync : cleanup is done in the main process,
>                        #        delaying other operations
>                        # async: cleanup is done in a forked process,
>                        #        it won't delay mail processing
>                        #        BEWARE: lockups have been reported
>                        #        and are still investigated
> +# cleanup_chance = 1000    # How often should DB cleanup be performed?
> +                           # By default once in every 1000 calls.
>  
>  ## Database clustering (for advanced setups)
>  #

> ------------------------------------------------------------------------------
> Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
> trial. Simplify your report design, integration and deployment - and focus on 
> what you do best, core application coding. Discover what's new with 
> Crystal Reports now.  http://p.sf.net/sfu/bobj-july
> _______________________________________________
> Sqlgrey-users mailing list
> Sql...@li...
> https://lists.sourceforge.net/lists/listinfo/sqlgrey-users