On 11/22/10 14:58, Christian Rohmann wrote:
Hey there,

On 22.11.2010 13:44, "Stähelin, Simon (ID)" wrote:
I got some further. It seems that (maybe) just the logline is wrong. I
sent two mail form my account, after the first mail, the log said
quota=5/500 (why not 1/500).

We are seeing the exact same problem with 2.0.11RC1, but not with 2.0.10
stable.  I can confirm that it's not JUST the logline. The handling of
the quota being calculated and written back to DB is broken.

I did set my counter limit to 1000 and started mailing ... from the
first mail on for quite a few it constantly logged 5/1000 (0.5%) (which
is already wrong). Then all of a sudden the values jumped like crazy to
270 and then to +800 and then over 1000 ->  further emails are blocked.

With 2.0.10 it works just like expected. policyd counts from 1 to 1000
with a dampening through the time that is already passed.



I also found the following entried in the logfile of 2.0.11RC1:

--- policyd.log ---
Use of uninitialized value $currentCounter in subtraction (-) at
cbp/modules/Quotas.pm line 179,<_READ>  line 25.
Use of uninitialized value $currentCounter in subtraction (-) at
cbp/modules/Quotas.pm line 179.
Use of uninitialized value $currentCounter in subtraction (-) at
cbp/modules/Quotas.pm line 179.
Use of uninitialized value $currentCounter in subtraction (-) at
cbp/modules/Quotas.pm line 179,<_READ>  line 33.
Use of uninitialized value $currentCounter in subtraction (-) at
cbp/modules/Quotas.pm line 179.
Use of uninitialized value $currentCounter in subtraction (-) at
cbp/modules/Quotas.pm line 179,<_READ>  line 12170.
Use of uninitialized value $currentCounter in subtraction (-) at
cbp/modules/Quotas.pm line 179,<_READ>  line 23154.
Use of uninitialized value $currentCounter in subtraction (-) at
cbp/modules/Quotas.pm line 179,<_READ>  line 16998.
Use of uninitialized value $currentCounter in subtraction (-) at
cbp/modules/Quotas.pm line 179,<_READ>  line 24152.
Use of uninitialized value $currentCounter in subtraction (-) at
cbp/modules/Quotas.pm line 179,<_READ>  line 26804.
Use of uninitialized value $currentCounter in subtraction (-) at
cbp/modules/Quotas.pm line 179,<_READ>  line 26804.
Use of uninitialized value $currentCounter in subtraction (-) at
cbp/modules/Quotas.pm line 179,<_READ>  line 16998.
Use of uninitialized value $currentCounter in subtraction (-) at
cbp/modules/Quotas.pm line 179,<_READ>  line 26761.
Use of uninitialized value $currentCounter in subtraction (-) at
cbp/modules/Quotas.pm line 179,<_READ>  line 28051.
Use of uninitialized value $currentCounter in subtraction (-) at
cbp/modules/Quotas.pm line 179,<_READ>  line 23747.
Use of uninitialized value $currentCounter in subtraction (-) at
cbp/modules/Quotas.pm line 179,<_READ>  line 26761.
--- / policyd.log ---



So I strongly believe there is a bug within the QUOTA module.

Hi Christian,

Could you try the attached patch using v2.0.11RC1? Please let us know should this fix the problems you are experiencing.

