On 09/29/2016 02:45 AM, Ivan Kartyshov wrote:
> Secondly, I see this bit added to the loop over buffers:
>
> if (bufHdr->tag.forkNum == -1)
> {
> fctx->record[i].blocknum = InvalidBlockNumber;
> continue;
> }
>
> and I have no idea why this is needed (when it was not needed before).
This helps to skip not used bufferpages. It is valuable on big and not
warmup shared memory.
That seems like an unrelated change. Checking for forkNum, instead of
e.g. BM_VALID seems a bit surprising, too.
On 09/02/2016 12:01 PM, Robert Haas wrote:
I think we certainly want to lock the buffer header, because otherwise
we might get a torn read of the buffer tag, which doesn't seem good.
But it's not obvious to me that there's any point in taking the lock
on the buffer mapping partition; I'm thinking that doesn't really do
anything unless we lock them all, and we all seem to agree that's
going too far.
Replace consistent method with semiconsistent (that lock buffer headers
without partition lock). Made some additional fixes thanks to reviewers.
This patch also does:
+++ b/contrib/pg_buffercache/pg_buffercache--1.2--1.3.sql
@@ -0,0 +1,6 @@
+/* contrib/pg_buffercache/pg_buffercache--1.2--1.3.sql */
+
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION pg_buffercache(text) UPDATE TO '1.3'" to
load this file. \quit
+
+ALTER FUNCTION pg_buffercache_pages() PARALLEL SAFE;
Why? That seems unrelated to what's been discussed in this thread.
I have committed the straightforward part of this, removing the
partition-locking. I think we're done here for this commitfest, but feel
free to post new patches for that PARALLEL SAFE and the quick-check for
unused buffers, if you think it's worthwhile.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
