Re: [DOCS] Autovacuum and XID wraparound

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Cc: Heikki Linnakangas <heikki(at)enterprisedb(dot)com>, Chris Browne <cbbrowne(at)acm(dot)org>, pgsql-patches(at)postgresql(dot)org
Subject: Re: [DOCS] Autovacuum and XID wraparound
Date: 2007-05-17 00:00:41
Message-ID: 9823.1179360041@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-docs pgsql-patches

Alvaro Herrera <alvherre(at)commandprompt(dot)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

In response to

Responses

Browse pgsql-docs by date

  From Date Subject
Next Message Alvaro Herrera 2007-05-17 15:53:19 Re: [DOCS] Autovacuum and XID wraparound
Previous Message Alvaro Herrera 2007-05-16 22:46:27 Re: [DOCS] Autovacuum and XID wraparound

Browse pgsql-patches by date

  From Date Subject
Next Message Bruce Momjian 2007-05-17 00:34:57 Re: [PATCHES] Full page writes improvement, code update
Previous Message Tom Lane 2007-05-16 22:56:44 Re: updated SORT/LIMIT patch