Heikki Linnakangas escribió:
> Here's a rebased version of the patch, including the above-mentioned
> fixes. Nothing else new.
Nice work. I apologize for the conflicts I created yesterday.
I would suggest renaming varsup_internal.h to varsup_xlog.h.
You added a FIXME comment to HeapTupleHeaderIsOnlyLocked. I think the
answer to the question is yes, because checking for locked might incur
examining the Xmax of tuples, which cannot be done if the page is
mature; perhaps the check for maturity needs to happen only after the
infomask has been checked.
The new stuff in GetNewTransactionId() is said to be "very expensive"
(it might involve grabbing the ProcArrayLock and scanning the whole
procarray, for example.) There's a comment about having this be done
only in other processes, and I think that's a good idea, otherwise we
risk adding a lot of latency, randomly, to client-connected processes
which might be latency sensitive. I think autovacuum is not a good
choice however (it might not even be running). How about using the
bgwriter or walwriter for this? As far as I can tell, there's no need
for this process to actually be able to run transactions or scan
catalogs; the ability to lock and scan ProcArray appears sufficient.
Using a RTC (instead of the Xid counter % 128) to determine when to run
this doesn't look like a problem to me. Something that sleeps for too
long might be, so we would need to ensure that it happens timely. Not
sure what's an appropriate time for this, however.
Another option is have backends check the Xid counter, and every 128
ticks set a flag in shmem so that the background process knows to
execute the switch.
heap_freeze_tuple() receives one Xid and one MultiXactId; they can be
passed as Invalid to mean forced freezing. Do we really need to offer
the possibility of freezing xids but not multis, and multis but not
xids? From a conceptual PoV, it seems to make more sense to me to pass
a boolean "force" meaning freeze everything, and ignore the numerical
values.
The greatest risk in this patch is the possibility that some
heap_freeze_page() call is forgotten. I think you covered all cases in
heapam.c.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services