Обсуждение: HeapTuple header changes cause core dumps in CVS tip

An example is:

regression=# create table foo (f1 text);
CREATE TABLE
regression=# insert into foo values ('zzzzzzzzzzzzz');
INSERT 148289 1
regression=# insert into foo select * from foo;
INSERT 148290 1
regression=# insert into foo select * from foo;
INSERT 0 2
regression=# insert into foo select * from foo;
INSERT 0 4
<< repeat enough times to have 1000 or so tuples in table >>

regression=# insert into foo values ('q');
INSERT 150337 1
regression=# delete from foo where f1 != 'q';
DELETE 2048
regression=# vacuum full foo;
VACUUM
regression=# update foo set f1 = 'qq';
server closed the connection unexpectedly       This probably means the server terminated abnormally       before or
whileprocessing the request.
 

Backtracing shows that the assertion in HeapTupleHeaderSetCmax fails,
because it's not expecting to find the HEAP_MOVED bits set in its input.
(The above test is simply an easy way of forcing an update on a tuple
that has been moved by VACUUM FULL.)

I am not sure if this is a bug introduced by the patch, or if it's
exposed a previously lurking bug.  It seems that the HEAP_MOVED bits
should be cleared before re-using cmax for something else, but I have
not dug through the old logic to see how it was done before.  Or perhaps
we cannot really reduce the number of fields this far.
        regards, tom lane

On Mon, 15 Jul 2002 16:46:44 -0400, Tom Lane <tgl@sss.pgh.pa.us>
wrote:
>regression=# update foo set f1 = 'qq';
>server closed the connection unexpectedly

Same with DELETE FROM foo;

>I am not sure if this is a bug introduced by the patch, or if it's
>exposed a previously lurking bug.

I suspect the former :-(

>It seems that the HEAP_MOVED bits
>should be cleared before re-using cmax for something else, but I have
>not dug through the old logic to see how it was done before.

AFAICS from a quick look at tqual it didn't matter before.  Once the
vacuum transaction had committed, on the next access to the tuple
MOVED_IN caused XMIN_COMMITTED to be set, and after that the MOVED
bits were never looked at again.  MOVED_OFF is not an issue.

I'll take a closer look at tqual and the vacuum source code tomorrow.
For now the attached patch cures both symptoms (UPDATE and DELETE) and
passes all regression tests.  A regression test for this case will
follow.

Servus
 Manfred
diff -ruN ../base/src/backend/access/heap/heapam.c src/backend/access/heap/heapam.c
--- ../base/src/backend/access/heap/heapam.c    2002-07-15 22:22:28.000000000 +0200
+++ src/backend/access/heap/heapam.c    2002-07-16 00:16:59.000000000 +0200
@@ -1123,11 +1123,11 @@
             CheckMaxObjectId(HeapTupleGetOid(tup));
     }

+    tup->t_data->t_infomask &= ~(HEAP_XACT_MASK);
     HeapTupleHeaderSetXmin(tup->t_data, GetCurrentTransactionId());
     HeapTupleHeaderSetCmin(tup->t_data, cid);
     HeapTupleHeaderSetXmaxInvalid(tup->t_data);
-    HeapTupleHeaderSetCmax(tup->t_data, FirstCommandId);
-    tup->t_data->t_infomask &= ~(HEAP_XACT_MASK);
+    /* HeapTupleHeaderSetCmax(tup->t_data, FirstCommandId); */
     tup->t_data->t_infomask |= HEAP_XMAX_INVALID;
     tup->t_tableOid = relation->rd_id;

@@ -1321,7 +1321,7 @@

     START_CRIT_SECTION();
     /* store transaction information of xact deleting the tuple */
-    tp.t_data->t_infomask &= ~(HEAP_XMAX_COMMITTED |
+    tp.t_data->t_infomask &= ~(HEAP_XMAX_COMMITTED | HEAP_MOVED |
                              HEAP_XMAX_INVALID | HEAP_MARKED_FOR_UPDATE);
     HeapTupleHeaderSetXmax(tp.t_data, GetCurrentTransactionId());
     HeapTupleHeaderSetCmax(tp.t_data, cid);
@@ -1554,7 +1554,7 @@
         _locked_tuple_.tid = oldtup.t_self;
         XactPushRollback(_heap_unlock_tuple, (void *) &_locked_tuple_);

-        oldtup.t_data->t_infomask &= ~(HEAP_XMAX_COMMITTED |
+        oldtup.t_data->t_infomask &= ~(HEAP_XMAX_COMMITTED | HEAP_MOVED |
                                        HEAP_XMAX_INVALID |
                                        HEAP_MARKED_FOR_UPDATE);
         oldtup.t_data->t_infomask |= HEAP_XMAX_UNLOGGED;
@@ -1645,7 +1645,7 @@
     }
     else
     {
-        oldtup.t_data->t_infomask &= ~(HEAP_XMAX_COMMITTED |
+        oldtup.t_data->t_infomask &= ~(HEAP_XMAX_COMMITTED | HEAP_MOVED |
                                        HEAP_XMAX_INVALID |
                                        HEAP_MARKED_FOR_UPDATE);
         HeapTupleHeaderSetXmax(oldtup.t_data, GetCurrentTransactionId());
@@ -1816,7 +1816,7 @@
     ((PageHeader) BufferGetPage(*buffer))->pd_sui = ThisStartUpID;

     /* store transaction information of xact marking the tuple */
-    tuple->t_data->t_infomask &= ~(HEAP_XMAX_COMMITTED | HEAP_XMAX_INVALID);
+    tuple->t_data->t_infomask &= ~(HEAP_XMAX_COMMITTED | HEAP_XMAX_INVALID | HEAP_MOVED);
     tuple->t_data->t_infomask |= HEAP_MARKED_FOR_UPDATE;
     HeapTupleHeaderSetXmax(tuple->t_data, GetCurrentTransactionId());
     HeapTupleHeaderSetCmax(tuple->t_data, cid);
@@ -2147,7 +2147,7 @@

     if (redo)
     {
-        htup->t_infomask &= ~(HEAP_XMAX_COMMITTED |
+        htup->t_infomask &= ~(HEAP_XMAX_COMMITTED | HEAP_MOVED |
                               HEAP_XMAX_INVALID | HEAP_MARKED_FOR_UPDATE);
         HeapTupleHeaderSetXmax(htup, record->xl_xid);
         HeapTupleHeaderSetCmax(htup, FirstCommandId);
@@ -2320,7 +2320,7 @@
         }
         else
         {
-            htup->t_infomask &= ~(HEAP_XMAX_COMMITTED |
+            htup->t_infomask &= ~(HEAP_XMAX_COMMITTED | HEAP_MOVED |
                              HEAP_XMAX_INVALID | HEAP_MARKED_FOR_UPDATE);
             HeapTupleHeaderSetXmax(htup, record->xl_xid);
             HeapTupleHeaderSetCmax(htup, FirstCommandId);