[Zope-dev] zope.sqlalchemy locks up transaction

[Laurence Rowe]

2008/9/19 Laurence Rowe <laurence at lrowe.co.uk>:
> 2008/9/19 Hermann Himmelbauer <dusty at qwer.tk>:
>> Am Donnerstag 18 September 2008 07:08:56 schrieb Dieter Maurer:
>>> Brian Sutherland wrote at 2008-9-17 12:33 +0200:
>>> >On Tue, Sep 16, 2008 at 02:01:07PM +0100, Laurence Rowe wrote:
>>> >> Brian Sutherland wrote:
>>> >> > Hi,
>>> >> >
>>> >> > I've recently seen a situation where zope.sqlalchemy locked up the
>>> >> > transaction machinery. I'm not sure exactly what happened, but have
>>> >> > attached a failing test for at least one bug which may have caused it.
>>> >> > Hopefully it's self explanatory;)
>>> >> >
>>> >> > If someone could help me solve this, that would be great!
>>> >>
>>> >> Could you try this with latest trunk. I checked in a fix the other day
>>> >> that may help.
>>> >
>>> >I just checked in a fix, please feel free to comment on/revert it if
>>> >it's not up to standard:)
>>>
>>> It looks not yet right to clear the state in "tpc_vote" when
>>> a two phase commit is used (which is now supported by "SQLAlchemy").
>>>
>>> In addition, there may be a problem in case "session.close()" raises
>>> an exception. Then, "_finish" would not be called.
>>
>> This may not be connected, but I just wanted to point out a problem with
>> configurations with "autoflush=False". If I change some database object
>> attributes in the session but do not call session.flush(), the data is not
>> stored into the database, as no commit() is called, which would then flush
>> out/commit the data.
>>
>> So perhaps this is something to think about, or, at least something to be
>> denoted in the documentation.
>>
>> I circumvented the problem by setting autoflush=True and modifying my code so
>> that it works (although it's quite complicated).
>
> I don't think this is related to the above problem, but it may be a
> bug in SQLAlchemy 0.4 whose behaviour I have replicated.
>
> In 0.4 orm.session during a commit flush is called:
>
> 245             if self.autoflush:
> 246                 self.session.flush()
>
> whereas in 0.5 flush is called unconditionally so long as there is not
> a flush in progress.
>
> 358             if not self.session._flushing:
> 359                 self.session.flush()
>
> The documentation for 0.4 appears to be misleading:
>
> commit() serves two purposes; it issues a flush() unconditionally to
> persist any remaining pending changes.
>
>
> I think I should change zope.sqlalchemy to flush during it's tpc_begin
> phase. This should only be done if a flush is not already in progress,
> as otherwise sqlalchemy would raise an error. self.session._flushing
> is only in 0.5 though, so maybe it is time to up the dependency. Or
> maybe it is impossible for it to be in the flushing state at that
> point anyway, as all user code will have completed.

Correction, this is my misunderstanding of sqlalchemy, as explained on irc:
    <stepz> elro: that's when you do session.begin(autoflush=False)

So I should just make it an unconditional flush()

Laurence