Re: [HACKERS] MARKED_FOR_UPDATE && XMAX_COMMITTED == XMAX_INVALID ?

Patch applied. Thanks.

---------------------------------------------------------------------------

Manfred Koizar wrote:
> On Wed, 11 Jun 2003 09:05:33 -0400, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
> wrote:
> >> If a transaction marks a tuple for update and later commits without
> >> actually having updated the tuple, [...] can we simply
> >> set the HEAP_XMAX_INVALID hint bit of the tuple?
> >
> >AFAICS this is a reasonable thing to do.
>
> Thanks for the confirmation. Here's a patch which also contains some
> more noncritical changes to tqual.c:
> . make code more readable by introducing local variables for xvac
> . no longer two separate branches for aborted and crashed.
> The actions were the same in all cases.
>
> Servus
> Manfred

> diff -rcN ../base/src/backend/utils/time/tqual.c src/backend/utils/time/tqual.c
> *** ../base/src/backend/utils/time/tqual.c Thu May 8 21:45:55 2003
> --- src/backend/utils/time/tqual.c Thu Jun 12 12:10:31 2003
> ***************
> *** 76,86 ****
>
> if (tuple->t_infomask & HEAP_MOVED_OFF)
> {
> ! if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXvac(tuple)))
> return false;
> ! if (!TransactionIdIsInProgress(HeapTupleHeaderGetXvac(tuple)))
> {
> ! if (TransactionIdDidCommit(HeapTupleHeaderGetXvac(tuple)))
> {
> tuple->t_infomask |= HEAP_XMIN_INVALID;
> return false;
> --- 76,87 ----
>
> if (tuple->t_infomask & HEAP_MOVED_OFF)
> {
> ! TransactionId xvac = HeapTupleHeaderGetXvac(tuple);
> ! if (TransactionIdIsCurrentTransactionId(xvac))
> return false;
> ! if (!TransactionIdIsInProgress(xvac))
> {
> ! if (TransactionIdDidCommit(xvac))
> {
> tuple->t_infomask |= HEAP_XMIN_INVALID;
> return false;
> ***************
> *** 90,100 ****
> }
> else if (tuple->t_infomask & HEAP_MOVED_IN)
> {
> ! if (!TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXvac(tuple)))
> {
> ! if (TransactionIdIsInProgress(HeapTupleHeaderGetXvac(tuple)))
> return false;
> ! if (TransactionIdDidCommit(HeapTupleHeaderGetXvac(tuple)))
> tuple->t_infomask |= HEAP_XMIN_COMMITTED;
> else
> {
> --- 91,102 ----
> }
> else if (tuple->t_infomask & HEAP_MOVED_IN)
> {
> ! TransactionId xvac = HeapTupleHeaderGetXvac(tuple);
> ! if (!TransactionIdIsCurrentTransactionId(xvac))
> {
> ! if (TransactionIdIsInProgress(xvac))
> return false;
> ! if (TransactionIdDidCommit(xvac))
> tuple->t_infomask |= HEAP_XMIN_COMMITTED;
> else
> {
> ***************
> *** 152,162 ****
> }
>
> /* xmax transaction committed */
> - tuple->t_infomask |= HEAP_XMAX_COMMITTED;
>
> if (tuple->t_infomask & HEAP_MARKED_FOR_UPDATE)
> return true;
>
> return false;
> }
>
> --- 154,167 ----
> }
>
> /* xmax transaction committed */
>
> if (tuple->t_infomask & HEAP_MARKED_FOR_UPDATE)
> + {
> + tuple->t_infomask |= HEAP_XMAX_INVALID;
> return true;
> + }
>
> + tuple->t_infomask |= HEAP_XMAX_COMMITTED;
> return false;
> }
>
> ***************
> *** 212,222 ****
>
> if (tuple->t_infomask & HEAP_MOVED_OFF)
> {
> ! if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXvac(tuple)))
> return false;
> ! if (!TransactionIdIsInProgress(HeapTupleHeaderGetXvac(tuple)))
> {
> ! if (TransactionIdDidCommit(HeapTupleHeaderGetXvac(tuple)))
> {
> tuple->t_infomask |= HEAP_XMIN_INVALID;
> return false;
> --- 217,228 ----
>
> if (tuple->t_infomask & HEAP_MOVED_OFF)
> {
> ! TransactionId xvac = HeapTupleHeaderGetXvac(tuple);
> ! if (TransactionIdIsCurrentTransactionId(xvac))
> return false;
> ! if (!TransactionIdIsInProgress(xvac))
> {
> ! if (TransactionIdDidCommit(xvac))
> {
> tuple->t_infomask |= HEAP_XMIN_INVALID;
> return false;
> ***************
> *** 226,236 ****
> }
> else if (tuple->t_infomask & HEAP_MOVED_IN)
> {
> ! if (!TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXvac(tuple)))
> {
> ! if (TransactionIdIsInProgress(HeapTupleHeaderGetXvac(tuple)))
> return false;
> ! if (TransactionIdDidCommit(HeapTupleHeaderGetXvac(tuple)))
> tuple->t_infomask |= HEAP_XMIN_COMMITTED;
> else
> {
> --- 232,243 ----
> }
> else if (tuple->t_infomask & HEAP_MOVED_IN)
> {
> ! TransactionId xvac = HeapTupleHeaderGetXvac(tuple);
> ! if (!TransactionIdIsCurrentTransactionId(xvac))
> {
> ! if (TransactionIdIsInProgress(xvac))
> return false;
> ! if (TransactionIdDidCommit(xvac))
> tuple->t_infomask |= HEAP_XMIN_COMMITTED;
> else
> {
> ***************
> *** 297,307 ****
> }
>
> /* xmax transaction committed */
> - tuple->t_infomask |= HEAP_XMAX_COMMITTED;
>
> if (tuple->t_infomask & HEAP_MARKED_FOR_UPDATE)
> return true;
>
> return false;
> }
>
> --- 304,317 ----
> }
>
> /* xmax transaction committed */
>
> if (tuple->t_infomask & HEAP_MARKED_FOR_UPDATE)
> + {
> + tuple->t_infomask |= HEAP_XMAX_INVALID;
> return true;
> + }
>
> + tuple->t_infomask |= HEAP_XMAX_COMMITTED;
> return false;
> }
>
> ***************
> *** 329,339 ****
>
> if (tuple->t_infomask & HEAP_MOVED_OFF)
> {
> ! if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXvac(tuple)))
> return false;
> ! if (!TransactionIdIsInProgress(HeapTupleHeaderGetXvac(tuple)))
> {
> ! if (TransactionIdDidCommit(HeapTupleHeaderGetXvac(tuple)))
> {
> tuple->t_infomask |= HEAP_XMIN_INVALID;
> return false;
> --- 339,350 ----
>
> if (tuple->t_infomask & HEAP_MOVED_OFF)
> {
> ! TransactionId xvac = HeapTupleHeaderGetXvac(tuple);
> ! if (TransactionIdIsCurrentTransactionId(xvac))
> return false;
> ! if (!TransactionIdIsInProgress(xvac))
> {
> ! if (TransactionIdDidCommit(xvac))
> {
> tuple->t_infomask |= HEAP_XMIN_INVALID;
> return false;
> ***************
> *** 343,353 ****
> }
> else if (tuple->t_infomask & HEAP_MOVED_IN)
> {
> ! if (!TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXvac(tuple)))
> {
> ! if (TransactionIdIsInProgress(HeapTupleHeaderGetXvac(tuple)))
> return false;
> ! if (TransactionIdDidCommit(HeapTupleHeaderGetXvac(tuple)))
> tuple->t_infomask |= HEAP_XMIN_COMMITTED;
> else
> {
> --- 354,365 ----
> }
> else if (tuple->t_infomask & HEAP_MOVED_IN)
> {
> ! TransactionId xvac = HeapTupleHeaderGetXvac(tuple);
> ! if (!TransactionIdIsCurrentTransactionId(xvac))
> {
> ! if (TransactionIdIsInProgress(xvac))
> return false;
> ! if (TransactionIdDidCommit(xvac))
> tuple->t_infomask |= HEAP_XMIN_COMMITTED;
> else
> {
> ***************
> *** 382,392 ****
>
> if (tuple->t_infomask & HEAP_MOVED_OFF)
> {
> ! if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXvac(tuple)))
> return HeapTupleInvisible;
> ! if (!TransactionIdIsInProgress(HeapTupleHeaderGetXvac(tuple)))
> {
> ! if (TransactionIdDidCommit(HeapTupleHeaderGetXvac(tuple)))
> {
> tuple->t_infomask |= HEAP_XMIN_INVALID;
> return HeapTupleInvisible;
> --- 394,405 ----
>
> if (tuple->t_infomask & HEAP_MOVED_OFF)
> {
> ! TransactionId xvac = HeapTupleHeaderGetXvac(tuple);
> ! if (TransactionIdIsCurrentTransactionId(xvac))
> return HeapTupleInvisible;
> ! if (!TransactionIdIsInProgress(xvac))
> {
> ! if (TransactionIdDidCommit(xvac))
> {
> tuple->t_infomask |= HEAP_XMIN_INVALID;
> return HeapTupleInvisible;
> ***************
> *** 396,406 ****
> }
> else if (tuple->t_infomask & HEAP_MOVED_IN)
> {
> ! if (!TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXvac(tuple)))
> {
> ! if (TransactionIdIsInProgress(HeapTupleHeaderGetXvac(tuple)))
> return HeapTupleInvisible;
> ! if (TransactionIdDidCommit(HeapTupleHeaderGetXvac(tuple)))
> tuple->t_infomask |= HEAP_XMIN_COMMITTED;
> else
> {
> --- 409,420 ----
> }
> else if (tuple->t_infomask & HEAP_MOVED_IN)
> {
> ! TransactionId xvac = HeapTupleHeaderGetXvac(tuple);
> ! if (!TransactionIdIsCurrentTransactionId(xvac))
> {
> ! if (TransactionIdIsInProgress(xvac))
> return HeapTupleInvisible;
> ! if (TransactionIdDidCommit(xvac))
> tuple->t_infomask |= HEAP_XMIN_COMMITTED;
> else
> {
> ***************
> *** 475,485 ****
> }
>
> /* xmax transaction committed */
> - tuple->t_infomask |= HEAP_XMAX_COMMITTED;
>
> if (tuple->t_infomask & HEAP_MARKED_FOR_UPDATE)
> return HeapTupleMayBeUpdated;
>
> return HeapTupleUpdated; /* updated by other */
> }
>
> --- 489,502 ----
> }
>
> /* xmax transaction committed */
>
> if (tuple->t_infomask & HEAP_MARKED_FOR_UPDATE)
> + {
> + tuple->t_infomask |= HEAP_XMAX_INVALID;
> return HeapTupleMayBeUpdated;
> + }
>
> + tuple->t_infomask |= HEAP_XMAX_COMMITTED;
> return HeapTupleUpdated; /* updated by other */
> }
>
> ***************
> *** 513,523 ****
>
> if (tuple->t_infomask & HEAP_MOVED_OFF)
> {
> ! if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXvac(tuple)))
> return false;
> ! if (!TransactionIdIsInProgress(HeapTupleHeaderGetXvac(tuple)))
> {
> ! if (TransactionIdDidCommit(HeapTupleHeaderGetXvac(tuple)))
> {
> tuple->t_infomask |= HEAP_XMIN_INVALID;
> return false;
> --- 530,541 ----
>
> if (tuple->t_infomask & HEAP_MOVED_OFF)
> {
> ! TransactionId xvac = HeapTupleHeaderGetXvac(tuple);
> ! if (TransactionIdIsCurrentTransactionId(xvac))
> return false;
> ! if (!TransactionIdIsInProgress(xvac))
> {
> ! if (TransactionIdDidCommit(xvac))
> {
> tuple->t_infomask |= HEAP_XMIN_INVALID;
> return false;
> ***************
> *** 527,537 ****
> }
> else if (tuple->t_infomask & HEAP_MOVED_IN)
> {
> ! if (!TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXvac(tuple)))
> {
> ! if (TransactionIdIsInProgress(HeapTupleHeaderGetXvac(tuple)))
> return false;
> ! if (TransactionIdDidCommit(HeapTupleHeaderGetXvac(tuple)))
> tuple->t_infomask |= HEAP_XMIN_COMMITTED;
> else
> {
> --- 545,556 ----
> }
> else if (tuple->t_infomask & HEAP_MOVED_IN)
> {
> ! TransactionId xvac = HeapTupleHeaderGetXvac(tuple);
> ! if (!TransactionIdIsCurrentTransactionId(xvac))
> {
> ! if (TransactionIdIsInProgress(xvac))
> return false;
> ! if (TransactionIdDidCommit(xvac))
> tuple->t_infomask |= HEAP_XMIN_COMMITTED;
> else
> {
> ***************
> *** 600,610 ****
> }
>
> /* xmax transaction committed */
> - tuple->t_infomask |= HEAP_XMAX_COMMITTED;
>
> if (tuple->t_infomask & HEAP_MARKED_FOR_UPDATE)
> return true;
>
> SnapshotDirty->tid = tuple->t_ctid;
> return false; /* updated by other */
> }
> --- 619,632 ----
> }
>
> /* xmax transaction committed */
>
> if (tuple->t_infomask & HEAP_MARKED_FOR_UPDATE)
> + {
> + tuple->t_infomask |= HEAP_XMAX_INVALID;
> return true;
> + }
>
> + tuple->t_infomask |= HEAP_XMAX_COMMITTED;
> SnapshotDirty->tid = tuple->t_ctid;
> return false; /* updated by other */
> }
> ***************
> *** 644,654 ****
>
> if (tuple->t_infomask & HEAP_MOVED_OFF)
> {
> ! if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXvac(tuple)))
> return false;
> ! if (!TransactionIdIsInProgress(HeapTupleHeaderGetXvac(tuple)))
> {
> ! if (TransactionIdDidCommit(HeapTupleHeaderGetXvac(tuple)))
> {
> tuple->t_infomask |= HEAP_XMIN_INVALID;
> return false;
> --- 666,677 ----
>
> if (tuple->t_infomask & HEAP_MOVED_OFF)
> {
> ! TransactionId xvac = HeapTupleHeaderGetXvac(tuple);
> ! if (TransactionIdIsCurrentTransactionId(xvac))
> return false;
> ! if (!TransactionIdIsInProgress(xvac))
> {
> ! if (TransactionIdDidCommit(xvac))
> {
> tuple->t_infomask |= HEAP_XMIN_INVALID;
> return false;
> ***************
> *** 658,668 ****
> }
> else if (tuple->t_infomask & HEAP_MOVED_IN)
> {
> ! if (!TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXvac(tuple)))
> {
> ! if (TransactionIdIsInProgress(HeapTupleHeaderGetXvac(tuple)))
> return false;
> ! if (TransactionIdDidCommit(HeapTupleHeaderGetXvac(tuple)))
> tuple->t_infomask |= HEAP_XMIN_COMMITTED;
> else
> {
> --- 681,692 ----
> }
> else if (tuple->t_infomask & HEAP_MOVED_IN)
> {
> ! TransactionId xvac = HeapTupleHeaderGetXvac(tuple);
> ! if (!TransactionIdIsCurrentTransactionId(xvac))
> {
> ! if (TransactionIdIsInProgress(xvac))
> return false;
> ! if (TransactionIdDidCommit(xvac))
> tuple->t_infomask |= HEAP_XMIN_COMMITTED;
> else
> {
> ***************
> *** 802,812 ****
> return HEAPTUPLE_DEAD;
> else if (tuple->t_infomask & HEAP_MOVED_OFF)
> {
> ! if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXvac(tuple)))
> return HEAPTUPLE_DELETE_IN_PROGRESS;
> ! if (TransactionIdIsInProgress(HeapTupleHeaderGetXvac(tuple)))
> return HEAPTUPLE_DELETE_IN_PROGRESS;
> ! if (TransactionIdDidCommit(HeapTupleHeaderGetXvac(tuple)))
> {
> tuple->t_infomask |= HEAP_XMIN_INVALID;
> return HEAPTUPLE_DEAD;
> --- 826,837 ----
> return HEAPTUPLE_DEAD;
> else if (tuple->t_infomask & HEAP_MOVED_OFF)
> {
> ! TransactionId xvac = HeapTupleHeaderGetXvac(tuple);
> ! if (TransactionIdIsCurrentTransactionId(xvac))
> return HEAPTUPLE_DELETE_IN_PROGRESS;
> ! if (TransactionIdIsInProgress(xvac))
> return HEAPTUPLE_DELETE_IN_PROGRESS;
> ! if (TransactionIdDidCommit(xvac))
> {
> tuple->t_infomask |= HEAP_XMIN_INVALID;
> return HEAPTUPLE_DEAD;
> ***************
> *** 815,825 ****
> }
> else if (tuple->t_infomask & HEAP_MOVED_IN)
> {
> ! if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXvac(tuple)))
> return HEAPTUPLE_INSERT_IN_PROGRESS;
> ! if (TransactionIdIsInProgress(HeapTupleHeaderGetXvac(tuple)))
> return HEAPTUPLE_INSERT_IN_PROGRESS;
> ! if (TransactionIdDidCommit(HeapTupleHeaderGetXvac(tuple)))
> tuple->t_infomask |= HEAP_XMIN_COMMITTED;
> else
> {
> --- 840,851 ----
> }
> else if (tuple->t_infomask & HEAP_MOVED_IN)
> {
> ! TransactionId xvac = HeapTupleHeaderGetXvac(tuple);
> ! if (TransactionIdIsCurrentTransactionId(xvac))
> return HEAPTUPLE_INSERT_IN_PROGRESS;
> ! if (TransactionIdIsInProgress(xvac))
> return HEAPTUPLE_INSERT_IN_PROGRESS;
> ! if (TransactionIdDidCommit(xvac))
> tuple->t_infomask |= HEAP_XMIN_COMMITTED;
> else
> {
> ***************
> *** 831,846 ****
> return HEAPTUPLE_INSERT_IN_PROGRESS;
> else if (TransactionIdDidCommit(HeapTupleHeaderGetXmin(tuple)))
> tuple->t_infomask |= HEAP_XMIN_COMMITTED;
> - else if (TransactionIdDidAbort(HeapTupleHeaderGetXmin(tuple)))
> - {
> - tuple->t_infomask |= HEAP_XMIN_INVALID;
> - return HEAPTUPLE_DEAD;
> - }
> else
> {
> /*
> ! * Not in Progress, Not Committed, Not Aborted - so it's from
> ! * crashed process. - vadim 11/26/96
> */
> tuple->t_infomask |= HEAP_XMIN_INVALID;
> return HEAPTUPLE_DEAD;
> --- 857,866 ----
> return HEAPTUPLE_INSERT_IN_PROGRESS;
> else if (TransactionIdDidCommit(HeapTupleHeaderGetXmin(tuple)))
> tuple->t_infomask |= HEAP_XMIN_COMMITTED;
> else
> {
> /*
> ! * Not in Progress, Not Committed, so either Aborted or crashed
> */
> tuple->t_infomask |= HEAP_XMIN_INVALID;
> return HEAPTUPLE_DEAD;
> ***************
> *** 868,878 ****
> {
> if (TransactionIdIsInProgress(HeapTupleHeaderGetXmax(tuple)))
> return HEAPTUPLE_LIVE;
> ! if (TransactionIdDidCommit(HeapTupleHeaderGetXmax(tuple)))
> ! tuple->t_infomask |= HEAP_XMAX_COMMITTED;
> ! else
> ! /* it's either aborted or crashed */
> ! tuple->t_infomask |= HEAP_XMAX_INVALID;
> }
> return HEAPTUPLE_LIVE;
> }
> --- 888,899 ----
> {
> if (TransactionIdIsInProgress(HeapTupleHeaderGetXmax(tuple)))
> return HEAPTUPLE_LIVE;
> ! /*
> ! * We don't really care whether xmax did commit, abort or crash.
> ! * We know that xmax did mark the tuple for update, but it did not
> ! * and will never actually update it.
> ! */
> ! tuple->t_infomask |= HEAP_XMAX_INVALID;
> }
> return HEAPTUPLE_LIVE;
> }
> ***************
> *** 883,898 ****
> return HEAPTUPLE_DELETE_IN_PROGRESS;
> else if (TransactionIdDidCommit(HeapTupleHeaderGetXmax(tuple)))
> tuple->t_infomask |= HEAP_XMAX_COMMITTED;
> - else if (TransactionIdDidAbort(HeapTupleHeaderGetXmax(tuple)))
> - {
> - tuple->t_infomask |= HEAP_XMAX_INVALID;
> - return HEAPTUPLE_LIVE;
> - }
> else
> {
> /*
> ! * Not in Progress, Not Committed, Not Aborted - so it's from
> ! * crashed process. - vadim 06/02/97
> */
> tuple->t_infomask |= HEAP_XMAX_INVALID;
> return HEAPTUPLE_LIVE;
> --- 904,913 ----
> return HEAPTUPLE_DELETE_IN_PROGRESS;
> else if (TransactionIdDidCommit(HeapTupleHeaderGetXmax(tuple)))
> tuple->t_infomask |= HEAP_XMAX_COMMITTED;
> else
> {
> /*
> ! * Not in Progress, Not Committed, so either Aborted or crashed
> */
> tuple->t_infomask |= HEAP_XMAX_INVALID;
> return HEAPTUPLE_LIVE;

>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073