Robert
Index: cbp/modules/Quotas.pm
===================================================================
--- cbp/modules/Quotas.pm	(revision 517)
+++ cbp/modules/Quotas.pm	(working copy)
@@ -163,30 +163,32 @@
 						# Check if we have a queue tracking item
 						if (defined($qtrack)) {
 							my $elapsedTime = defined($qtrack->{'LastUpdate'}) ? ( $now - $qtrack->{'LastUpdate'} ) : $quota->{'Period'};
+							# If elapsed time is less than zero, its time diff between servers, meaning no time has elapsed
+							$elapsedTime = 0 if ($elapsedTime < 0);
 							
 							# Check if elapsedTime is longer than period, or negative (time diff between servers?)
 							my $currentCounter;
-							if ($elapsedTime > $quota->{'Period'} || $elapsedTime < 0) {
-								$qtrack->{'Counter'} = 0;
+							if ($elapsedTime > $quota->{'Period'}) {
+								$currentCounter = 0;
 	
 							# Calculate the % of the period we have, and multiply it with the counter ... this should give us a reasonably
 							# accurate counting
 							} else {
 								$currentCounter = ( 1 - ($elapsedTime / $quota->{'Period'}) ) * $qtrack->{'Counter'};
 							}
-								
-							# Make sure increment is at least 0
-							$newCounters{$qtrack->{'QuotasLimitsID'}} = defined($qtrack->{'Counter'}) ?
-									$qtrack->{'Counter'} - $currentCounter : $qtrack->{'Counter'}
-									if (!defined($newCounters{$qtrack->{'QuotasLimitsID'}}));
-	
+
+							# Work out the difference to the DB value, we ONLY DO THIS ONCE!!! so if its defined, leave it alone!
+							if (!defined($newCounters{$qtrack->{'QuotasLimitsID'}})) {
+								$newCounters{$qtrack->{'QuotasLimitsID'}} = $currentCounter - $qtrack->{'Counter'};
+							}
+
 							# Limit type
 							my $limitType = lc($limit->{'Type'});
 
 							# Make sure its the MessageCount counter
 							if ($limitType eq "messagecount") {
 								# Check for violation
-								if ($qtrack->{'Counter'} > $limit->{'CounterLimit'}) {
+								if ($currentCounter > $limit->{'CounterLimit'}) {
 									$hasExceeded = "Policy rejection; Message count quota exceeded";
 								}
 								# Bump up limit
@@ -195,7 +197,7 @@
 							# Check for cumulative size violation
 							} elsif ($limitType eq "messagecumulativesize") {
 								# Check for violation
-								if ($qtrack->{'Counter'} > $limit->{'CounterLimit'}) {
+								if ($currentCounter > $limit->{'CounterLimit'}) {
 									$hasExceeded = "Policy rejection; Cumulative message size quota exceeded";
 								}
 							}
@@ -206,9 +208,10 @@
 							$qtrack->{'Counter'} = 0;
 							$qtrack->{'LastUpdate'} = $now;
 								
-							# Make sure increment is at least 0
-							$newCounters{$qtrack->{'QuotasLimitsID'}} = $qtrack->{'Counter'} 
-									if (!defined($newCounters{$qtrack->{'QuotasLimitsID'}}));
+							# Work out the difference to the DB value, we ONLY DO THIS ONCE!!! so if its defined, leave it alone!
+							if (!defined($newCounters{$qtrack->{'QuotasLimitsID'}})) {
+								$newCounters{$qtrack->{'QuotasLimitsID'}} = $qtrack->{'Counter'};
+							}
 							
 							# Check if this is a message counter
 							if (lc($limit->{'Type'}) eq "messagecount") {
@@ -247,7 +250,7 @@
 			foreach my $qtrack (@trackingList) {
 					
 				# Percent used
-				my $pused =  sprintf('%.1f', ( ($newCounters{$qtrack->{'QuotasLimitsID'}} + $qtrack->{'QuotasLimitsID'}) / $qtrack->{'CounterLimit'} ) * 100);
+				my $pused =  sprintf('%.1f', ( ($newCounters{$qtrack->{'QuotasLimitsID'}} + $qtrack->{'Counter'}) / $qtrack->{'CounterLimit'} ) * 100);
 
 				# Update database
 				my $sth = DBDo("
@@ -296,7 +299,7 @@
 							$qtrack->{'LimitID'},
 							$qtrack->{'DBKey'},
 							$qtrack->{'LimitType'},
-							sprintf('%.0f',$newCounters{$qtrack->{'QuotasLimitsID'}} + $qtrack->{'QuotasLimitsID'}),
+							sprintf('%.2f',$newCounters{$qtrack->{'QuotasLimitsID'}} + $qtrack->{'Counter'}),
 							$qtrack->{'CounterLimit'},
 							$pused);
 
@@ -315,7 +318,7 @@
 							$qtrack->{'LimitID'},
 							$qtrack->{'DBKey'},
 							$qtrack->{'LimitType'},
-							sprintf('%.0f',$newCounters{$qtrack->{'QuotasLimitsID'}} + $qtrack->{'QuotasLimitsID'}),
+							sprintf('%.2f',$newCounters{$qtrack->{'QuotasLimitsID'}} + $qtrack->{'Counter'}),
 							$qtrack->{'CounterLimit'},
 							$pused);
 
@@ -329,7 +332,7 @@
 		# If we have exceeded, set verdict
 		} else {
 			# Percent used
-			my $pused =  sprintf('%.1f', ( ($newCounters{$exceededQtrack->{'QuotasLimitsID'}} + $exceededQtrack->{'QuotasLimitsID'}) / $exceededQtrack->{'CounterLimit'} ) * 100);
+			my $pused =  sprintf('%.1f', ( ($newCounters{$exceededQtrack->{'QuotasLimitsID'}} + $exceededQtrack->{'Counter'}) / $exceededQtrack->{'CounterLimit'} ) * 100);
 
 			# Log rejection to mail log
 			$server->maillog("module=Quotas, action=%s, host=%s, helo=%s, from=%s, to=%s, reason=quota_match, policy=%s, quota=%s, limit=%s, track=%s, "
@@ -344,10 +347,9 @@
 					$exceededQtrack->{'LimitID'},
 					$exceededQtrack->{'DBKey'},
 					$exceededQtrack->{'LimitType'},
-					sprintf('%.0f',$newCounters{$exceededQtrack->{'QuotasLimitsID'}} + $exceededQtrack->{'QuotasLimitsID'}),
+					sprintf('%.2f',$newCounters{$exceededQtrack->{'QuotasLimitsID'}} + $exceededQtrack->{'Counter'}),
 					$exceededQtrack->{'CounterLimit'},
 					$pused);
-
 			$verdict = $exceededQtrack->{'Verdict'};
 			$verdict_data = (defined($exceededQtrack->{'VerdictData'}) && $exceededQtrack->{'VerdictData'} ne "") 
 					? $exceededQtrack->{'VerdictData'} : $hasExceeded;
@@ -411,14 +413,14 @@
 							# Check if we're working with cumulative sizes
 							if (lc($limit->{'Type'}) eq "messagecumulativesize") {
 								# Bump up counter
-								$qtrack->{'Counter'} += $sessionData->{'Size'};
+								my $currentCounter = $qtrack->{'Counter'} + $sessionData->{'Size'};
 								
 								# Update database
 								my $sth = DBDo("
 									UPDATE 
 										quotas_tracking
 									SET
-										Counter = ".DBQuote($qtrack->{'Counter'}).",
+										Counter = Counter + ".DBQuote($sessionData->{'Size'}).",
 										LastUpdate = ".DBQuote($now)."
 									WHERE
 										QuotasLimitsID = ".DBQuote($qtrack->{'QuotasLimitsID'})."
@@ -430,7 +432,7 @@
 								}
 
 								# Percent used
-								my $pused =  sprintf('%.1f', ( $qtrack->{'Counter'} / $limit->{'CounterLimit'} ) * 100);
+								my $pused =  sprintf('%.1f', ( $currentCounter / $limit->{'CounterLimit'} ) * 100);
 
 								# Log update to mail log
 								$server->maillog("module=Quotas, mode=update, host=%s, helo=%s, from=%s, to=%s, reason=quota_update, policy=%s, "
@@ -444,7 +446,7 @@
 										$limit->{'ID'},
 										$key,
 										$limit->{'Type'},
-										sprintf('%.0f',$qtrack->{'Counter'}),
+										sprintf('%.2f',$currentCounter),
 										$limit->{'CounterLimit'},
 										$pused);
 							} # if (lc($limit->{'Type'}) eq "messagecumulativesize")
_______________________________________________
Users mailing list
[email protected]
http://lists.policyd.org/mailman/listinfo/users

Reply via email to