Re: [DOCS] Autovacuum and XID wraparound - Mailing list pgsql-patches

Alvaro Herrera <alvherre@commandprompt.com> writes:
> Here is my proposed patch.

Actually, the original patch in this series was fairly horrid, and
things haven't been made better by the subsequent changes.  It lacked
any comment explaining what it was doing; failed to comment on the way
it was abusing heap_freeze_tuple (the latter thinks it's getting a tuple
that's in a disk buffer); and IMHO puts the heap_freeze_tuple call in
the wrong place anyway.  raw_heap_insert has no business editorializing
on the tuple it's given.  It'd be better to have the call in
rewrite_heap_tuple, which is already busy messing with the tuple's
visibility info.  Perhaps like this, in addition to your changes:

*** src/backend/access/heap/rewriteheap.c~    Wed May 16 19:22:55 2007
--- src/backend/access/heap/rewriteheap.c    Wed May 16 19:54:46 2007
***************
*** 292,298 ****
  /*
   * Add a tuple to the new heap.
   *
!  * Visibility information is copied from the original tuple.
   *
   * state        opaque state as returned by begin_heap_rewrite
   * old_tuple    original tuple in the old heap
--- 292,300 ----
  /*
   * Add a tuple to the new heap.
   *
!  * Visibility information is copied from the original tuple, except that
!  * we "freeze" very-old tuples.  Note that since we scribble on new_tuple,
!  * it had better be temp storage not a pointer to the original tuple.
   *
   * state        opaque state as returned by begin_heap_rewrite
   * old_tuple    original tuple in the old heap
***************
*** 324,329 ****
--- 326,342 ----
          old_tuple->t_data->t_infomask & HEAP_XACT_MASK;

      /*
+      * While we have our hands on the tuple, we may as well freeze any
+      * very-old xmin or xmax, so that future VACUUM effort can be saved.
+      *
+      * Note we abuse heap_freeze_tuple() a bit here, since it's expecting
+      * to be given a pointer to a tuple in a disk buffer.  It happens
+      * though that we can get the right things to happen by passing
+      * InvalidBuffer for the buffer.
+      */
+     heap_freeze_tuple(new_tuple->t_data, state->rs_oldest_xmin, InvalidBuffer);
+
+     /*
       * Invalid ctid means that ctid should point to the tuple itself.
       * We'll override it later if the tuple is part of an update chain.
       */
***************
*** 537,544 ****
      Size            len;
      OffsetNumber    newoff;
      HeapTuple        heaptup;
-
-     heap_freeze_tuple(tup->t_data, state->rs_oldest_xmin, InvalidBuffer);

      /*
       * If the new tuple is too big for storage or contains already toasted
--- 550,555 ----


            regards, tom lane