Basic subtransaction facility

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)

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

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)

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)

[ 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