Alvaro Herrera wrote:
> I'm thinking about the comparison of full infomask as you propose
> instead of just the bits that we actually care about. I think the only
> thing that could cause a spurious failure (causing an extra execution of
> the HeapTupleSatisfiesUpdate call and the stuff below) is somebody
> setting HEAP_XMIN_COMMITTED concurrently; but that seems infrequent
> enough that it should pretty harmless. However, should we worry about
> possible future infomask bit changes that could negatively affect this
> behavior?
Here's a complete patch illustrating what I mean. This is slightly more
expensive than straight infomask comparison in terms of machine
instructions, but that seems okay to me.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
*** a/src/backend/access/heap/heapam.c
--- b/src/backend/access/heap/heapam.c
***************
*** 2586,2591 **** compute_infobits(uint16 infomask, uint16 infomask2)
--- 2586,2612 ----
}
/*
+ * Given two versions of the same t_infomask for a tuple, compare them and
+ * return whether the relevant status for a tuple Xmax has changed. This is
+ * used after a buffer lock has been released and reacquired: we want to ensure
+ * that the tuple state continues to be the same it was when we previously
+ * examined it.
+ *
+ * Note the Xmax field itself must be compared separately.
+ */
+ static inline bool
+ xmax_infomsk_changed(uint16 infomask1, uint16 infomask2)
+ {
+ const uint16 interesting =
+ HEAP_XMAX_IS_MULTI | HEAP_XMAX_LOCK_ONLY | HEAP_LOCK_MASK;
+
+ if ((infomask1 & interesting) != (infomask2 & interesting))
+ return true;
+
+ return false;
+ }
+
+ /*
* heap_delete - delete a tuple
*
* NB: do not call this directly unless you are prepared to deal with
***************
*** 2722,2728 **** l1:
* update this tuple before we get to this point. Check for xmax
* change, and start over if so.
*/
! if (!(tp.t_data->t_infomask & HEAP_XMAX_IS_MULTI) ||
!TransactionIdEquals(HeapTupleHeaderGetRawXmax(tp.t_data),
xwait))
goto l1;
--- 2743,2749 ----
* update this tuple before we get to this point. Check for xmax
* change, and start over if so.
*/
! if (xmax_infomsk_changed(tp.t_data->t_infomask, infomask) ||
!TransactionIdEquals(HeapTupleHeaderGetRawXmax(tp.t_data),
xwait))
goto l1;
***************
*** 2748,2754 **** l1:
* other xact could update this tuple before we get to this point.
* Check for xmax change, and start over if so.
*/
! if ((tp.t_data->t_infomask & HEAP_XMAX_IS_MULTI) ||
!TransactionIdEquals(HeapTupleHeaderGetRawXmax(tp.t_data),
xwait))
goto l1;
--- 2769,2775 ----
* other xact could update this tuple before we get to this point.
* Check for xmax change, and start over if so.
*/
! if (xmax_infomsk_changed(tp.t_data->t_infomask, infomask) ||
!TransactionIdEquals(HeapTupleHeaderGetRawXmax(tp.t_data),
xwait))
goto l1;
***************
*** 3275,3281 **** l2:
* update this tuple before we get to this point. Check for xmax
* change, and start over if so.
*/
! if (!(oldtup.t_data->t_infomask & HEAP_XMAX_IS_MULTI) ||
!TransactionIdEquals(HeapTupleHeaderGetRawXmax(oldtup.t_data),
xwait))
goto l2;
--- 3296,3302 ----
* update this tuple before we get to this point. Check for xmax
* change, and start over if so.
*/
! if (xmax_infomsk_changed(oldtup.t_data->t_infomask, infomask) ||
!TransactionIdEquals(HeapTupleHeaderGetRawXmax(oldtup.t_data),
xwait))
goto l2;
***************
*** 3324,3335 **** l2:
if (HEAP_XMAX_IS_KEYSHR_LOCKED(infomask) && key_intact)
{
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
-
/*
* recheck the locker; if someone else changed the tuple while
* we weren't looking, start over.
*/
! if ((oldtup.t_data->t_infomask & HEAP_XMAX_IS_MULTI) ||
!TransactionIdEquals(
HeapTupleHeaderGetRawXmax(oldtup.t_data),
xwait))
--- 3345,3355 ----
if (HEAP_XMAX_IS_KEYSHR_LOCKED(infomask) && key_intact)
{
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
/*
* recheck the locker; if someone else changed the tuple while
* we weren't looking, start over.
*/
! if (xmax_infomsk_changed(oldtup.t_data->t_infomask, infomask) ||
!TransactionIdEquals(
HeapTupleHeaderGetRawXmax(oldtup.t_data),
xwait))
***************
*** 3350,3356 **** l2:
* some other xact could update this tuple before we get to
* this point. Check for xmax change, and start over if so.
*/
! if ((oldtup.t_data->t_infomask & HEAP_XMAX_IS_MULTI) ||
!TransactionIdEquals(
HeapTupleHeaderGetRawXmax(oldtup.t_data),
xwait))
--- 3370,3376 ----
* some other xact could update this tuple before we get to
* this point. Check for xmax change, and start over if so.
*/
! if (xmax_infomsk_changed(oldtup.t_data->t_infomask, infomask) ||
!TransactionIdEquals(
HeapTupleHeaderGetRawXmax(oldtup.t_data),
xwait))
***************
*** 4354,4360 **** l3:
* over.
*/
LockBuffer(*buffer, BUFFER_LOCK_EXCLUSIVE);
! if (!(tuple->t_data->t_infomask & HEAP_XMAX_IS_MULTI) ||
!TransactionIdEquals(HeapTupleHeaderGetRawXmax(tuple->t_data),
xwait))
{
--- 4374,4380 ----
* over.
*/
LockBuffer(*buffer, BUFFER_LOCK_EXCLUSIVE);
! if (xmax_infomsk_changed(tuple->t_data->t_infomask, infomask) ||
!TransactionIdEquals(HeapTupleHeaderGetRawXmax(tuple->t_data),
xwait))
{
***************
*** 4373,4379 **** l3:
LockBuffer(*buffer, BUFFER_LOCK_EXCLUSIVE);
/* if the xmax changed in the meantime, start over */
! if ((tuple->t_data->t_infomask & HEAP_XMAX_IS_MULTI) ||
!TransactionIdEquals(
HeapTupleHeaderGetRawXmax(tuple->t_data),
xwait))
--- 4393,4399 ----
LockBuffer(*buffer, BUFFER_LOCK_EXCLUSIVE);
/* if the xmax changed in the meantime, start over */
! if (xmax_infomsk_changed(tuple->t_data->t_infomask, infomask) ||
!TransactionIdEquals(
HeapTupleHeaderGetRawXmax(tuple->t_data),
xwait))
***************
*** 4441,4447 **** l3:
* could update this tuple before we get to this point. Check
* for xmax change, and start over if so.
*/
! if (!(tuple->t_data->t_infomask & HEAP_XMAX_IS_MULTI) ||
!TransactionIdEquals(
HeapTupleHeaderGetRawXmax(tuple->t_data),
xwait))
--- 4461,4467 ----
* could update this tuple before we get to this point. Check
* for xmax change, and start over if so.
*/
! if (xmax_infomsk_changed(tuple->t_data->t_infomask, infomask) ||
!TransactionIdEquals(
HeapTupleHeaderGetRawXmax(tuple->t_data),
xwait))
***************
*** 4497,4503 **** l3:
* some other xact could update this tuple before we get to
* this point. Check for xmax change, and start over if so.
*/
! if ((tuple->t_data->t_infomask & HEAP_XMAX_IS_MULTI) ||
!TransactionIdEquals(
HeapTupleHeaderGetRawXmax(tuple->t_data),
xwait))
--- 4517,4523 ----
* some other xact could update this tuple before we get to
* this point. Check for xmax change, and start over if so.
*/
! if (xmax_infomsk_changed(tuple->t_data->t_infomask, infomask) ||
!TransactionIdEquals(
HeapTupleHeaderGetRawXmax(tuple->t_data),
xwait))
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
