Re: [PATCH] Lazy xid assingment V2

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "Florian G(dot) Pflug" <fgp(at)phlo(dot)org>
Cc: Postgresql-Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Lazy xid assingment V2
Date: 2007-08-31 21:10:41
Message-ID: 1705.1188594641@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

"Florian G. Pflug" <fgp(at)phlo(dot)org> writes:
> The patch can be found at: http://soc.phlo.org/lazyxidassign.v2.patch
> (Seems that large attachments don't get through on this list - or was that
> just bad luck when I tried posting the first version of that patch?)

No, the message size limit on -hackers is relatively small. You
shouldn't post patches here anyway, they belong on -patches.

A few comments ...

> GetOldestXmin() is changed slightly - it used to use
> GetTopTransactionId() as a starting point for it's xmin
> calculation, falling back to ReadNewTransactionId() if called
> from outside a transaction. Now, it always starts with ReadNewTransactionId().

It's kind of annoying to do ReadNewTransactionId() when most of the time
it won't affect the result. However I don't see much choice; if we
tried to read it only after failing to find any entries in the
procarray, we'd have a race condition, since someone might acquire
an xid and put it into the procarray after we look at their PGPROC
and before we can do ReadNewTransactionId.

It might be worth doing GetTopTransactionIdIfAny and only calling
ReadNewTransactionId when that fails, since the former is far
cheaper than the latter. Also we could cheaply check to see if
our own xmin is set, and use that to initialize the computation.

> The function xid_age computes the age of a given xid relative
> to the transaction's toplevel xid. Since forcing xid assignment
> just to do that computation seemed silly, I modified xid_age
> to use MyProc->xmin as a reference instead. I could have used
> ReadNewTransactionId(), but that involves locking overhead,
> and we probably want to use something that remains constant
> during a transaction.

I don't much like this, since as I mentioned before I don't think
MyProc->xmin is going to be constant over a whole transaction for
long. I don't think xid_age is performance-critical in any way,
so I'd vote for letting it force XID assignment.

> The first requirement is already satisfied - file creation
> will surely be followed by a catalog update, which will force xid assingment,
> and thus writing a COMMIT record. There is an assertion in
> RecordTransactionCommit that checks for no schedules file deletions when
> we don't have a xid assigned.

Rather than an Assert (which won't be there in production), I'd suggest
a standard test and elog(ERROR). This point is sufficiently critical
that I don't want the test to be missing in production builds ...

> Therefore, the concept of a ResourceOwnerId is introduced. This id identifies
> the toplevel resourceowner of a transaction, and the owning transaction holds
> a lock on it just as it does for transaction ids. Porting the first waiting
> phase to this is trivial - we just collect the RIDs instead of the XIDs of
> the current lock holders, and wait for them in turn.

I don't particularly like calling these ResourceOwnerId --
ResourceOwners are an implementation artifact that's purely
backend-local. If we were to get rid of them in favor of some other
resource management mechanism, we'd still need RIDs for the waiting
purposes you mention. So I'm in favor of calling them something else,
maybe virtual transaction ids (VXID or VID) --- not wedded to that if
someone has a better idea.

> Locks on ResourceOwnerIds are not saved to the 2PC state file on PREPARE, since
> the RID or a transaction will be reused at time point after it was prepared.

Hmm. I think this is all right since we don't actually need to wait for
a prepared xact, but it still seems like a wart.

Other than these relatively minor quibbles, the description seems all
right, and it's so complete that I'm not sure I need to read the code
;-)

As I just mentioned over in pgsql-performance, I'm strongly inclined
to push this patch into 8.3, even though we're certainly far past the
feature freeze deadline. Not allocating XIDs for transactions that
only do SELECTs seems to me like it'll be a big win for a lot of
high-performance applications. It's not so much that the act of
allocating an XID is really expensive, it's that making a significant
reduction in the rate at which XIDs are used up translates directly
to a reduction in many overhead costs. For instance, the "live" area of
pg_clog and pg_subtrans gets smaller, reducing pressure on those buffer
areas, and the frequency with which vacuums are needed for
anti-wraparound purposes drops.

Comments?

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Decibel! 2007-08-31 21:17:50 Re: [GENERAL] Undetected corruption of table files
Previous Message Kevin Grittner 2007-08-31 20:30:31 Re: Attempt to stop dead instance can stop a random process?