public inbox for [email protected]
help / color / mirror / Atom feedFrom: Thomas Munro <[email protected]>
To: Maxim Orlov <[email protected]>
Cc: vignesh C <[email protected]>
Cc: Palak Chaturvedi <[email protected]>
Cc: Jim Nasby <[email protected]>
Cc: [email protected]
Cc: Nitin Jadhav <[email protected]>
Subject: Re: Extension Enhancement: Buffer Invalidation in pg_buffercache
Date: Thu, 4 Apr 2024 16:22:17 +1300
Message-ID: <CA+hUKGKGyVjqoaSC7AZrgYKHAk9Nid9G4Ao3ZtL_OtLZDb+_cA@mail.gmail.com> (raw)
In-Reply-To: <CACG=ezbBCXaP_KH3N15_tHSwAcCmHQjTyOVsd2o5qG6B_dPzPg@mail.gmail.com>
References: <CALfch19pW48ZwWzUoRSpsaV9hqt0UPyaBPC4bOZ4W+c7FF566A@mail.gmail.com>
<CAMm1aWbk-0C460G9ooy2yNSy1riecqvw4F7jPnqvToFZ_s4oGw@mail.gmail.com>
<CALfch1890bL5SddyOpZDC8d-jZFOUGCGbOr+_11jLTwwJ_WEOA@mail.gmail.com>
<CALfch1_s-X3wxeQbYxhXVuWZKBeY5YvU1D+nJxe21ri5iv0qrw@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
<CALDaNm20+pri5yvEF-4HZqCwRjHB+qjWHLcfanyEjnwgF96z6Q@mail.gmail.com>
<CA+hUKGL9Y3RgK=SjzSwvUnS4-5RcFEXJ3fwqbLCa9bo8g4uOOg@mail.gmail.com>
<CACG=ezbBCXaP_KH3N15_tHSwAcCmHQjTyOVsd2o5qG6B_dPzPg@mail.gmail.com>
On Fri, Mar 8, 2024 at 6:20 AM Maxim Orlov <[email protected]> wrote:
> Quite an interesting patch, in my opinion. I've decided to work on it a bit, did some refactoring (sorry) and add
> basic tests. Also, I try to take into account as much as possible notes on the patch, mentioned by Cédric Villemain.
Thanks! Unfortunately I don't think it's possible to include a
regression test that looks at the output, because it'd be
non-deterministic. Any other backend could pin or dirty the buffer
you try to evict, changing the behaviour.
> > and maybe better to go with FlushOneBuffer() ?
> It's a good idea, but I'm not sure at the moment. I'll try to dig some deeper into it. At least, FlushOneBuffer does
> not work for a local buffers. So, we have to decide whatever pg_buffercache_invalidate should or should not
> work for local buffers. For now, I don't see why it should not. Maybe I miss something?
I think it's OK to ignore local buffers for now. pg_buffercache
generally doesn't support/show them so I don't feel inclined to
support them for this. I removed a few traces of local support.
It didn't seem appropriate to use the pg_monitor role for this, so I
made it superuser-only. I don't think it makes much sense to use this
on any kind of production system so I don't think we need a new role
for it, and existing roles don't seem too appropriate. pageinspect et
al use the same approach.
I added a VOLATILE qualifier to the function.
I added some documentation.
I changed the name to pg_buffercache_evict().
I got rid of the 'force' flag which was used to say 'I only want to
evict this buffer it is clean'. I don't really see the point in that,
we might as well keep it simple. You could filter buffers on
"isdirty" if you want.
I added comments to scare anyone off using EvictBuffer() for anything
much, and marking it as something for developer convenience. (I am
aware of an experimental patch that uses this same function as part of
a buffer pool resizing operation, but that has other infrastructure to
make that safe and would adjust those remarks accordingly.)
I wondered whether it should really be testing for BM_TAG_VALID
rather than BM_VALID. Arguably, but it doesn't seem important for
now. The distinction would arise if someone had tried to read in a
buffer, got an I/O error and abandoned ship, leaving a buffer with a
valid tag but not valid contents. Anyone who tries to ReadBuffer() it
will then try to read it again, but in the meantime this function
won't be able to evict it (it'll just return false). Doesn't seem
that obvious to me that this obscure case needs to be handled. That
doesn't happen *during* a non-error case, because then it's pinned and
we already return false in this code for pins.
I contemplated whether InvalidateBuffer() or InvalidateVictimBuffer()
would be better here and realised that Andres's intuition was probably
right when he suggested the latter up-thread. It is designed with the
right sort of arbitrary concurrent activity in mind, where the former
assumes things about locking and dropping, which could get us into
trouble if not now maybe in the future.
I ran the following diabolical buffer blaster loop while repeatedly
running installcheck:
do
$$
begin
loop
perform pg_buffercache_evict(bufferid)
from pg_buffercache
where random() <= 0.25;
end loop;
End;
$$;
The only ill-effect was a hot laptop.
Thoughts, objections, etc?
Very simple example of use:
create or replace function uncache_relation(name text)
returns boolean
begin atomic;
select bool_and(pg_buffercache_evict(bufferid))
from pg_buffercache
where reldatabase = (select oid
from pg_database
where datname = current_database())
and relfilenode = pg_relation_filenode(name);
end;
More interesting for those of us hacking on streaming I/O stuff was
the ability to evict just parts of things and see how the I/O merging
and I/O depth react.
Attachments:
[application/octet-stream] v5-0001-Add-pg_buffercache_evict-function-for-testing.patch (9.4K, ../CA+hUKGKGyVjqoaSC7AZrgYKHAk9Nid9G4Ao3ZtL_OtLZDb+_cA@mail.gmail.com/2-v5-0001-Add-pg_buffercache_evict-function-for-testing.patch)
download | inline diff:
From 8d6f0abb79e5986a77d2ba90a8e8b9d7b2be36af Mon Sep 17 00:00:00 2001
From: Palak <[email protected]>
Date: Fri, 30 Jun 2023 08:21:06 +0000
Subject: [PATCH v5] Add pg_buffercache_evict() function for testing.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
When testing buffer pool logic, it is very useful to be able to evict
arbitrary pages. It's a pretty crude tool but can be used in SQL
queries on the pg_buffercache view to express a wide range of buffer
invalidation scenarios. Superuser-only.
Author: Palak Chaturvedi <[email protected]>
Author: Thomas Munro <[email protected]> (small adjustments)
Reviewed-by: Nitin Jadhav <[email protected]>
Reviewed-by: Andres Freund <[email protected]>
Reviewed-by: Cary Huang <[email protected]>
Reviewed-by: Cédric Villemain <[email protected]>
Reviewed-by: Jim Nasby <[email protected]>
Reviewed-by: Maxim Orlov <[email protected]>
Reviewed-by: Thomas Munro <[email protected]>
Discussion: https://postgr.es/m/CALfch19pW48ZwWzUoRSpsaV9hqt0UPyaBPC4bOZ4W+c7FF566A@mail.gmail.com
---
contrib/pg_buffercache/Makefile | 2 +-
contrib/pg_buffercache/meson.build | 1 +
.../pg_buffercache--1.4--1.5.sql | 6 ++
contrib/pg_buffercache/pg_buffercache.control | 2 +-
contrib/pg_buffercache/pg_buffercache_pages.c | 20 +++++++
doc/src/sgml/pgbuffercache.sgml | 37 ++++++++++--
src/backend/storage/buffer/bufmgr.c | 57 +++++++++++++++++++
src/include/storage/bufmgr.h | 2 +
8 files changed, 119 insertions(+), 8 deletions(-)
create mode 100644 contrib/pg_buffercache/pg_buffercache--1.4--1.5.sql
diff --git a/contrib/pg_buffercache/Makefile b/contrib/pg_buffercache/Makefile
index d6b58d4da9..eae65ead9e 100644
--- a/contrib/pg_buffercache/Makefile
+++ b/contrib/pg_buffercache/Makefile
@@ -8,7 +8,7 @@ OBJS = \
EXTENSION = pg_buffercache
DATA = pg_buffercache--1.2.sql pg_buffercache--1.2--1.3.sql \
pg_buffercache--1.1--1.2.sql pg_buffercache--1.0--1.1.sql \
- pg_buffercache--1.3--1.4.sql
+ pg_buffercache--1.3--1.4.sql pg_buffercache--1.4--1.5.sql
PGFILEDESC = "pg_buffercache - monitoring of shared buffer cache in real-time"
REGRESS = pg_buffercache
diff --git a/contrib/pg_buffercache/meson.build b/contrib/pg_buffercache/meson.build
index c86e33cc95..1ca3452918 100644
--- a/contrib/pg_buffercache/meson.build
+++ b/contrib/pg_buffercache/meson.build
@@ -22,6 +22,7 @@ install_data(
'pg_buffercache--1.2--1.3.sql',
'pg_buffercache--1.2.sql',
'pg_buffercache--1.3--1.4.sql',
+ 'pg_buffercache--1.4--1.5.sql',
'pg_buffercache.control',
kwargs: contrib_data_args,
)
diff --git a/contrib/pg_buffercache/pg_buffercache--1.4--1.5.sql b/contrib/pg_buffercache/pg_buffercache--1.4--1.5.sql
new file mode 100644
index 0000000000..0fb18ff786
--- /dev/null
+++ b/contrib/pg_buffercache/pg_buffercache--1.4--1.5.sql
@@ -0,0 +1,6 @@
+\echo Use "ALTER EXTENSION pg_buffercache UPDATE TO '1.5'" to load this file. \quit
+
+CREATE FUNCTION pg_buffercache_evict(IN int)
+RETURNS bool
+AS 'MODULE_PATHNAME', 'pg_buffercache_evict'
+LANGUAGE C PARALLEL SAFE VOLATILE STRICT;
diff --git a/contrib/pg_buffercache/pg_buffercache.control b/contrib/pg_buffercache/pg_buffercache.control
index a82ae5f9bb..5ee875f77d 100644
--- a/contrib/pg_buffercache/pg_buffercache.control
+++ b/contrib/pg_buffercache/pg_buffercache.control
@@ -1,5 +1,5 @@
# pg_buffercache extension
comment = 'examine the shared buffer cache'
-default_version = '1.4'
+default_version = '1.5'
module_pathname = '$libdir/pg_buffercache'
relocatable = true
diff --git a/contrib/pg_buffercache/pg_buffercache_pages.c b/contrib/pg_buffercache/pg_buffercache_pages.c
index 3316732365..fc33dffeeb 100644
--- a/contrib/pg_buffercache/pg_buffercache_pages.c
+++ b/contrib/pg_buffercache/pg_buffercache_pages.c
@@ -63,6 +63,7 @@ typedef struct
PG_FUNCTION_INFO_V1(pg_buffercache_pages);
PG_FUNCTION_INFO_V1(pg_buffercache_summary);
PG_FUNCTION_INFO_V1(pg_buffercache_usage_counts);
+PG_FUNCTION_INFO_V1(pg_buffercache_evict);
Datum
pg_buffercache_pages(PG_FUNCTION_ARGS)
@@ -347,3 +348,22 @@ pg_buffercache_usage_counts(PG_FUNCTION_ARGS)
return (Datum) 0;
}
+
+/*
+ * Try to evict a block from a shared buffer.
+ */
+Datum
+pg_buffercache_evict(PG_FUNCTION_ARGS)
+{
+ Buffer buf = PG_GETARG_INT32(0);
+
+ if (!superuser())
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("must be superuser to use pg_buffercache_evict function")));
+
+ if (buf > NBuffers || buf < -NLocBuffer || BufferIsInvalid(buf))
+ elog(ERROR, "bad buffer ID: %d", buf);
+
+ PG_RETURN_BOOL(EvictBuffer(buf));
+}
diff --git a/doc/src/sgml/pgbuffercache.sgml b/doc/src/sgml/pgbuffercache.sgml
index afe2d97834..0aaffd5786 100644
--- a/doc/src/sgml/pgbuffercache.sgml
+++ b/doc/src/sgml/pgbuffercache.sgml
@@ -21,11 +21,16 @@
<primary>pg_buffercache_summary</primary>
</indexterm>
+ <indexterm>
+ <primary>pg_buffercache_evict</primary>
+ </indexterm>
+
<para>
This module provides the <function>pg_buffercache_pages()</function>
function (wrapped in the <structname>pg_buffercache</structname> view),
- the <function>pg_buffercache_summary()</function> function, and the
- <function>pg_buffercache_usage_counts()</function> function.
+ the <function>pg_buffercache_summary()</function> function, the
+ <function>pg_buffercache_usage_counts()</function> function and
+ the <function>pg_buffercache_evict()</function> function.
</para>
<para>
@@ -47,9 +52,15 @@
</para>
<para>
- By default, use is restricted to superusers and roles with privileges of the
- <literal>pg_monitor</literal> role. Access may be granted to others
- using <command>GRANT</command>.
+ By default, use of the above functions is restricted to superusers and roles
+ with privileges of the <literal>pg_monitor</literal> role. Access may be
+ granted to others using <command>GRANT</command>.
+ </para>
+
+ <para>
+ The <function>pg_buffercache_evict()</function> function allows a block to
+ be evicted from the buffer pool given a buffer identifier. Use of this
+ function is restricted to superusers only.
</para>
<sect2 id="pgbuffercache-pg-buffercache">
@@ -351,7 +362,21 @@
</para>
</sect2>
- <sect2 id="pgbuffercache-sample-output">
+ <sect2 id="pgbuffercache-pg-buffercache-evict">
+ <title>The <structname>pg_buffercache_evict</structname> Function</title>
+ <para>
+ The <function>pg_buffercache_evict()</function> takes a buffer identifier
+ from the <structfield>bufferid</structfield> of the
+ <structname>pg_buffercache</structname> view. It returns true on success, and
+ false if the buffer wasn't valid, couldn't be evicted because it was pinned, or
+ became dirty again after an attempt to write it out. The result is
+ immediately out of date upon return, as the buffer might become valid again at
+ any time due to concurrent activity. The function is intended for developer
+ testing.
+ </para>
+ </sect2>
+
+<sect2 id="pgbuffercache-sample-output">
<title>Sample Output</title>
<screen>
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 06e9ffd2b0..673d596b26 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -6003,3 +6003,60 @@ ResOwnerPrintBufferPin(Datum res)
{
return DebugPrintBufferRefcount(DatumGetInt32(res));
}
+
+/*
+ * Try to evict a shared a buffer.
+ *
+ * This function is intended for testing/development use only. By the time it
+ * returns, the buffer may again be occupied, perhaps even by the same block.
+ *
+ * Returns true if the buffer was valid and it has now been made invalid.
+ * Returns false if the wasn't valid, or it couldn't be evicted due to a pin,
+ * or if the buffer becomes dirty again while we're trying to write it out.
+ */
+bool
+EvictBuffer(Buffer buf)
+{
+ BufferDesc *desc;
+ uint32 buf_state;
+ bool result;
+
+ /* Make sure we can pin the buffer if it turned out require writing. */
+ ReservePrivateRefCountEntry();
+
+ Assert(!BufferIsLocal(buf));
+ desc = GetBufferDescriptor(buf - 1);
+
+ /* Lock the header and check if it's valid. */
+ buf_state = LockBufHdr(desc);
+ if ((buf_state & BM_VALID) == 0)
+ {
+ UnlockBufHdr(desc, buf_state);
+ return false;
+ }
+
+ /* Check that it's not pinned by someone else. */
+ if (BUF_STATE_GET_REFCOUNT(buf_state) > 0)
+ {
+ UnlockBufHdr(desc, buf_state);
+ return false;
+ }
+
+ PinBuffer_Locked(desc); /* releases spinlock */
+
+ /* If it was dirty, try to clean it once. */
+ if (buf_state & BM_DIRTY)
+ {
+ ResourceOwnerEnlarge(CurrentResourceOwner);
+ LWLockAcquire(BufferDescriptorGetContentLock(desc), LW_SHARED);
+ FlushBuffer(desc, NULL, IOOBJECT_RELATION, IOCONTEXT_NORMAL);
+ LWLockRelease(BufferDescriptorGetContentLock(desc));
+ }
+
+ /* This will return false if it becomes dirty or someone else pins it. */
+ result = InvalidateVictimBuffer(desc);
+
+ UnpinBuffer(desc);
+
+ return result;
+}
diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h
index f380f9d9a6..f278a697c3 100644
--- a/src/include/storage/bufmgr.h
+++ b/src/include/storage/bufmgr.h
@@ -305,6 +305,8 @@ extern bool BgBufferSync(struct WritebackContext *wb_context);
extern void LimitAdditionalPins(uint32 *additional_pins);
extern void LimitAdditionalLocalPins(uint32 *additional_pins);
+extern bool EvictBuffer(Buffer buf);
+
/* in buf_init.c */
extern void InitBufferPool(void);
extern Size BufferShmemSize(void);
--
2.39.3 (Apple Git-146)
view thread (8+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Extension Enhancement: Buffer Invalidation in pg_buffercache
In-Reply-To: <CA+hUKGKGyVjqoaSC7AZrgYKHAk9Nid9G4Ao3ZtL_OtLZDb+_cA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox