Storable and 3.1.0

[Justin Mason <jm...@jmason.org>]

BTW, I think we should try to get the code that relies on
Storable out of SpamAssassin for 3.1.0. why?

Because as far as I can see we still have a lot of users reporting
problems with spamd hanging, and (last I heard) it looked like Storable
was the issue.

We've hacked around it with some alarm() timeouts, but we're not sure if
they'll work or not in all cases.

as far as I can see, we are possibly the biggest perl project using
Storable for such an essential and complex piece of code, and I think we
may be running up against the poorly-debugged edge case that nobody else
has had to deal with.

On top of that, I don't see exactly *why* Storable is required to
implement what it's doing (keeping a copy of the basic system-wide Conf
object's data).  as far as I can see, we can do that a la

    %{$conf->{tests}} = %{$basic_conf->{tests}}

ie. direct copying and assignment, in pretty much exactly the same
way we use Storable, but without the worries.

We have spamd reliability issues, I think, in the field -- and
cutting out one possible cause of that seems like a very good idea.
thoughts?

--j.

Re: Storable and 3.1.0

[Theo Van Dinter <fe...@kluge.net>]

On Thu, Apr 14, 2005 at 12:07:00PM -0700, Justin Mason wrote:
> On top of that, I don't see exactly *why* Storable is required to
> implement what it's doing (keeping a copy of the basic system-wide Conf
> object's data).  as far as I can see, we can do that a la
> 
>     %{$conf->{tests}} = %{$basic_conf->{tests}}
> 
> ie. direct copying and assignment, in pretty much exactly the same
> way we use Storable, but without the worries.

Well, the short version is that it works fine for simple scalars, but any
form of reference breaks things horribly.  For example:

# perl -e 'use Data::Dumper; $conf{a}={foo=>bar}; $conf{b}={bar=>baz}; %back = %conf; $conf{b}->{baz} = "schmoo"; print Dumper(\%back);'
$VAR1 = {
          'a' => {
                   'foo' => 'bar'
                 },
          'b' => {
                   'bar' => 'baz',
                   'baz' => 'schmoo'
                 }
        };

copy_config() goes through %conf one at a time and copies, but there's no
guarantee the lower data structures are simple.  We wanted dclone() since
it'll copy complex structures recursively.  We don't need to worry about
references, etc.

> We have spamd reliability issues, I think, in the field -- and
> cutting out one possible cause of that seems like a very good idea.
> thoughts?

I'm +1 for getting rid of Storable in general, and we've even discussed
how to do this before so as to get rid of Storable, increase memory
sharing between parent and client, decrease usage (via not needing a
backup conf hash), etc.

I think that's going to be a bit of work for 3.1, especially if we want
it out sooner rather than later.

-- 
Randomly Generated Tagline:
"sendmail is a big cloud of murky sysadmin magic ...  I'm still an
 apprentice."                             - Theo