Nested database transactions

classic Classic list List threaded Threaded
19 messages Options
Reply | Threaded
Open this post in threaded view
|

Nested database transactions

Daniel Kinzler
Hi all

I think it would be extremely useful to allow nested database transactions - or
simulate them using a counter that would only to the actual commit after
commit() has been called as many times as begin() was called before.

This actually used to be the case, according to the comment on Database::trxLevel:

         * Historically, transactions were allowed to be "nested". This is no
         * longer supported, so this function really only returns a boolean.

This means that currently, if you call begin() while a transaction is already in
progress, the previous transaction is inadvertently committed, possibly causing
inconsistencies (at least on MySQL).

Why was this feature removed? Not counting transaction levels is causing a world
of pain for us on the Wikidata project, and I'm sure the same problem arises
elsewhere. Here's the problem:

* Before saving a change using WikiPage::doEdit(), i want to perform some checks
on the database, enforcing some global consistency constraints.
* The check should be in the same transaction, so the DB can't change after the
check but before the save.
* I can't open a transaction before my check, because WikiPage::doEdit()already
opens a transaction which would in turn abort mine, causing the save to be
performed in a separate transaction after all.
* I could try to inject my check into WikiPage::doEdit() using some hook. That
may work but it cumbersome and annoying, and I have to hope that the hook is
never moved outside the transaction (hooks generally don't guarantee whther they
are called in a transaction or not).


Basically, any code that needs transactional logic needs to first check whether
a transaction is already ongoing, open a transaction if not, remember whether it
owns the transaction, and commit the transaction in the end only if it's owned
locally. This essentially implements the is-in-transaction-counter based on the
call stack.

Looking at the code, this kind of check is hardly ever done. So the code just
*assumes* that it is, or is not, called within a transaction. This is bad.

So... why was the nice and simple trxLevel counting removed? What would break if
we put it back? Is there some other magic method to do this safely and nicely?


Thanks,
Daniel


PS: btw, if the non-counting behavior is to be kept, Database::begin() should
really fail if a transaction is already in progress. Silently committing the
previous transaction is very likely to cause creeping, low volume and hard to
track down database corruption.

_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Nested database transactions

Brion Vibber
On Thu, Aug 23, 2012 at 1:37 PM, Daniel Kinzler <[hidden email]>wrote:

> I think it would be extremely useful to allow nested database transactions
> - or
> simulate them using a counter that would only to the actual commit after
> commit() has been called as many times as begin() was called before.
>
> This actually used to be the case, according to the comment on
> Database::trxLevel:
>
>          * Historically, transactions were allowed to be "nested". This is
> no
>          * longer supported, so this function really only returns a
> boolean.
>
> This means that currently, if you call begin() while a transaction is
> already in
> progress, the previous transaction is inadvertently committed, possibly
> causing
> inconsistencies (at least on MySQL).
>
> Why was this feature removed? Not counting transaction levels is causing a
> world
> of pain for us on the Wikidata project, and I'm sure the same problem
> arises
> elsewhere.


Well, the main reason is probably that MySQL doesn't support nested
transactions... trying to simulate them with a counter sounds fragile, as a
single rollback would roll back the entire transaction "tree", not just the
last nested one you started (or else do nothing if you just decrement the
counter, also possibly dangerous if you expected the rollback to work).

-- brion
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Nested database transactions

