Re: Removing PD_ALL_VISIBLE

On Wed, 2012-11-21 at 18:25 -0800, Jeff Davis wrote:
> Follow up to discussion:
> http://archives.postgresql.org/pgsql-hackers/2012-11/msg00817.php
>
> I worked out a patch that replaces PD_ALL_VISIBLE with calls to
> visibilitymap_test. It rips out a lot of complexity, with a net drop of
> about 300 lines (not a lot, but some of that code is pretty complex).

Updated patch attached.

Now it tries to keep the VM buffer pinned during scans, inserts,
updates, and deletes. This should avoid increased contention pinning the
VM pages, but performance tests are required.

For updates, it currently only tries to hold a pin on the VM buffer for
the page of the original tuple. For HOT updates, that's always the same
as the new buffer anyway. For cold updates, we could also try to keep a
pin on the buffer for the new tuple, but right now I don't see an
obvious need for that complexity. It may plausibly be a problem when
doing a bulk update on a freshly-loaded table.

It occurred to me that it might be difficult to test this patch without
a fairly large test case. A big assumption of my patch is that there
will be locality of access (and the VM page you already have a pin on is
likely to be needed the next time), which is obvious during a scan but
not so obvious during I/U/D. But a single 8K VM page represents some 60K
pages, or about 500MB of data. So anything less than that means that
there is only one VM page, and locality is trivial... it seems like any
test on a table less than 5GB would not be fair.

Then again, if a 5GB table is being randomly accessed, an extra pin is
unlikely to matter. Also, without locality, the contention would not be
nearly as bad either. I'm still pretty unclear what the "worst case" for
this patch is supposed to look like.

Regards,
Jeff Davis