Re: Removing PD_ALL_VISIBLE - Mailing list pgsql-hackers

From Jeff Davis
Subject Re: Removing PD_ALL_VISIBLE
Date
Msg-id 1358759417.5162.145.camel@jdavis-laptop
Whole thread Raw
In response to Re: Removing PD_ALL_VISIBLE  (Pavan Deolasee <pavan.deolasee@gmail.com>)
Responses Re: Removing PD_ALL_VISIBLE
List pgsql-hackers
On Mon, 2013-01-21 at 12:49 +0530, Pavan Deolasee wrote:

> At the minimum your patch will need to have one additional buffer
> pinned for every K < 8192 * 8 heap pages.

I assume it's the same K I referred to when responding to Robert: the
max number of heap buffers we read before we unpin and repin the VM
buffer. Right now it's unlimited, because there is currently no need to
have it at all -- I was just offering the solution in case we did want
to do VM page cleanup in the future or something.

>  For most cases, the value of K will be large enough to ignore the
> overhead, but for small values of K, I'm not sure if we can say that
> with confidence.

It's a constant, and we can easily set it high enough that it wouldn't
matter. There's no reason at all to choose a small value for K. Consider
that an index root page pin is held for the entire scan, and we don't
have a problem with that.

> Of course, for very small tables the real contention might be
> somewhere else and so this change may not show up anything on the
> benchmarks. Doesn't your own tests for 33 page tables confirm that
> there is indeed contention for this case ? I agree its not huge, but I
> don't know if there are workloads where it will matter.

That appears to happen because of the fraction of pinned pages being too
high (aside: it was fairly challenging to construct a test where that
happened). I believe it was mostly solved by adding a threshold to use
the VM, and I can probably solve it completely by doing some more
experiments and finding a better threshold value.
>         
>         I understand that my patch is probably not going in, 
> 
> Sorry about that. I know how that feels. But we need some more reasons
> to justify this change, especially because the performance numbers
> themselves are not showing any signs either way.

That confuses me. The testing was to show it didn't hurt other workloads
(like scans or inserts/updates/deletes); so the best possible result is
that they don't show signs either way.

But yes, I see that others are not interested in the benefits offered by
the patch, which is why I'm giving up on it. If people are concerned
about the costs, then I can fix those; but there's nothing I can do to
increase the benefits.

Regards,Jeff Davis





pgsql-hackers by date:

Previous
From: Xi Wang
Date:
Subject: Re: [PATCH] Fix NULL checking in check_TSCurrentConfig()
Next
From: Craig Ringer
Date:
Subject: Re: Visual Studio 2012 RC