Re: [PATCHES] Static snapshot data

From: Alvaro Herrera <alvherre(at)dcc(dot)uchile(dot)cl>
To: Manfred Koizar <mkoi-pg(at)aon(dot)at>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCHES] Static snapshot data
Date: 2003-05-15 23:39:55
Message-ID: 20030515233955.GB4030@dcc.uchile.cl
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

On Tue, May 13, 2003 at 09:57:39PM +0200, Manfred Koizar wrote:
> On Mon, 12 May 2003 23:55:31 -0400, Alvaro Herrera
> <alvherre(at)dcc(dot)uchile(dot)cl> wrote:
> >On Mon, May 12, 2003 at 09:40:37AM -0400, Tom Lane wrote:

> Tom replied:
> |I have a feeling that there might be some value in running a
> |SERIALIZABLE subtransaction inside a READ COMMITTED parent.

After digesting Manfred's ideas, I agree that there's no need for
parentxact or childxact arrays in the Snapshots. Also I agree that
there's no need for a QuerySnapshot stack -- there's always one, no
matter how deep you are in the transaction tree. It also doesn't matter
if you are in a SERIALIZABLE or READ COMMITTED transaction: if in READ
COMMITTED, the QuerySnapshot is always calculated afresh for each query,
and if in SERIALIZABLE, the QuerySnapshot is always equal to the
SerializableSnapshot.

When a SERIALIZABLE subtransaction is started in a READ COMMITTED
parent, the SerializableSnapshot is just calculated again. There's no
need to keep the old SerializableSnapshot, because it's useless.

> Rule 1) Subtransactions can be nested to arbitrary levels.
> Rule 2) On subtransaction ROLLBACK, changes done by the
> subtransaction are effectively undone.
> Rule 3) After subtransaction COMMIT, changes done by the
> subtransaction are effectively treated as if done by the enclosing
> transaction.

Right.

> I'm inclined to include "SET TRANSACTION ISOLATION LEVEL" in the kind
> of changes that rules 2 and 3 deal with. Perhaps the rules should say
> "commands executed" instead of "changes done". This would forbid
> running parts of the same main transaction with different isolation
> levels.

Yeah, the TRANSACTION ISOLATION LEVEL should be part of the current
transaction state, but it is inherited to subtransactions. The user can
change from READ COMMITTED to SERIALIZABLE when starting a
subtransaction, but not the other way around. (Note that it _is_
possible to change from SERIALIZABLE to READ COMMITTED in the topmost
transaction).

It's also not possible to start a SERIALIZABLE transaction with a
different SerializableSnapshot inside a SERIALIZABLE parent. It would
violate the rules of serializability, as it means changing the snapshot
in the middle of the (parent) SERIALIZABLE.

> GetSnapshotData does not care about subtransactions.

Right.

> :In the current implementation, it's sufficient to know
> :a) what transactions come before me (Xmin),
> :b) what transactions come after me (Xmax),
> :c) what transactions are in progress (xip), and
> :d) what commands come before me in the current transaction
> : (curcid)
>
> I propose that we don't change this, except that d) should say "... in
> the current transaction tree"

There's no need for this. Starting a transaction should be a new
CommandId for the parent transaction, so the tuples written by a
transaction that's not me but belong to my transaction tree are
effectively treated as if they were from a previous CommandId.

It should only be a matter of incrementing CommandId in the parent
transaction just before starting the subtransaction, or just after
ending it. It is critical to do so, or we risk considering changes
neighbouring the subxact with the same CommandId, which would be bogus.

> ad e) I can't see a need to directly answer this question. What we
> need is e') Does a given xid belong to the current xact tree?
> This can be answered using pg_subtrans and the transaction information
> stack (see below).
> ad f) I'd write this as:
> f') What commands of my transaction tree come before me?

I agree.

> It might help, if we continue to increment cid across subtransaction
> boundaries.

We don't need to, because

> This will be handled by HeapTupleSatisfiesXxxx using pg_subtrans:
>
> . We find a tuple (having p=2) with xmin=3.
>
> . In pg_clog we find that xact 3 is a committed subtransaction.
>
> . We lookup xact 3's parent transaction in pg_subtrans and get
> parent xact = 1.
>
> . Consulting the transaction information stack we find out that
> xact 1 is one of our own currently active transactions (in this
> case the only one).
>
> . Because the tuple's cmin (4) is less than CurrentCommandId (6)
> the tuple is visible.

This last rule should be replaced by:

. Because the tuple's xmin is not my XID, the tuple is visible.

We need to check the CurrentCommandId only if xmin (or xmax) is my own
XID.

> :Both cases are not implementable with the current notion of a Snapshot.
>
> I think they are. What we need is not an extension to the snapshot
> structure, but a transaction information stack holding transaction
> specific information: TransactionId, TransState, TBlockState, ...
>
> This looks almost like struct TransactionStateData, except that
> commandId, startTime, and startTimeUsec belong only to the main
> transaction.

Hm, why do you want to left out the startTime and startTimeUsec? Maybe
it's useful to know the start time of the current transaction, instead
of the start time of the current transaction tree.

Also the CommandId belongs to each transaction and not only the topmost.
This way we are not limited to 2^32 commands per transaction tree, but
to 2^32 commands per transaction. No, I don't really think anyone cares
about that limit :-)

--
Alvaro Herrera (<alvherre[a]dcc.uchile.cl>)
"La soledad es compañia"

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2003-05-15 23:42:14 Re: plperl doesn't build in CVS tip?
Previous Message Tom Lane 2003-05-15 23:14:50 plperl doesn't build in CVS tip?

Browse pgsql-patches by date

  From Date Subject
Next Message Joe Conway 2003-05-16 04:53:51 fix for krb5 breakage
Previous Message Sean Chittenden 2003-05-15 21:12:01 Removal of krb4 support...