Mailing List Archive

[Bug 3503] [review] local.cf not reread, forced to use @GLOBAL for everything...
http://bugzilla.spamassassin.org/show_bug.cgi?id=3503

parkerm@pobox.com changed:

What |Removed |Added
----------------------------------------------------------------------------
Summary|local.cf not reread, forced |[review] local.cf not
|to use @GLOBAL for |reread, forced to use
|everything... |@GLOBAL for everything...



------- Additional Comments From parkerm@pobox.com 2004-06-15 22:12 -------
Please give a careful review.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 3503] [review] local.cf not reread, forced to use @GLOBAL for everything... [ In reply to ]
http://bugzilla.spamassassin.org/show_bug.cgi?id=3503





------- Additional Comments From jm@jmason.org 2004-06-15 23:00 -------
just wondering: is there a reason the config shouldn't be saved for *all* cases?
I'm worried we're going to miss one this way.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 3503] [review] local.cf not reread, forced to use @GLOBAL for everything... [ In reply to ]
http://bugzilla.spamassassin.org/show_bug.cgi?id=3503





------- Additional Comments From parkerm@pobox.com 2004-06-16 09:48 -------
Subject: Re: [review] local.cf not reread, forced to use @GLOBAL for everything...

On Tue, Jun 15, 2004 at 11:00:37PM -0700, bugzilla-daemon@bugzilla.spamassassin.org wrote:
>
> ------- Additional Comments From jm@jmason.org 2004-06-15 23:00 -------
> just wondering: is there a reason the config shouldn't be saved for *all* cases?
> I'm worried we're going to miss one this way.
>

This is a good question. One I'm not completely prepared to answer,
Theo would have a much better answer for you I'm sure. Here are a few
things off the top of my head:

1) Performance, for the cases where it is not needed it's a
performance hit to have to copy/restore the config each time.
* Goes off an runs a benchmark...applies patch from 3334 because
the problem crops up with the change(which fixes it BTW)...I'm
seeing ~8% slowdown when forced to copy/restore for every request
(when it isn't necessary) *

2) Requires Storable, which is yet another dependency, but IMHO not
one that should be much of a problem, tons of modules these days
require Storable.

3) Probably more....

In reality, once you add in all the setuid stuffs, sql-config and
ldap-config the remaining use cases are pretty small so perhaps we
should turn this on it's head and add a parameter to turn off the
copy/restore so folks who know it doesn't affect them can turn it
off.

Like I said, I'm not totally comfortable with the change and am
willing to do something else if it makes more sense.

Michael






------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 3503] [review] local.cf not reread, forced to use @GLOBAL for everything... [ In reply to ]
http://bugzilla.spamassassin.org/show_bug.cgi?id=3503





------- Additional Comments From parkerm@pobox.com 2004-06-16 12:58 -------
Created an attachment (id=2039)
--> (http://bugzilla.spamassassin.org/attachment.cgi?id=2039&action=view)
Patch - Copy/Restore config on every request

Here is one alternative, copy/restore the config on every request. My
benchmarks showed ~8% slowdown with this patch in place. Please note that this
includes a patch from Bug 3334 since this code change causes that error to
re-appear.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 3503] [review] local.cf not reread, forced to use @GLOBAL for everything... [ In reply to ]
http://bugzilla.spamassassin.org/show_bug.cgi?id=3503

parkerm@pobox.com changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #2037|Patch File |Patch - copy/restore config
description| |for setuid cases as well as
| |sql-config and ldap-config





------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 3503] [review] local.cf not reread, forced to use @GLOBAL for everything... [ In reply to ]
http://bugzilla.spamassassin.org/show_bug.cgi?id=3503





------- Additional Comments From parkerm@pobox.com 2004-06-16 13:03 -------
I should add that if we go with the patch in 2039 we probably want to do some
Storable checking in Makefile.PL and all of the spamd tests.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 3503] [review] local.cf not reread, forced to use @GLOBAL for everything... [ In reply to ]
http://bugzilla.spamassassin.org/show_bug.cgi?id=3503





------- Additional Comments From jm@jmason.org 2004-06-17 22:43 -------
we really need a comment from Theo on this one. Theo?



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 3503] [review] local.cf not reread, forced to use @GLOBAL for everything... [ In reply to ]
http://bugzilla.spamassassin.org/show_bug.cgi?id=3503





------- Additional Comments From parkerm@pobox.com 2004-06-18 08:23 -------
Subject: Re: [review] local.cf not reread, forced to use @GLOBAL for everything...

The best work around for this problem is to set
--max-conn-per-child=1

That will basically revert spamd to same behavior it had before the
pre-fork code was added.

I'm beginning to feel much more confident in the first patch (2037)
BTW, if it turns out there are yet more options that we need to
copy/restore config for then it should be fairly easy to add in.

So, I'm +1 for 2037 (yes, I will fix the extra parentheses before I
commit).

Michael





------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 3503] [review] local.cf not reread, forced to use @GLOBAL for everything... [ In reply to ]
http://bugzilla.spamassassin.org/show_bug.cgi?id=3503

parkerm@pobox.com changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #2037 is|0 |1
obsolete| |
Attachment #2039 is|0 |1
obsolete| |



------- Additional Comments From parkerm@pobox.com 2004-06-18 18:21 -------
Created an attachment (id=2051)
--> (http://bugzilla.spamassassin.org/attachment.cgi?id=2051&action=view)
Patch File




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 3503] [review] local.cf not reread, forced to use @GLOBAL for everything... [ In reply to ]
http://bugzilla.spamassassin.org/show_bug.cgi?id=3503





------- Additional Comments From parkerm@pobox.com 2004-06-18 18:22 -------
Ok, last update, changed just slightly.

Please review, I think this is suitable for putting in and getting a pre-release
out. Theo can chime in when he returns and we can fix/update then.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 3503] [review] local.cf not reread, forced to use @GLOBAL for everything... [ In reply to ]
http://bugzilla.spamassassin.org/show_bug.cgi?id=3503





------- Additional Comments From jm@jmason.org 2004-06-18 18:31 -------
+1.

btw the INSTALL doc needs to be changed to note that Storable is req'd to use
SQL/LDAP as well, I think. but that doesn't require a new patch since it's a
doco change.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 3503] [review] local.cf not reread, forced to use @GLOBAL for everything... [ In reply to ]
http://bugzilla.spamassassin.org/show_bug.cgi?id=3503





------- Additional Comments From quinlan@pathname.com 2004-06-18 21:28 -------
+1 ditto what Justin said




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 3503] [review] local.cf not reread, forced to use @GLOBAL for everything... [ In reply to ]
http://bugzilla.spamassassin.org/show_bug.cgi?id=3503

parkerm@pobox.com changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution| |FIXED



------- Additional Comments From parkerm@pobox.com 2004-06-18 21:39 -------
Committed revision 21439.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.