[Mailman-Developers] (More) pristine archives

[Barry A. Warsaw]

>>>>> "DN" == Dale Newfield <Dale@Newfield.org> writes:

    >> Thus, if the code in the try causes an exception, the list will
    >> still get unlocked (otherwise the list would be hosed), but the
    >> transaction is aborted by virtue of not getting saved.  A
    >> subsequent load of the mlist would begin a new transaction,
    >> with the old data.

    DN> Right, but since .UnLock doesn't reload the data, any code
    DN> refering to that mlist after unlocking would have different
    DN> semantics depending upon which MemberAdaptor implementation is
    DN> backing the list: With OldStyle, membership data in that mlist
    DN> object is whatever it's been (temporarily) set to (but not
    DN> saved); with SQL, membership data in that mlist has reverted
    DN> to whatever it was when .Lock() was called.

You're right, and it should be cleaned up, but in practice it should
be okay.  For the one-shot scripts (cron, cgi), you're restarting the
process each time anyway, so no worries.  For the qrunner daemons, I
basically had to solve the same problem, i.e. the next iteration
through the loop needs to have consistent data or you're screwed.
Each runner should either do a .Load() or a .Lock() at the top of
their _dispose() methods, depending on if they only need read access
to the list data or read-write access.

    >> It's this last bit that's dodgy.  There should probably be an
    >> explicit abort on exceptions inside the try, but there's no way
    >> to spell that with the current, legacy persistence mechanism,
    >> so it isn't in any of the code.

    DN> Wouldn't that abort be triggered by a call to .UnLock()
    DN> without a call to .Save()?  I would think that all calls to
    DN> .Lock() and any calls to .UnLock() without a prior call to
    DN> .Save() should abort any current transaction.

What about the qrunners that don't lock the list because they only
need read access to the data?  That's why I think we need an explicit
abort, even if it's no-op'd for the old-style persistence.

    >> I /think/ that if your MailList.Load() implicitly aborts any
    >> active transaction, you should be okay, but of course, none of
    >> that's tested.

    DN> MailList.Load() or MailList.Lock()?  Having .Load() abort any
    DN> active transaction means that you cannot load other mlists
    DN> (even read-only) inside a .Lock();try....Save();finally
    DN> .UnLock() block and have the transaction succeed...

    DN> The place these two models (SQL transactions and mailList
    DN> load, lock/save/unlock) break down is what happens when there
    DN> are more than one MailList object in memory at a time.  SQL
    DN> transactions assumes only one MailList can be modified at a
    DN> time, and the lock/save/unlock model doesn't make that
    DN> assumption.

Ah, so the problem probably isn't the transaction boundaries, but that
Mailman assumes that each list's persistence is completely independent
of other lists.  I think the one place where you'll get hosed by this
is in the cgi's where "global" operations loop through all the lists
(yes, this sucks and is inefficient, but its the best we can currently
do).  I'm not sure how to get around this, except through some kind of
elaborate nested transaction support.

    DN> If we can assume that only one mlist gets locked at a time,
    DN> the SQL system will work, but I see no way to enforce mailman
    DN> developers to abide by that assumption.  (Except to implicitly
    DN> abort active transactions as described above--and since the
    DN> MailList.Save() method doesn't have a success/failure return
    DN> code, there would be no indication that anything went wrong
    DN> except silently ignoring requested DB changes.)

Hmm.  I'm going to have to think about this some more.  I'm off line
right now so can't look at code details.

-Barry