Thread: Basic subtransaction facility
Hackers, Here is a very preliminar patch that allows the user to say "BEGIN" inside a transaction and have the system react accordingly. This is only a modification to xact.c (and slightly to other places to allow it to work); the important functions are empty. It compiles fine for me with both SUBTRANSACTIONS defined and not defined; when not defined, the behavior is the same as the current code. Please note that I have made some errors more fatal than they are now, as bugs in this code will have much worse effects than a flaw in the current transaction system. One quick note: there are two ENDABORT states for a subtransaction, SUBENDABORT_OK and SUBENDABORT_ERROR. They signal whether the parent transaction should be aborted after the child transaction finishes or not: an aborted subtransaction where the user issues COMMIT should abort the parent transaction; if the user issues ROLLBACK, the parent can be allowed to continue. Please have a look and comment. This file does not move a lot so I don't think it will suffer from a lot of code drift. -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "I think my standards have lowered enough that now I think 'good design' is when the page doesn't irritate the living f*ck out of me." (JWZ)
Attachment
Do you want this applied? --------------------------------------------------------------------------- Alvaro Herrera wrote: > Hackers, > > Here is a very preliminar patch that allows the user to say "BEGIN" > inside a transaction and have the system react accordingly. This is > only a modification to xact.c (and slightly to other places to allow it > to work); the important functions are empty. > > It compiles fine for me with both SUBTRANSACTIONS defined and not > defined; when not defined, the behavior is the same as the current code. > Please note that I have made some errors more fatal than they are now, > as bugs in this code will have much worse effects than a flaw in the > current transaction system. > > One quick note: there are two ENDABORT states for a subtransaction, > SUBENDABORT_OK and SUBENDABORT_ERROR. They signal whether the parent > transaction should be aborted after the child transaction finishes or > not: an aborted subtransaction where the user issues COMMIT should > abort the parent transaction; if the user issues ROLLBACK, the parent > can be allowed to continue. > > > Please have a look and comment. This file does not move a lot so I > don't think it will suffer from a lot of code drift. > > -- > Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) > "I think my standards have lowered enough that now I think 'good design' > is when the page doesn't irritate the living f*ck out of me." (JWZ) [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 3: if posting/reading through Usenet, please send an appropriate > subscribe-nomail command to majordomo@postgresql.org so that your > message can get through to the mailing list cleanly -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
On Sat, Apr 17, 2004 at 10:03:40AM -0400, Bruce Momjian wrote: > Do you want this applied? If you want. When not #defined, the behavior is the same as the current code, so it shouldn't affect anything. However I posted mainly so people could comment on the modifications, and maybe Heikki Linnakangas could see how it affects his two phase commit patch. Also, that code does not change a lot, so there's little risk of code drift to worry about; this makes it unlikely that I'd have a lot of work to do to update it to a future CVS tip. But maybe applying it means it gets more testing. > --------------------------------------------------------------------------- > > Alvaro Herrera wrote: > > Hackers, > > > > Here is a very preliminar patch that allows the user to say "BEGIN" > > inside a transaction and have the system react accordingly. This is > > only a modification to xact.c (and slightly to other places to allow it > > to work); the important functions are empty. > > > > It compiles fine for me with both SUBTRANSACTIONS defined and not > > defined; when not defined, the behavior is the same as the current code. > > Please note that I have made some errors more fatal than they are now, > > as bugs in this code will have much worse effects than a flaw in the > > current transaction system. > > > > One quick note: there are two ENDABORT states for a subtransaction, > > SUBENDABORT_OK and SUBENDABORT_ERROR. They signal whether the parent > > transaction should be aborted after the child transaction finishes or > > not: an aborted subtransaction where the user issues COMMIT should > > abort the parent transaction; if the user issues ROLLBACK, the parent > > can be allowed to continue. > > > > > > Please have a look and comment. This file does not move a lot so I > > don't think it will suffer from a lot of code drift. -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "At least to kernel hackers, who really are human, despite occasional rumors to the contrary" (LWN.net)
Alvaro Herrera <alvherre@dcc.uchile.cl> writes:
> If you want. When not #defined, the behavior is the same as the current
> code, so it shouldn't affect anything. However I posted mainly so
> people could comment on the modifications, and maybe Heikki Linnakangas
> could see how it affects his two phase commit patch.
I have not reviewed it yet, but would like to do so before it goes in.
regards, tom lane
Tom Lane wrote: > Alvaro Herrera <alvherre@dcc.uchile.cl> writes: > > If you want. When not #defined, the behavior is the same as the current > > code, so it shouldn't affect anything. However I posted mainly so > > people could comment on the modifications, and maybe Heikki Linnakangas > > could see how it affects his two phase commit patch. > > I have not reviewed it yet, but would like to do so before it goes in. OK, thanks. Yea, I think we should get it in rather than waiting for Alvaro to finish the whole project. This way, we have something in that can't drift. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Added to queue until Tom's review and/or application.
Your patch has been added to the PostgreSQL unapplied patches list at:
http://momjian.postgresql.org/cgi-bin/pgpatches
I will try to apply it within the next 48 hours.
---------------------------------------------------------------------------
Alvaro Herrera wrote:
> Hackers,
>
> Here is a very preliminar patch that allows the user to say "BEGIN"
> inside a transaction and have the system react accordingly. This is
> only a modification to xact.c (and slightly to other places to allow it
> to work); the important functions are empty.
>
> It compiles fine for me with both SUBTRANSACTIONS defined and not
> defined; when not defined, the behavior is the same as the current code.
> Please note that I have made some errors more fatal than they are now,
> as bugs in this code will have much worse effects than a flaw in the
> current transaction system.
>
> One quick note: there are two ENDABORT states for a subtransaction,
> SUBENDABORT_OK and SUBENDABORT_ERROR. They signal whether the parent
> transaction should be aborted after the child transaction finishes or
> not: an aborted subtransaction where the user issues COMMIT should
> abort the parent transaction; if the user issues ROLLBACK, the parent
> can be allowed to continue.
>
>
> Please have a look and comment. This file does not move a lot so I
> don't think it will suffer from a lot of code drift.
>
> --
> Alvaro Herrera (<alvherre[a]dcc.uchile.cl>)
> "I think my standards have lowered enough that now I think 'good design'
> is when the page doesn't irritate the living f*ck out of me." (JWZ)
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo@postgresql.org so that your
> message can get through to the mailing list cleanly
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
On Sun, Apr 18, 2004 at 11:29:05AM -0400, Tom Lane wrote: > Alvaro Herrera <alvherre@dcc.uchile.cl> writes: > > If you want. When not #defined, the behavior is the same as the current > > code, so it shouldn't affect anything. However I posted mainly so > > people could comment on the modifications, and maybe Heikki Linnakangas > > could see how it affects his two phase commit patch. > > I have not reviewed it yet, but would like to do so before it goes in. I noticed that I sent an old version because of a system crash (the *one* time I don't review vi -r differences it bites me ... argh). It has several obvious mistakes. Please do not waste your time reviewing that; I'll submit a corrected version later, which will also contain some more changes. Thanks. -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "Find a bug in a program, and fix it, and the program will work today. Show the program how to find and fix a bug, and the program will work forever" (Oliver Silfridge)
Patch withdrawn by author. --------------------------------------------------------------------------- Alvaro Herrera wrote: > Hackers, > > Here is a very preliminar patch that allows the user to say "BEGIN" > inside a transaction and have the system react accordingly. This is > only a modification to xact.c (and slightly to other places to allow it > to work); the important functions are empty. > > It compiles fine for me with both SUBTRANSACTIONS defined and not > defined; when not defined, the behavior is the same as the current code. > Please note that I have made some errors more fatal than they are now, > as bugs in this code will have much worse effects than a flaw in the > current transaction system. > > One quick note: there are two ENDABORT states for a subtransaction, > SUBENDABORT_OK and SUBENDABORT_ERROR. They signal whether the parent > transaction should be aborted after the child transaction finishes or > not: an aborted subtransaction where the user issues COMMIT should > abort the parent transaction; if the user issues ROLLBACK, the parent > can be allowed to continue. > > > Please have a look and comment. This file does not move a lot so I > don't think it will suffer from a lot of code drift. > > -- > Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) > "I think my standards have lowered enough that now I think 'good design' > is when the page doesn't irritate the living f*ck out of me." (JWZ) [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 3: if posting/reading through Usenet, please send an appropriate > subscribe-nomail command to majordomo@postgresql.org so that your > message can get through to the mailing list cleanly -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
On Mon, Apr 19, 2004 at 11:13:35AM -0400, Alvaro Herrera wrote: > I noticed that I sent an old version because of a system crash (the > *one* time I don't review vi -r differences it bites me ... argh). It > has several obvious mistakes. Please do not waste your time reviewing > that; I'll submit a corrected version later, which will also contain > some more changes. Ok, hopefully this one is better. I'm thinking that I'll to add a new elog level to signal a can't-happen condition within the transaction machinery, which would abort the whole transaction tree (more than ERROR) but would not take the whole backend down (less than FATAL). What should it be called? Do people agree that it's needed? -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "Et put se mouve" (Galileo Galilei)
Attachment
Alvaro Herrera <alvherre@dcc.uchile.cl> writes:
> I'm thinking that I'll to add a new elog level to signal a can't-happen
> condition within the transaction machinery, which would abort the whole
> transaction tree (more than ERROR) but would not take the whole backend
> down (less than FATAL). What should it be called? Do people agree that
> it's needed?
If you think it's just for can't-happen conditions, FATAL (or even Assert)
should cover it.
regards, tom lane
Alvaro, where are we on this patch. I think the suggestion was to throw FATAL rather than add a new error level. Is this ready to be applied? --------------------------------------------------------------------------- Alvaro Herrera wrote: > On Mon, Apr 19, 2004 at 11:13:35AM -0400, Alvaro Herrera wrote: > > > I noticed that I sent an old version because of a system crash (the > > *one* time I don't review vi -r differences it bites me ... argh). It > > has several obvious mistakes. Please do not waste your time reviewing > > that; I'll submit a corrected version later, which will also contain > > some more changes. > > Ok, hopefully this one is better. > > I'm thinking that I'll to add a new elog level to signal a can't-happen > condition within the transaction machinery, which would abort the whole > transaction tree (more than ERROR) but would not take the whole backend > down (less than FATAL). What should it be called? Do people agree that > it's needed? > > -- > Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) > "Et put se mouve" (Galileo Galilei) [ Attachment, skipping... ] -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
I wrote ten seconds ago: > This version does. This patch includes both patches I > posted and a few more changes, and does the following: I mean this one. -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "¿Qué importan los años? Lo que realmente importa es comprobar que a fin de cuentas la mejor edad de la vida es estar vivo" (Mafalda)
Attachment
On Mon, Apr 26, 2004 at 11:30:16PM -0400, Bruce Momjian wrote: > > Alvaro, where are we on this patch. I think the suggestion was to > throw FATAL rather than add a new error level. > > Is this ready to be applied? I forgot to verify if it worked correctly with #undef SUBTRANSACTIONS --- it didn't. This version does. This patch includes both patches I posted and a few more changes, and does the following: - adds subtransaction state knowledge to xact.c - adds subtransaction support to smgr, portals (cursors) and async notifies. - adds a new memory context related to the subxact tree (is reset only on subtrans abort). - corrects a couple of bugs in the previous patches. - mantains a Xid list of committed subxacts, for use in future changes involving pg_clog - adds support for executing BEGIN inside an aborted transaction, not only as a simple query (1st patch did this) but also as messages of v3 protocol and prepared statements. - works cleanly with SUBTRANSACTIONS undefined (you get the current behavior, no BEGIN is allowed inside a running transaction) and defined (all of the above). - keeps the original behavior of using FATAL whenever an bug is found inside xact.c I feel this one is ready to be applied. Tom wanted to review it, of course. Still missing: - deal with prepared statements, deferred triggers - save state in pg_clog - visibility rules -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "La realidad se compone de muchos sueños, todos ellos diferentes, pero en cierto aspecto, parecidos..." (Yo, hablando de sueños eróticos)
[ Tom will review.]
Description from previous patch added to patched queue too.
Your patch has been added to the PostgreSQL unapplied patches list at:
http://momjian.postgresql.org/cgi-bin/pgpatches
I will try to apply it within the next 48 hours.
---------------------------------------------------------------------------
Alvaro Herrera wrote:
> I wrote ten seconds ago:
>
> > This version does. This patch includes both patches I
> > posted and a few more changes, and does the following:
>
> I mean this one.
>
> --
> Alvaro Herrera (<alvherre[a]dcc.uchile.cl>)
> "?Qu? importan los a?os? Lo que realmente importa es comprobar que
> a fin de cuentas la mejor edad de la vida es estar vivo" (Mafalda)
[ Attachment, skipping... ]
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
On Mon, Apr 26, 2004 at 11:30:16PM -0400, Bruce Momjian wrote: > Alvaro, where are we on this patch. I think the suggestion was to > throw FATAL rather than add a new error level. The assumption was that we would only want an additional level for catching can't-happen conditions. ISTM this is not true. Consider an out of memory error: do we want to only rollback the affected subtransaction, or the whole transaction tree? If we want the latter we will have to invent a new elevel. In fact, I think we should mark ERROR as aborting the whole transaction tree, and create a new level which would abort the innermost subtransaction. We would then change whatever is appropiate to the new elevel. Doing otherwise would leave us open to unexpected conditions causing only subtrans abort, which could lead to unreliable behavior. In short, I think all elog(ERROR) should have different behaviour from ereport(ERROR), at least. And I don't think the answer should be elog(FATAL) for the former. -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "Ni aun el genio muy grande llegaría muy lejos si tuviera que sacarlo todo de su propio interior" (Goethe)
Alvaro Herrera wrote: > On Mon, Apr 26, 2004 at 11:30:16PM -0400, Bruce Momjian wrote: > > > Alvaro, where are we on this patch. I think the suggestion was to > > throw FATAL rather than add a new error level. > > The assumption was that we would only want an additional level for > catching can't-happen conditions. ISTM this is not true. Consider an > out of memory error: do we want to only rollback the affected > subtransaction, or the whole transaction tree? If we want the latter we > will have to invent a new elevel. > > In fact, I think we should mark ERROR as aborting the whole transaction > tree, and create a new level which would abort the innermost > subtransaction. We would then change whatever is appropiate to the new > elevel. Doing otherwise would leave us open to unexpected conditions > causing only subtrans abort, which could lead to unreliable behavior. > > In short, I think all elog(ERROR) should have different behaviour from > ereport(ERROR), at least. And I don't think the answer should be > elog(FATAL) for the former. Agreed we need a new error code to abort a subtransaction rather than the entire transaction. I don't understand your elog(ERROR) vs. ereport(ERROR) distinction. Was that a typo? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
On Wed, 28 Apr 2004 12:02:44 -0400, Alvaro Herrera <alvherre@dcc.uchile.cl> wrote: >In fact, I think we should mark ERROR as aborting the whole transaction >tree, and create a new level which would abort the innermost >subtransaction. We would then change whatever is appropiate to the new >elevel. Doing otherwise would leave us open to unexpected conditions >causing only subtrans abort, which could lead to unreliable behavior. Why? Subtransaction commit propagates an error state to the parent transaction. And if a subtransaction is rolled back the parent can continue cleanly no matter what was the reason for the subtrans abort. Servus Manfred
Manfred Koizar wrote: > On Wed, 28 Apr 2004 12:02:44 -0400, Alvaro Herrera > <alvherre@dcc.uchile.cl> wrote: > >In fact, I think we should mark ERROR as aborting the whole transaction > >tree, and create a new level which would abort the innermost > >subtransaction. We would then change whatever is appropiate to the new > >elevel. Doing otherwise would leave us open to unexpected conditions > >causing only subtrans abort, which could lead to unreliable behavior. > > Why? Subtransaction commit propagates an error state to the parent > transaction. And if a subtransaction is rolled back the parent can > continue cleanly no matter what was the reason for the subtrans abort. I think his point was that there are some errors that should abort the outer transaction too. I think Alvaro mentioned out of memory, but that is a FATAL error. Alvaro, what error were you thinking of that should abort the outer transaction? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Bruce Momjian wrote: > I think his point was that there are some errors that should abort > the outer transaction too. I think Alvaro mentioned out of memory, > but that is a FATAL error. Alvaro, what error were you thinking of > that should abort the outer transaction? Theoretically, if you abort the inner transaction, that could free up memory for use by the outer transaction.
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Manfred Koizar wrote:
>> Why? Subtransaction commit propagates an error state to the parent
>> transaction. And if a subtransaction is rolled back the parent can
>> continue cleanly no matter what was the reason for the subtrans abort.
> I think his point was that there are some errors that should abort the
> outer transaction too. I think Alvaro mentioned out of memory, but that
> is a FATAL error.
Nonsense. In the first place, out-of-memory hasn't been FATAL for
years. In the second, there is no reason to think that we can't
continue the outer transaction(s), as aborting the innermost one is
likely to free quite a lot of memory. (And if it doesn't, well, the
outer one will get its own out-of-memory ERROR soon enough.)
In general I tend to agree with Manfred's point: if you have reason to
suspect global corruption of a backend's state then you should do FATAL
(or possibly PANIC). If you do not suspect this then you ought to just
ERROR. I do not see the use-case for abort-all-levels-of-xact-but-
don't-exit.
regards, tom lane
On Thu, Apr 29, 2004 at 12:26:01AM -0400, Bruce Momjian wrote: > I don't understand your elog(ERROR) vs. ereport(ERROR) distinction. Was > that a typo? Nope. When Tom upgraded the error handling, he changed almost everything to ereport(), but in the places where there's a violation of expected conditions, he retained elog(). We don't provide special error code, nor there is space for errhints etc. Those unexpected conditions I thought we could just abort the transaction tree; but maybe we have to close the backend as Manfred and Tom say. I don't think there's space for PANIC though (unless we suspect shared state corruption ... is that checked for anywhere? I haven't looked.) -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "No single strategy is always right (Unless the boss says so)" (Larry Wall)
On Thu, Apr 29, 2004 at 02:42:23PM -0400, Tom Lane wrote: > In general I tend to agree with Manfred's point: if you have reason to > suspect global corruption of a backend's state then you should do FATAL > (or possibly PANIC). If you do not suspect this then you ought to just > ERROR. I do not see the use-case for abort-all-levels-of-xact-but- > don't-exit. Ok, I'm not wedded to the idea of a new elevel. So you think elog(ERROR) should rather be elog(FATAL) ? -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) Y una voz del caos me habló y me dijo "Sonríe y sé feliz, podría ser peor". Y sonreí. Y fui feliz. Y fue peor.
On Thu, Apr 29, 2004 at 07:29:07PM +0200, Peter Eisentraut wrote: > Bruce Momjian wrote: > > I think his point was that there are some errors that should abort > > the outer transaction too. I think Alvaro mentioned out of memory, > > but that is a FATAL error. Alvaro, what error were you thinking of > > that should abort the outer transaction? > > Theoretically, if you abort the inner transaction, that could free up > memory for use by the outer transaction. Yes, this is planned to happen. -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "La tristeza es un muro entre dos jardines" (Khalil Gibran)
On Thu, Apr 29, 2004 at 06:42:31PM +0200, Manfred Koizar wrote: > On Wed, 28 Apr 2004 12:02:44 -0400, Alvaro Herrera > <alvherre@dcc.uchile.cl> wrote: > >In fact, I think we should mark ERROR as aborting the whole transaction > >tree, and create a new level which would abort the innermost > >subtransaction. We would then change whatever is appropiate to the new > >elevel. Doing otherwise would leave us open to unexpected conditions > >causing only subtrans abort, which could lead to unreliable behavior. > > Why? Subtransaction commit propagates an error state to the parent > transaction. And if a subtransaction is rolled back the parent can > continue cleanly no matter what was the reason for the subtrans abort. Not necessarily; consider can't-happen conditions, like everything that is marked elog(ERROR) rather than ereport(ERROR). Corrupt hashes, should-exist catalog entries that are not there, etc. They should not be frequent, be we should be prepared for them. -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "La virtud es el justo medio entre dos defectos" (Aristóteles)