HeapTupleSatisfiesUpdate missing a bet?

From: Alvaro Herrera <alvherre(at)dcc(dot)uchile(dot)cl>
To: Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: HeapTupleSatisfiesUpdate missing a bet?
Date: 2005-03-25 22:54:35
Message-ID: 20050325225435.GA7588@dcc.uchile.cl
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hackers,

I got very strange results in my shared-row-locking test today, so I
took a look at HeapTupleSatisfiesUpdate and came to the conclusion
that it's delivering the wrong answer in some cases; specifically, it
returns HeapTupleBeingUpdated for tuples whose Xmax were touched by a
crashed transaction.

Apparently this doesn't matter on the current code because all callers
iterate using XactLockTableWait, which marks the deleting transaction as
aborted if it isn't running. However, I don't want to use
XactLockTableWait in the new code (in case we may be able to remove it).
So I'd like to have HeapTupleSatisfiesUpdate call TransactionIdAbort()
in case it's necessary.

What do people think of this patch? This is of course only to
illustrate what I'm talking about -- the real patch needs to change
other HeapTupleSatisfies routines as well. The other possibility would
be to have the new heap_locktuple routine check Xmax instead. Not sure
which is worst.

Index: src/backend/utils/time/tqual.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/backend/utils/time/tqual.c,v
retrieving revision 1.86
diff -c -r1.86 tqual.c
*** src/backend/utils/time/tqual.c 20 Mar 2005 23:40:27 -0000 1.86
--- src/backend/utils/time/tqual.c 25 Mar 2005 22:40:53 -0000
***************
*** 539,544 ****
--- 539,550 ----
tuple->t_infomask |= HEAP_XMIN_INVALID;
SetBufferCommitInfoNeedsSave(buffer);
}
+ else if (!TransactionIdIsInProgress(HeapTupleHeaderGetXmin(tuple)))
+ {
+ TransactionIdAbort(HeapTupleHeaderGetXmin(tuple));
+ tuple->t_infomask |= HEAP_XMIN_INVALID;
+ SetBufferCommitInfoNeedsSave(buffer);
+ }
return HeapTupleInvisible;
}
else
***************
*** 579,584 ****
--- 585,597 ----
SetBufferCommitInfoNeedsSave(buffer);
return HeapTupleMayBeUpdated;
}
+ else if (!TransactionIdIsInProgress(HeapTupleHeaderGetXmax(tuple)))
+ {
+ TransactionIdAbort(HeapTupleHeaderGetXmax(tuple));
+ tuple->t_infomask |= HEAP_XMAX_INVALID;
+ SetBufferCommitInfoNeedsSave(buffer);
+ return HeapTupleMayBeUpdated;
+ }
/* running xact */
return HeapTupleBeingUpdated; /* in updation by other */
}

--
Alvaro Herrera (<alvherre[(at)]dcc(dot)uchile(dot)cl>)
"I personally became interested in Linux while I was dating an English major
who wouldn't know an operating system if it walked up and bit him."
(Val Henson)

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2005-03-25 23:06:02 Re: pg_autovacuum not having enough suction ?
Previous Message Tom Lane 2005-03-25 22:35:58 Re: lazy_update_relstats considered harmful (was Re: [PERFORM] pg_autovacuum not having enough suction ?)