From: | Peter Geoghegan <pg(at)heroku(dot)com> |
---|---|
To: | Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> |
Cc: | Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>, Andres Freund <andres(at)2ndquadrant(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Subject: | Re: INSERT...ON DUPLICATE KEY LOCK FOR UPDATE |
Date: | 2014-01-16 01:25:37 |
Message-ID: | CAM3SWZSvUAtxfJ7UM4GqaO-F5E-SLZzuXZ7CgOeV5dGZUJ=yNQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Jan 14, 2014 at 3:25 PM, Heikki Linnakangas
<hlinnakangas(at)vmware(dot)com> wrote:
> Attached is a patch doing that, to again demonstrate what I mean. I'm not
> sure if setting xmin to invalid is really the best way to mark the tuple
> dead; I don't think a tuple's xmin can currently be InvalidTransaction under
> any other circumstances, so there might be some code out there that's not
> prepared for it. So using an infomask bit might indeed be better. Or
> something else entirely.
Have you thought about the implications for other snapshot types (or
other tqual.c routines)? My concern is that a client of that
infrastructure (either current or future) could spuriously conclude
that a heap tuple satisfied it, when in fact only a promise tuple
satisfied it. It wouldn't necessarily follow that the promise would be
fulfilled, nor that there would be some other proper heap tuple
equivalent to that fulfilled promise tuple as far as those clients are
concerned.
heap_delete() will not call HeapTupleSatisfiesUpdate() when you're
deleting a promise tuple, which on the face of it is fine - it's
always going to technically be instantaneously invisible, because it's
always created by the same command id (i.e. HeapTupleSatisfiesUpdate()
would just return HeapTupleInvisible if called). So far so good, but
we are technically doing something else quite new - deleting a
would-be instantaneously invisible tuple. So like your concern about
setting xmin to invalid, my concern is that code may exist that treats
cmin < cmax as an invariant. Now, you might think that that would be a
manageable concern, and to be fair a look at the ComboCids code that
mostly arbitrates that stuff seems to indicate that it's okay, but
it's still worth noting.
I think you should consider breaking off the relcache parts of my
patch and committing them, because they're independently useful. If we
are going to have a lot of conflicts that need to be handled by a
heap_delete(), there is no point in inserting non-unique index tuples
for what is not yet conclusively a proper (non-promise) tuple. Those
should always come last. And even without upsert, strictly inserting
into unique indexes first seems like a useful thing relative to the
cost. Unique violations are the cause of many aborted transactions,
and there is no need to ever bloat non-unique indexes of the same slot
when that happens.
--
Peter Geoghegan
From | Date | Subject | |
---|---|---|---|
Next Message | Simon Riggs | 2014-01-16 01:29:15 | Re: 9.2.1 & index-only scans : abnormal heap fetches after VACUUM FULL |
Previous Message | Dave Chinner | 2014-01-16 00:47:17 | Re: [Lsf-pc] Linux kernel impact on PostgreSQL performance |