On Tue, Feb 05, 2008 at 05:30:13PM +0000, Tony Finch wrote:
> I have been working on some improvements to Exim's ratelimit ACL
> condition. I think it basically works, so I thought it would be helpful to
> get some feedback on it before the next release. Here's the draft of the
> NewStuff entry, and the patch is after my .sig:
Great! Exim's ratelimiting feature is already very powerful, and the
changes you describe sound like good improvements. We use
rate-limiting heavily for a number of things at our site, and it works
quite well.
> The code now checks that the per_* options are not used in ACLs where it
> does not make sense, for example using per_mail in acl_smtp_helo or any
> other time when Exim is not handling a message.
Hmmm. I use /per_rcpt in our acl_smtp_mail section, but I use the
/noupdate tag to do a read-only read of the value. That way I can
increment recipients at RCPT time, but I can reject future messages
at MAIL time based upon previous rcpt counts.
Would your change prevent this?
Using your example above, someone might still want to do a read of
/per_rcpt counters at acl_smtp_helo time using the /noupdate tag. If
you blocked per_rcpt, perhaps someone could use /per_cmd for the read
since it is (or was) a synonym for /per_rcpt?
Just keep in mind that people might (and will) want to do /noupdate reads
of /per_rcpt and /per_mail values from unusual places.
> The /leaky, /strict, and /noupdate options are now alternatives.
> (Previously the /noupdate option was in addition to /strict or /leaky.)
> Also, the update mode is no longer recorded in the database, which may
> cause clashes if you are using /leaky and /strict with the same key.
Makes sense. Are one of the three options required to be used, or
will the default still be /leaky if none are specified?
> The first new feature is the /count= option. This is a generalization
> of the per_byte option, so that you can measure the throughput of other
> aggregate values. For example, the /per_byte option is now equivalent
> to /per_mail/count=${if <{0}{$message_size} {0} {$message_size} }, and
> when it isn't used in acl_smtp_rcpt, the per_rcpt option is equivalent
> to /per_mail/count=$recipients_count.
I like what you've done here. Last year, I submitted the patch to
make ratelimiting use $recipients_count when used in a non-smtp
context. The idea to use /count=$recipients_count outside of
acl_smtp_rcpt is a much more elegant way to do the same thing without
hardcoding an unusual exception.
> The other new feature is a mechanism for counting the rate of unique
> events. The new /per_addr option counts the number of different
> recipients that someone has sent messages to in the last time period.
> Like the /count= option this is a general mechanism, so the /per_addr
> option is equivalent to /per_rcpt/unique=$local_part@$domain. You can,
> for example, measure the rate that a client uses different sender
> addresses with the options /per_mail/unique=$sender_address.
I'll have to play with this one to get a feel for it, but sounds like a
good addition.
> The exim_dumpdb utility does not display /unique= event sets because they
> are represented in a form that makes a human-readable representation
> impossible. However you can use exim_fixdb to test membership of a set or
> to add events to it.
The only other consideration I can think of is to make sure exim_tidydb
still properly tidies the ratelimit database properly. Without tidying,
ratelimit databases can grow VERY large, especially if used on incoming
MX mail.
--
Dean Brooks
dean@???
