Re: Not HOT enough

Did we want to apply this?

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

On Wed, Nov 23, 2011 at 07:33:18PM +0000, Simon Riggs wrote:
> On Wed, Nov 23, 2011 at 6:15 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> >> The real question is do we favour HOT cleanup on those small 8 tables,
> >> or do we favour HOT cleanup of every other table?
> >
> > No, the real question is why not think a little harder and see if we can
> > come up with a solution that doesn't involve making some cases worse to
> > make others better.
>
> Slightly modified patch attached.
>
> When we access a shared relation and the page is prunable we re-derive
> the cutoff value using GetOldestXmin.
>
> That code path is rarely taken. In particular, pg_shdepend is only
> accessed during object creation/alter/drop, so this isn't a path we
> can't spare a small amount little extra on, just like the trade-off
> we've taken to make locking faster for DML while making DDL a little
> slower.
>
> If this is still unacceptable, then I'll have to look at reducing
> impact on pg_shdepend from temp tables - which is still a plan.
>
> --
>  Simon Riggs                   http://www.2ndQuadrant.com/
>  PostgreSQL Development, 24x7 Support, Training & Services

> diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
> index 61f2ce4..5feaedc 100644
> --- a/src/backend/access/heap/pruneheap.c
> +++ b/src/backend/access/heap/pruneheap.c
> @@ -16,13 +16,13 @@
>
> #include "access/heapam.h"
> #include "access/transam.h"
> +#include "catalog/catalog.h"
> #include "miscadmin.h"
> #include "pgstat.h"
> #include "storage/bufmgr.h"
> #include "utils/rel.h"
> #include "utils/tqual.h"
>
> -
> /* Working data for heap_page_prune and subroutines */
> typedef struct
> {
> @@ -72,6 +72,7 @@ heap_page_prune_opt(Relation relation, Buffer buffer, TransactionId OldestXmin)
> {
> Page page = BufferGetPage(buffer);
> Size minfree;
> + Transactionid CutoffXmin = OldestXmin;
>
> /*
> * Let's see if we really need pruning.
> @@ -91,6 +92,18 @@ heap_page_prune_opt(Relation relation, Buffer buffer, TransactionId OldestXmin)
> return;
>
> /*
> + * We choose to set RecentGlobalXmin only for the current database, which
> + * means we cannot use it to prune shared relations when reading them with
> + * MVCC snapshots. By making that choice it allows our snapshots to be
> + * smaller and faster, and the RecentGlobalXmin will be further forward
> + * and offer better pruning of non-shared relations. So when accessing a
> + * shared relation and we see the page is prunable (above) we get an
> + * OldestXmin across all databases.
> + */
> + if (IsSharedRelation(relation->rd_id))
> + CutoffXmin = GetOldestXmin(true, true);
> +
> + /*
> * We prune when a previous UPDATE failed to find enough space on the page
> * for a new tuple version, or when free space falls below the relation's
> * fill-factor target (but not less than 10%).
> @@ -124,7 +137,7 @@ heap_page_prune_opt(Relation relation, Buffer buffer, TransactionId OldestXmin)
> * needed */
>
> /* OK to prune */
> - (void) heap_page_prune(relation, buffer, OldestXmin, true, &ignore);
> + (void) heap_page_prune(relation, buffer, CutoffXmin, true, &ignore);
> }
>
> /* And release buffer lock */
> diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
> index 1a48485..60153f4 100644
> --- a/src/backend/storage/ipc/procarray.c
> +++ b/src/backend/storage/ipc/procarray.c
> @@ -55,6 +55,7 @@
> #include "storage/spin.h"
> #include "utils/builtins.h"
> #include "utils/snapmgr.h"
> +#include "utils/tqual.h"
>
>
> /* Our shared memory area */
> @@ -1185,6 +1186,8 @@ GetMaxSnapshotSubxidCount(void)
> * RecentGlobalXmin: the global xmin (oldest TransactionXmin across all
> * running transactions, except those running LAZY VACUUM). This is
> * the same computation done by GetOldestXmin(true, true).
> + * For MVCC snapshots we examine transactions running only in our
> + * database, ignoring longer running transactions in other databases.
> *
> * Note: this function should probably not be called with an argument that's
> * not statically allocated (see xip allocation below).
> @@ -1200,9 +1203,12 @@ GetSnapshotData(Snapshot snapshot)
> int count = 0;
> int subcount = 0;
> bool suboverflowed = false;
> + bool allDbs = false;
>
> Assert(snapshot != NULL);
>
> + allDbs = !IsMVCCSnapshot(snapshot);
> +
> /*
> * Allocating space for maxProcs xids is usually overkill; numProcs would
> * be sufficient. But it seems better to do the malloc while not holding
> @@ -1278,6 +1284,12 @@ GetSnapshotData(Snapshot snapshot)
> if (proc->vacuumFlags & PROC_IN_VACUUM)
> continue;
>
> + /* MVCC snapshots ignore other databases */
> + if (!allDbs &&
> + proc->databaseId != MyDatabaseId &&
> + proc->databaseId != 0) /* always include WalSender */
> + continue;
> +
> /* Update globalxmin to be the smallest valid xmin */
> xid = proc->xmin; /* fetch just once */
> if (TransactionIdIsNormal(xid) &&

>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

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

+ It's impossible for everything to be true. +