Evan Priestley
We solve this in Phabricator by using BEGIN (depth 0) or SAVEPOINT (depth 1+) when incrementing the counter, ROLLBACK TO SAVEPOINT (depth 1+) or ROLLBACK (depth 0) when decrementing it after a failure, and nothing (depth 1) or COMMIT (depth 0) when decrementing it after a success. Our experience with transaction stacks has generally been good (no real surprises, doesn't feel magical, significantly reduces the complexity of transactional code), although we don't support anything but MySQL.

On Aug 23, 2012, at 1:49 PM, Brion Vibber wrote:

> On Thu, Aug 23, 2012 at 1:37 PM, Daniel Kinzler <[hidden email]>wrote:
>
>> I think it would be extremely useful to allow nested database transactions
>> - or
>> simulate them using a counter that would only to the actual commit after
>> commit() has been called as many times as begin() was called before.
>>
>> This actually used to be the case, according to the comment on
>> Database::trxLevel:
>>
>>         * Historically, transactions were allowed to be "nested". This is
>> no
>>         * longer supported, so this function really only returns a
>> boolean.
>>
>> This means that currently, if you call begin() while a transaction is
>> already in
>> progress, the previous transaction is inadvertently committed, possibly
>> causing
>> inconsistencies (at least on MySQL).
>>
>> Why was this feature removed? Not counting transaction levels is causing a
>> world
>> of pain for us on the Wikidata project, and I'm sure the same problem
>> arises
>> elsewhere.
>
>
> Well, the main reason is probably that MySQL doesn't support nested
> transactions... trying to simulate them with a counter sounds fragile, as a
> single rollback would roll back the entire transaction "tree", not just the
> last nested one you started (or else do nothing if you just decrement the
> counter, also possibly dangerous if you expected the rollback to work).
>
> -- brion
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Nested database transactions

Brion Vibber
On Thu, Aug 23, 2012 at 2:02 PM, Evan Priestley <[hidden email]>wrote:

> We solve this in Phabricator by using BEGIN (depth 0) or SAVEPOINT (depth
> 1+) when incrementing the counter, ROLLBACK TO SAVEPOINT (depth 1+) or
> ROLLBACK (depth 0) when decrementing it after a failure, and nothing (depth
> 1) or COMMIT (depth 0) when decrementing it after a success. Our experience
> with transaction stacks has generally been good (no real surprises, doesn't
> feel magical, significantly reduces the complexity of transactional code),
> although we don't support anything but MySQL.
>

Oooh, nice! Hadn't come across SAVEPOINT before.

http://dev.mysql.com/doc/refman/5.0/en/savepoint.html

-- brion
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Nested database transactions

Tyler Romeo
Also, as a matter of record, I just checked and the SAVEPOINT command (or
an equivalent) is supported on SQLite, Postgresql, and mssql.

*--*
*Tyler Romeo*
Stevens Institute of Technology, Class of 2015
Major in Computer Science
www.whizkidztech.com | [hidden email]



On Thu, Aug 23, 2012 at 5:21 PM, Brion Vibber <[hidden email]> wrote:

> On Thu, Aug 23, 2012 at 2:02 PM, Evan Priestley <[hidden email]
> >wrote:
>
> > We solve this in Phabricator by using BEGIN (depth 0) or SAVEPOINT (depth
> > 1+) when incrementing the counter, ROLLBACK TO SAVEPOINT (depth 1+) or
> > ROLLBACK (depth 0) when decrementing it after a failure, and nothing
> (depth
> > 1) or COMMIT (depth 0) when decrementing it after a success. Our
> experience
> > with transaction stacks has generally been good (no real surprises,
> doesn't
> > feel magical, significantly reduces the complexity of transactional
> code),
> > although we don't support anything but MySQL.
> >
>
> Oooh, nice! Hadn't come across SAVEPOINT before.
>
> http://dev.mysql.com/doc/refman/5.0/en/savepoint.html
>
> -- brion
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Nested database transactions

Platonides
In reply to this post by Brion Vibber
On 23/08/12 23:21, Brion Vibber wrote:

> On Thu, Aug 23, 2012 at 2:02 PM, Evan Priestley <[hidden email]>wrote:
>
>> We solve this in Phabricator by using BEGIN (depth 0) or SAVEPOINT (depth
>> 1+) when incrementing the counter, ROLLBACK TO SAVEPOINT (depth 1+) or
>> ROLLBACK (depth 0) when decrementing it after a failure, and nothing (depth
>> 1) or COMMIT (depth 0) when decrementing it after a success. Our experience
>> with transaction stacks has generally been good (no real surprises, doesn't
>> feel magical, significantly reduces the complexity of transactional code),
>> although we don't support anything but MySQL.
>
> Oooh, nice! Hadn't come across SAVEPOINT before.
>
> http://dev.mysql.com/doc/refman/5.0/en/savepoint.html
>
> -- brion

I proposed this same thing on the mailing list two years ago:
 http://thread.gmane.org/gmane.science.linguistics.wikipedia.technical/49500

Yes, I completely support changing the transactions to be nested using
savepoints. They even are supported (or were) on all our db backends.

However, I was told that it "might make us hold a lot of locks for much
too long". So with fear to cause magical db overload, nothing was
changed. :(


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Nested database transactions

Jeroen De Dauw-2
Hey,

One concern I have with big transcations that have lots of stuff in them is
that some code might get called in which needs to run in a transaction with
a higher isolation level then the current one. For MySQLs InnoDB the
default is repeatable read, so if you have code that requires
serializability and gets called during that transaction, you're basically
fucked. Or am I missing something?

Cheers

--
Jeroen De Dauw
http://www.bn2vs.com
Don't panic. Don't be evil.
--
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Nested database transactions

Platonides
On 23/08/12 23:54, Jeroen De Dauw wrote:

> Hey,
>
> One concern I have with big transcations that have lots of stuff in them is
> that some code might get called in which needs to run in a transaction with
> a higher isolation level then the current one. For MySQLs InnoDB the
> default is repeatable read, so if you have code that requires
> serializability and gets called during that transaction, you're basically
> fucked. Or am I missing something?
>
> Cheers

I don't think we have any code requiring a different transaction isolation.


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Nested database transactions

b-jorsch
In reply to this post by Platonides
On Thu, Aug 23, 2012 at 02:02:41PM -0700, Evan Priestley wrote:
> We solve this in Phabricator by using BEGIN (depth 0) or SAVEPOINT
> (depth 1+) when incrementing the counter, ROLLBACK TO SAVEPOINT (depth
> 1+) or ROLLBACK (depth 0) when decrementing it after a failure, and
> nothing (depth 1) or COMMIT (depth 0) when decrementing it after a
> success. Our experience with transaction stacks has generally been
> good (no real surprises, doesn't feel magical, significantly reduces
> the complexity of transactional code), although we don't support
> anything but MySQL.

We do the same thing in our PostgreSQL-based app at my day job, although
for commit at depth > 0 we use RELEASE SAVEPOINT rather than doing
nothing. I don't think it makes much difference, though, beyond allowing
for the release of resources related to the savepoint itself.

FWIW, our savepoints are simply named along the lines of
"savepoint$depth". It's been working for us without issue for years.


On Thu, Aug 23, 2012 at 05:24:25PM -0400, Tyler Romeo wrote:
> Also, as a matter of record, I just checked and the SAVEPOINT command (or
> an equivalent) is supported on SQLite, Postgresql, and mssql.

According to the PostgreSQL documentation (which is usually pretty good
about this sort of thing), it's standard SQL. So any sufficiently-new
(and sufficiently-good) SQL database should support it.


On Thu, Aug 23, 2012 at 11:30:20PM +0200, Platonides wrote:
>
> However, I was told that it "might make us hold a lot of locks for much
> too long". So with fear to cause magical db overload, nothing was
> changed. :(

:(

Although it seems to me that avoiding that problem by making people have
to know whether the function they're calling is "safe" to call within a
transaction or not isn't the best idea.

_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Nested database transactions

Jeroen De Dauw-2
In reply to this post by Platonides
I'm not saying we have any such code, I'm asking what to do when one wants
to introduce such code. It's entirely feasible some new functionality
requires serializable transactions, so we might want to keep that into
consideration.

Sent from my Android phone.
On 24 Aug 2012 00:30, "Platonides" <[hidden email]> wrote:

> On 23/08/12 23:54, Jeroen De Dauw wrote:
> > Hey,
> >
> > One concern I have with big transcations that have lots of stuff in them
> is
> > that some code might get called in which needs to run in a transaction
> with
> > a higher isolation level then the current one. For MySQLs InnoDB the
> > default is repeatable read, so if you have code that requires
> > serializability and gets called during that transaction, you're basically
> > fucked. Or am I missing something?
> >
> > Cheers
>
> I don't think we have any code requiring a different transaction isolation.
>
>
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Nested database transactions

Aaron Schulz
In reply to this post by Daniel Kinzler
The counter idea kind of reminds of what I have in https://gerrit.wikimedia.org/r/#/c/16696/ .

I think the whole implicit commit issue is definitely pretty annoying and I wish there was a reasonable way to address it without reasonable backwards compatibility. rollback() is the hard case to deal with (I ended up not even having it in that gerrit patch).

In general callers should avoid using rollback() for detecting problems or race conditions. They should be checked up front. I put some comments about this in the tiny IDBAccessObject interface a while ago. This avoids complexity with "what if someone rollback". It also avoid mysql undo segment usage (though rollback is faster in PG).

SAVEPOINTs are useful if we really need to support people rollback transactions *and* we need nested transaction support. I think they could be made to work, but I'm not sold on their necessity for any use cases we have.
Reply | Threaded
Open this post in threaded view
|

Re: Nested database transactions

Freako F. Freakolowsky
In reply to this post by Jeroen De Dauw-2
I was also thinking about introducing a similar functionality, not
exactly nested transaction but just a way to prevent $db->commit and
$db->rollback from doing enything.
Akshay(my GSoC) wanted to have his extension code transaction safe, but
there was just no way of doing it without tinkering with the core. So
it's true in core there is (as it should be) no actual need for
transaction nesting, but there are extensions that could use some tx safety.

The simplest way i can think of how this could be done is changing
$db->mTrxLevel from a flag to a counter and start using savepoints
(doable in Mysql, PG and Oracle, don't know for other DBs). So only the
first mTrxLevel would do actual commit or blank rollback (maybe add an
optional parameter to calls to override), on higher levels you would
just skip on commit and rollback to savepoint on rollback call.

LP, Jure



On 08/24/2012 01:13 AM, Jeroen De Dauw wrote:

> I'm not saying we have any such code, I'm asking what to do when one wants
> to introduce such code. It's entirely feasible some new functionality
> requires serializable transactions, so we might want to keep that into
> consideration.
>
> Sent from my Android phone.
> On 24 Aug 2012 00:30, "Platonides" <[hidden email]> wrote:
>
>> On 23/08/12 23:54, Jeroen De Dauw wrote:
>>> Hey,
>>>
>>> One concern I have with big transcations that have lots of stuff in them
>> is
>>> that some code might get called in which needs to run in a transaction
>> with
>>> a higher isolation level then the current one. For MySQLs InnoDB the
>>> default is repeatable read, so if you have code that requires
>>> serializability and gets called during that transaction, you're basically
>>> fucked. Or am I missing something?
>>>
>>> Cheers
>> I don't think we have any code requiring a different transaction isolation.
>>
>>
>> _______________________________________________
>> Wikitech-l mailing list
>> [hidden email]
>> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>>
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Nested database transactions

Daniel Kinzler
In reply to this post by Daniel Kinzler
On 23.08.2012 22:49, Brion Vibber wrote:
> Well, the main reason is probably that MySQL doesn't support nested
> transactions... trying to simulate them with a counter sounds fragile, as a
> single rollback would roll back the entire transaction "tree", not just the last
> nested one you started (or else do nothing if you just decrement the counter,
> also possibly dangerous if you expected the rollback to work).

To me it seems correct and safe to assume that a ROLLBACK will cause all open
transactions to fail. I don't see any problem with handling things this way. Am
I missing something? Was there any *concrete* problem that caused this feature
to be removed?

On 23.08.2012 23:21, Brion Vibber wrote:

> On Thu, Aug 23, 2012 at 2:02 PM, Evan Priestley <[hidden email]>wrote:
>
>> We solve this in Phabricator by using BEGIN (depth 0) or SAVEPOINT (depth
>> 1+) when incrementing the counter, ROLLBACK TO SAVEPOINT (depth 1+) or
>> ROLLBACK (depth 0) when decrementing it after a failure, and nothing (depth
>> 1) or COMMIT (depth 0) when decrementing it after a success. Our experience
>> with transaction stacks has generally been good (no real surprises, doesn't
>> feel magical, significantly reduces the complexity of transactional code),
>> although we don't support anything but MySQL.
>>
>
> Oooh, nice! Hadn't come across SAVEPOINT before.
>
> http://dev.mysql.com/doc/refman/5.0/en/savepoint.html

Hm... that'S a 404 for me. For some reason, this is missing in the 5.0 manual,
even though it exists in 4.1 and 5.1:

http://dev.mysql.com/doc/refman/5.1/en/savepoint.html

Anyway, this seems like a neat solution if it is handled automatically by
begin(), rollback() and commit(), so the calling code doesn't have to be aware
of the current transaction level.

I'm tempted to implement this. Any objections?

-- daniel

PS:



--
Daniel Kinzler, Softwarearchitekt

Wikimedia Deutschland e.V. | Eisenacher Straße 2 | 10777 Berlin
http://wikimedia.de  | Tel. (030) 219 158 260

Wikimedia Deutschland - Gesellschaft zur Förderung Freien Wissens e.V.
Eingetragen im Vereinsregister des Amtsgerichts Berlin-Charlottenburg
unter der Nummer 23855 B. Als gemeinnützig anerkannt durch das Finanzamt
für Körperschaften I Berlin, Steuernummer 27/681/51985.

_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Nested database transactions

Daniel Kinzler
In reply to this post by Aaron Schulz
On 24.08.2012 03:14, Aaron Schulz wrote:
> SAVEPOINTs are useful if we really need to support people rollback
> transactions *and* we need nested transaction support. I think they could be
> made to work, but I'm not sold on their necessity for any use cases we have.

So, how would you solve the use case I described? What I need to do is to
perform some checks before calling WikiPage::doEdit, and make sure the result of
the check is still valid when the actual save occurs.

I can't see a clean way to do this without supporting nested transactions in
*some* way.

-- daniel


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Nested database transactions

Tyler Romeo
> So, how would you solve the use case I described? What I need to do is to
> perform some checks before calling WikiPage::doEdit, and make sure the
result of
> the check is still valid when the actual save occurs.

SAVEPOINTs are basically nested transactions. Can you describe the use case
in more detail?

*--*
*Tyler Romeo*
Stevens Institute of Technology, Class of 2015
Major in Computer Science
www.whizkidztech.com | [hidden email]



On Fri, Aug 24, 2012 at 12:36 PM, Daniel Kinzler <[hidden email]>wrote:

> On 24.08.2012 03:14, Aaron Schulz wrote:
> > SAVEPOINTs are useful if we really need to support people rollback
> > transactions *and* we need nested transaction support. I think they
> could be
> > made to work, but I'm not sold on their necessity for any use cases we
> have.
>
> So, how would you solve the use case I described? What I need to do is to
> perform some checks before calling WikiPage::doEdit, and make sure the
> result of
> the check is still valid when the actual save occurs.
>
> I can't see a clean way to do this without supporting nested transactions
> in
> *some* way.
>
> -- daniel
>
>
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Nested database transactions

Daniel Kinzler
On 24.08.2012 18:55, Tyler Romeo wrote:
>> So, how would you solve the use case I described? What I need to do is to
>> perform some checks before calling WikiPage::doEdit, and make sure the
> result of
>> the check is still valid when the actual save occurs.
>
> SAVEPOINTs are basically nested transactions.

Yes. I'd like to use them.

MediaWiki's current behaviour is calling begin() when a transaction is open
silently commits the old transaction and starts a new one.

This SUCKS.

> Can you describe the use case
> in more detail?

So, in wikidata, we have global constraints, e.g. the requirement that only one
data item can have a sitelink to a given wikipedage (there's a 1:1 relationship
between wikipedia pages and data items). Before saving the item page, this
constraint needs to be checked, just before calling WikiPage::doEdit(). And we
also want to check for edit conflicts (comparing the base revision - note that
we are not using EditPage).

Anyway, wee need to do some checks before we call WikiPage::doEdit. And make
sure the database doesn't change before the actual save is done. So our checks
should be in the same transaction as the actual save.

But WikiPage::doEdit already opens a transaction. So we can no open a
surrounding transactiopn bracket - because nested transactions are not supported.

This could be solved be the "counting" or the "safepoint" solution, the latter
being somewhat nicer. But we need to st least *one* of them, as far as I can tell.

The current situation is that code always has to know whether it is safe to call
some function X from inside a transaction, and conversely, any function needs to
decide on whether it expects to be called from within an existing transaction,
or if it should open its own.

These things can often not really be known in advance. This has caused trouble
in the past (caused by transactions being committed prematurely, because another
transaction started). I'm sure it will cause more pain in the future.

So I'm proposing to implement support for nested transactions, either by just
counting (and, on rollback, roll back all open transactions). Or by using
savepoints.

-- daniel


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Nested database transactions

Aaron Schulz
In reply to this post by Daniel Kinzler
I'd have to see what you are doing to see if rollback is really needed.
Reply | Threaded
Open this post in threaded view
|

Re: Nested database transactions

Daniel Kinzler
On 28.08.2012 06:16, Aaron Schulz wrote:
> I'd have to see what you are doing to see if rollback is really needed.

As far as I see, nested transactions are needed, but no rollback. at least not
from our side.

So, for that case, a simple counter would be sufficient. I still wonder why that
feature got removed from the code.

-- daniel


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Nested database transactions

Freako F. Freakolowsky
May i barge into this discussion a bit.... and please, feel free to shut
me down if i'm missing a point here.

I'm failing to see why all this debate. We already have transactions, we
have begin, commit and rollback in the abstraction (Database.php lines
2830+). And this works. All that needs to be done is extending this
functonality without breaking the existing MO which maintains a single
transaction level.

Daniel, you have the same issue as Akshay has in his extension, which is
the fact that doEdit closes a transation before you complete the whole
editing process. So you don't need nested transactions as such ... just
a way to prevent core from commiting if your extension spans over
multiple core transactions and taking over the commiting/rollingback by
yourself ... basically what you need is this (not sure about PG
compatibility here):



public function begin( $fname = 'DatabaseBase::begin', $maskTransaction ) {
        if ( $this->mTrxLevel < 2 ) {
                $this->query( 'BEGIN', $fname );
                $this->mTrxLevel = 1;
                // this is here just to make sure that begin works the same way as now when on txLevel = 1
                if ( $maskTransaction ) {
                        $this->mTrxLevel++;
                }
         } else {
                $this->mTrxLevel++;
                $this->query( 'SAVEPOINT SP'.$this->mTrxLevel, $fname );
        }

}

public function commit( $fname = 'DatabaseBase::commit', $doGlobally = false ) {
        if ( $this->mTrxLevel == 1 || $this->mTrxLevel == 2 || $doGlobally ) {
                $this->query( 'COMMIT', $fname );
                $this->mTrxLevel = 0;
        } else {
                $this->mTrxLevel--;
        }
}

public function rollback( $fname = 'DatabaseBase::rollback', $doGlobally = false ) {
        if ( $this->mTrxLevel == 1|| $this->mTrxLevel == 2 || $doGlobally ) {
                $this->query( 'ROLLBACK', $fname, true );
                $this->mTrxLevel = 0;
        } elseif ( $this->txLevel > 0 ) {
                $this->query( 'ROLLBACK TO SAVEPOINT SP'.$this->mTrxLevel, $fname, true );
                $this->mTrxLevel--;
        }
}



This should not change anything in core, and an extension gets a way to
block core's commits and restarts of an already open transaction. I've
added the savepoints-foo, just to have a complete solution.

So what's so drastic about such change? Why all this debate?

On 08/28/2012 09:16 AM, Daniel Kinzler wrote:

> On 28.08.2012 06:16, Aaron Schulz wrote:
>> I'd have to see what you are doing to see if rollback is really needed.
> As far as I see, nested transactions are needed, but no rollback. at least not
> from our side.
>
> So, for that case, a simple counter would be sufficient. I still wonder why that
> feature got removed from the code.
>
> -- daniel
>
>
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>
>


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l