Tom Lane wrote:
"Florian G. Pflug" <[EMAIL PROTECTED]> writes:
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.
Sounds worthwile, I'm going to do this.
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.
Hm... I agree that xid_age is probably not performance-critical.
I guess it's more the complete counter-intuitivity of forcing
xid assignment in some arbitrary function thats bugging me a bit.
Maybe it'd be sufficient to guarantee a constant return value
of this function within one statement for read-committed
transaction? For serializable transactions I'd still be constant
Sounds not completely ill-logical to me - after all the age
of an xid *does* really change if my xmin moves forward.
But I can live with just doing GetTopTransactionId() too...
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 ...
I agree - I didn't consider the fact that Asserts are disable in
production builds when I made it an assertion.
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.
I not particularly proud of the name "ResourceOwnerId" - it was just the
best I could come up with ;-)
I quite like VirtualTransactionId actually - it emphasizes the point
that they never hit the disk. So I'll rename it to VirtualTransactionId
if no better idea comes up.
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.
It certainly is a wart, though but removing it will create a bigger one
I fear... The only other I idea I had was to make locks on RIDs session
locks, and to drop them explicitly at toplevel commit and abort time,
and that certainly has a quite high warty-ness score...
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
;-)
I *did* warn that I tend to be a bit chatty at times, though ;-)
greetings, Florian Pflug
---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly
