Re: Advice on MyXactMade* flags, MyLastRecPtr, pendingDeletes and lazy XID assignment

Tom Lane wrote:
> One comment is that at the time we make an entry into smgr's
> pending-deletes list, I think we might not have acquired an XID yet
> --- if I understand your patch correctly, a CREATE TABLE would acquire
> an XID when it makes its first catalog insertion, and that happens
> after creating the on-disk table file. So it seems like a good idea
> for smgr itself to trigger acquisition of an XID before it makes a
> pending-deletes entry. This ensures that you can't have a situation
> where you have deletes to record and no XID; otherwise, an elog
> between smgr insertion and catalog insertion would lead to just that.

Hm.. I was just going to implement this, but I'm now wondering if
thats really worth it.

For smgrcreate, this would catch the following case:
.) CREATE something
.) smgrcreate: Creates file, and puts it onto the delete-on-abort
list
.) We elog() *before* acquiring an XID
.) RecordTransactionAbort or RecordSubTransactionAbort:
We don't write an ABORT record.
.) We crash *before* actually deleting the file

Compare the probability of that happening (The elog *and* the crash)
with the probability of
.) CREATE something
.) smgrcreate: Creates the file
.) We crash *before* we have to chance to commit or abort.

The window in which a crash causes us to leak the file seems to be much
wider in the second case, yet forcing XID assignment will not help to
preven it, unless I'm overlooking something.

In the smgrunlink case, there is no reason at all to force XID assignment,
because if we abort or crash, we don't want to unlink anyway, and if we
survive until we commit, we'll assign the XID during the inevitable catalog
update.

The only thing the forced XID assignment would buy is to be able to stick
if (TransactionIdIsValid(GetCurrentTransactionIdIfAny()))
Assert(nrels == 0);
into the various Record{Sub|}Transction{Commit|Abort} functions

So unless I'm overlooking something, I believe for now it's best to ignore this
issued, and to do a proper fix in the long run that removes *all* possible
leakages.

greetings, Florian Pflug