Re: [HACKERS] Trivial patch to double vacuum speed

From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Gregory Stark <stark(at)enterprisedb(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)postgresql(dot)org, pgsql-patches(at)postgresql(dot)org
Subject: Re: [HACKERS] Trivial patch to double vacuum speed
Date: 2006-09-04 21:40:25
Message-ID: 200609042140.k84LePD03774@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches


Patch applied. Thanks.

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

Gregory Stark wrote:
>
> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
>
> > The reason the patch is so short is that it's a kluge. If we really
> > cared about supporting this case, more wide-ranging changes would be
> > needed (eg, there's no need to eat maintenance_work_mem worth of RAM
> > for the dead-TIDs array); and a decent respect to the opinions of
> > mankind would require some attention to updating the header comments
> > and function descriptions, too.
>
> The only part that seems klugy to me is how it releases the lock and
> reacquires it rather than wait in the first place until it can acquire the
> lock. Fixed that and changed lazy_space_alloc to allocate only as much space
> as is really necessary.
>
> Gosh, I've never been accused of offending all mankind before.
>
>
>
> --- vacuumlazy.c 31 Jul 2006 21:09:00 +0100 1.76
> +++ vacuumlazy.c 28 Aug 2006 09:58:41 +0100
> @@ -16,6 +16,10 @@
> * perform a pass of index cleanup and page compaction, then resume the heap
> * scan with an empty TID array.
> *
> + * As a special exception if we're processing a table with no indexes we can
> + * vacuum each page as we go so we don't need to allocate more space than
> + * enough to hold as many heap tuples fit on one page.
> + *
> * We can limit the storage for page free space to MaxFSMPages entries,
> * since that's the most the free space map will be willing to remember
> * anyway. If the relation has fewer than that many pages with free space,
> @@ -106,7 +110,7 @@
> TransactionId OldestXmin);
> static BlockNumber count_nondeletable_pages(Relation onerel,
> LVRelStats *vacrelstats, TransactionId OldestXmin);
> -static void lazy_space_alloc(LVRelStats *vacrelstats, BlockNumber relblocks);
> +static void lazy_space_alloc(LVRelStats *vacrelstats, BlockNumber relblocks, unsigned nindexes);
> static void lazy_record_dead_tuple(LVRelStats *vacrelstats,
> ItemPointer itemptr);
> static void lazy_record_free_space(LVRelStats *vacrelstats,
> @@ -206,7 +210,8 @@
> * This routine sets commit status bits, builds lists of dead tuples
> * and pages with free space, and calculates statistics on the number
> * of live tuples in the heap. When done, or when we run low on space
> - * for dead-tuple TIDs, invoke vacuuming of indexes and heap.
> + * for dead-tuple TIDs, or after every page if the table has no indexes
> + * invoke vacuuming of indexes and heap.
> *
> * It also updates the minimum Xid found anywhere on the table in
> * vacrelstats->minxid, for later storing it in pg_class.relminxid.
> @@ -247,7 +252,7 @@
> vacrelstats->rel_pages = nblocks;
> vacrelstats->nonempty_pages = 0;
>
> - lazy_space_alloc(vacrelstats, nblocks);
> + lazy_space_alloc(vacrelstats, nblocks, nindexes);
>
> for (blkno = 0; blkno < nblocks; blkno++)
> {
> @@ -282,8 +287,14 @@
>
> buf = ReadBuffer(onerel, blkno);
>
> - /* In this phase we only need shared access to the buffer */
> - LockBuffer(buf, BUFFER_LOCK_SHARE);
> + /* In this phase we only need shared access to the buffer unless we're
> + * going to do the vacuuming now which we do if there are no indexes
> + */
> +
> + if (nindexes)
> + LockBuffer(buf, BUFFER_LOCK_SHARE);
> + else
> + LockBufferForCleanup(buf);
>
> page = BufferGetPage(buf);
>
> @@ -450,6 +461,12 @@
> {
> lazy_record_free_space(vacrelstats, blkno,
> PageGetFreeSpace(page));
> + } else if (!nindexes) {
> + /* If there are no indexes we can vacuum the page right now instead
> + * of doing a second scan */
> + lazy_vacuum_page(onerel, blkno, buf, 0, vacrelstats);
> + lazy_record_free_space(vacrelstats, blkno, PageGetFreeSpace(BufferGetPage(buf)));
> + vacrelstats->num_dead_tuples = 0;
> }
>
> /* Remember the location of the last page with nonremovable tuples */
> @@ -891,16 +908,20 @@
> * See the comments at the head of this file for rationale.
> */
> static void
> -lazy_space_alloc(LVRelStats *vacrelstats, BlockNumber relblocks)
> +lazy_space_alloc(LVRelStats *vacrelstats, BlockNumber relblocks, unsigned nindexes)
> {
> long maxtuples;
> int maxpages;
>
> - maxtuples = (maintenance_work_mem * 1024L) / sizeof(ItemPointerData);
> - maxtuples = Min(maxtuples, INT_MAX);
> - maxtuples = Min(maxtuples, MaxAllocSize / sizeof(ItemPointerData));
> - /* stay sane if small maintenance_work_mem */
> - maxtuples = Max(maxtuples, MaxHeapTuplesPerPage);
> + if (nindexes) {
> + maxtuples = (maintenance_work_mem * 1024L) / sizeof(ItemPointerData);
> + maxtuples = Min(maxtuples, INT_MAX);
> + maxtuples = Min(maxtuples, MaxAllocSize / sizeof(ItemPointerData));
> + /* stay sane if small maintenance_work_mem */
> + maxtuples = Max(maxtuples, MaxHeapTuplesPerPage);
> + } else {
> + maxtuples = MaxHeapTuplesPerPage;
> + }
>
> vacrelstats->num_dead_tuples = 0;
> vacrelstats->max_dead_tuples = (int) maxtuples;
> --
> Gregory Stark
> EnterpriseDB http://www.enterprisedb.com
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Have you searched our list archives?
>
> http://archives.postgresql.org

--
Bruce Momjian bruce(at)momjian(dot)us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2006-09-04 21:43:43 Re: [PATCHES] Documentation fix for --with-ldap
Previous Message Bruce Momjian 2006-09-04 21:39:03 Re: [COMMITTERS] pgsql: Sequences were not being shown due to

Browse pgsql-patches by date

  From Date Subject
Next Message Tom Lane 2006-09-04 21:45:28 Re: [HACKERS] Trivial patch to double vacuum speed on tables with no indexes
Previous Message Bruce Momjian 2006-09-04 21:32:50 Re: plpgsql, return can contains any expression