public inbox for [email protected]
help / color / mirror / Atom feedFrom: Palak Chaturvedi <[email protected]>
To: Thomas Munro <[email protected]>
To: [email protected]
Subject: Re: Extension Enhancement: Buffer Invalidation in pg_buffercache
Date: Mon, 3 Jul 2023 13:56:29 +0530
Message-ID: <CALfch1_cu_uurBGVG+GO_AtyyYxnuLU5GWf0ahHMre_HDqGDyw@mail.gmail.com> (raw)
In-Reply-To: <CA+hUKG+i3Dnsv_M+Opk6tTv96w8ZfWFPQAYScj_LRtvw=qx0+A@mail.gmail.com>
References: <CALfch19pW48ZwWzUoRSpsaV9hqt0UPyaBPC4bOZ4W+c7FF566A@mail.gmail.com>
<CA+hUKG+i3Dnsv_M+Opk6tTv96w8ZfWFPQAYScj_LRtvw=qx0+A@mail.gmail.com>
Hi Thomas,
Thank you for your suggestions. I have added the sql in the meson
build as well.
On Sat, 1 Jul 2023 at 03:39, Thomas Munro <[email protected]> wrote:
>
> On Fri, Jun 30, 2023 at 10:47 PM Palak Chaturvedi
> <[email protected]> wrote:
> > pgbench=# select count(pg_buffercache_invalidate(bufferid)) from
> > pg_buffercache where relfilenode =
> > pg_relation_filenode('pgbench_accounts'::regclass);
>
> Hi Palak,
>
> Thanks for working on this! I think this will be very useful for
> testing existing workloads but also for testing future work on
> prefetching with AIO (and DIO), work on putting SLRUs (or anything
> else) into the buffer pool, nearby proposals for caching buffer
> mapping information, etc etc.
>
> Palak and I talked about this idea a bit last week (stimulated by a
> recent thread[1], but the topic has certainly come up before), and we
> discussed some different ways one could specify which pages are
> dropped. For example, perhaps the pg_prewarm extension could have an
> 'unwarm' option instead. I personally thought the buffer ID-based
> approach was quite good because it's extremely simple, while giving
> the user the full power of SQL to say which buffers. Half a table?
> Visibility map? Everything? Root page of an index? I think that's
> probably better than something that requires more code and
> complication but is less flexible in the end. It feels like the right
> level of rawness for something primarily of interest to hackers and
> advanced users. I don't think it matters that there is a window
> between selecting a buffer ID and invalidating it, for the intended
> use cases. That's my vote, anyway, let's see if others have other
> ideas...
>
> We also talked a bit about how one might control the kernel page cache
> in more fine-grained ways for testing purposes, but it seems like the
> pgfincore project has that covered with its pgfadvise_willneed() and
> pgfadvise_dontneed(). IMHO that project could use more page-oriented
> operations (instead of just counts and coarse grains operations) but
> that's something that could be material for patches to send to the
> extension maintainers. This work, in contrast, is more tangled up
> with bufmgr.c internals, so it feels like this feature belongs in a
> core contrib module.
>
> Some initial thoughts on the patch:
>
> I wonder if we should include a simple exercise in
> contrib/pg_buffercache/sql/pg_buffercache.sql. One problem is that
> it's not guaranteed to succeed in general. It doesn't wait for pins
> to go away, and it doesn't retry cleaning dirty buffers after one
> attempt, it just returns false, which I think is probably the right
> approach, but it makes the behaviour too non-deterministic for simple
> tests. Perhaps it's enough to include an exercise where we call it a
> few times to hit a couple of cases, but not verify what effect it has.
>
> It should be restricted by role, but I wonder which role it should be.
> Testing for superuser is now out of fashion.
>
> Where the Makefile mentions 1.4--1.5.sql, the meson.build file needs
> to do the same. That's because PostgreSQL is currently in transition
> from autoconf/gmake to meson/ninja[2], so for now we have to maintain
> both build systems. That's why it fails to build in some CI tasks[3].
> You can enable CI in your own GitHub account if you want to run test
> builds on several operating systems, see [4] for info.
>
> [1] https://www.postgresql.org/message-id/flat/CAFSGpE3y_oMK1uHhcHxGxBxs%2BKrjMMdGrE%2B6HHOu0vttVET0UQ%4...
> [2] https://wiki.postgresql.org/wiki/Meson
> [3] http://cfbot.cputube.org/palak-chaturvedi.html
> [4] https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob_plain;f=src/tools/ci/README;hb=HEAD
Attachments:
[application/octet-stream] v1-0001-Invalidate-Buffer-By-Bufnum.patch (5.9K, ../CALfch1_cu_uurBGVG+GO_AtyyYxnuLU5GWf0ahHMre_HDqGDyw@mail.gmail.com/2-v1-0001-Invalidate-Buffer-By-Bufnum.patch)
download | inline diff:
From 02ea352d84ed87e156617de8b8020811680cb412 Mon Sep 17 00:00:00 2001
From: Palak <[email protected]>
Date: Fri, 30 Jun 2023 08:21:06 +0000
Subject: [PATCH v1] Invalidate Buffer By Bufnum
---
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 | 27 ++++++++
src/backend/storage/buffer/bufmgr.c | 64 +++++++++++++++++++
src/include/storage/bufmgr.h | 3 +
7 files changed, 103 insertions(+), 2 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 c51edf37d1..748463bc19 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..c7e47456d7
--- /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_invalidate(IN int)
+RETURNS bool
+AS 'MODULE_PATHNAME', 'pg_buffercache_invalidate'
+LANGUAGE C PARALLEL SAFE;
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..6dd02fe6af 100644
--- a/contrib/pg_buffercache/pg_buffercache_pages.c
+++ b/contrib/pg_buffercache/pg_buffercache_pages.c
@@ -64,6 +64,33 @@ 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_invalidate);
+Datum
+pg_buffercache_invalidate(PG_FUNCTION_ARGS)
+{
+ Buffer bufnum;
+ bool result;
+
+ if (PG_ARGISNULL(0))
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("buffernum cannot be NULL")));
+ }
+
+ bufnum = PG_GETARG_INT32(0);
+ if (bufnum < 0 || bufnum > NBuffers)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("buffernum is not valid")));
+
+ }
+
+ result = TryInvalidateBuffer(bufnum);
+ PG_RETURN_BOOL(result);
+}
+
Datum
pg_buffercache_pages(PG_FUNCTION_ARGS)
{
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 3c59bbd04e..376afcf996 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -5592,3 +5592,67 @@ TestForOldSnapshot_impl(Snapshot snapshot, Relation relation)
(errcode(ERRCODE_SNAPSHOT_TOO_OLD),
errmsg("snapshot too old")));
}
+
+/*
+Try Invalidating a buffer using bufnum.
+If the buffer is invalid, the function returns false.
+The function checks for dirty buffer and flushes the dirty buffer before invalidating.
+If the buffer is still dirty it returns false.
+*/
+bool
+TryInvalidateBuffer(Buffer bufnum)
+{
+ BufferDesc *bufHdr = GetBufferDescriptor(bufnum - 1);
+ uint32 buf_state;
+
+ ReservePrivateRefCountEntry();
+
+ buf_state = LockBufHdr(bufHdr);
+ if ((buf_state & BM_VALID) == BM_VALID)
+ {
+ /*
+ * The buffer is pinned therefore cannot invalidate.
+ */
+ if (BUF_STATE_GET_REFCOUNT(buf_state) > 0)
+ {
+ UnlockBufHdr(bufHdr, buf_state);
+ return false;
+ }
+ if ((buf_state & BM_DIRTY) == BM_DIRTY)
+ {
+ /*
+ * Try once to flush the dirty buffer.
+ */
+ PinBuffer_Locked(bufHdr);
+ LWLockAcquire(BufferDescriptorGetContentLock(bufHdr), LW_SHARED);
+ FlushBuffer(bufHdr, NULL, IOOBJECT_RELATION, IOCONTEXT_NORMAL);
+ LWLockRelease(BufferDescriptorGetContentLock(bufHdr));
+ UnpinBuffer(bufHdr);
+ buf_state = LockBufHdr(bufHdr);
+ if (BUF_STATE_GET_REFCOUNT(buf_state) > 0)
+ {
+ UnlockBufHdr(bufHdr, buf_state);
+ return false;
+ }
+
+ /*
+ * If its dirty again or not valid anymore give up.
+ */
+
+ if ((buf_state & (BM_DIRTY | BM_VALID)) != (BM_VALID))
+ {
+ UnlockBufHdr(bufHdr, buf_state);
+ return false;
+ }
+
+ }
+
+ InvalidateBuffer(bufHdr);
+ return true;
+ }
+ else
+ {
+ UnlockBufHdr(bufHdr, buf_state);
+ return false;
+ }
+}
diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h
index 0f5fb6be00..4ba3d9089b 100644
--- a/src/include/storage/bufmgr.h
+++ b/src/include/storage/bufmgr.h
@@ -252,6 +252,9 @@ extern bool BgBufferSync(struct WritebackContext *wb_context);
extern void TestForOldSnapshot_impl(Snapshot snapshot, Relation relation);
+
+extern bool TryInvalidateBuffer(Buffer bufnum);
+
/* in buf_init.c */
extern void InitBufferPool(void);
extern Size BufferShmemSize(void);
--
2.25.1
view thread (4+ 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]
Subject: Re: Extension Enhancement: Buffer Invalidation in pg_buffercache
In-Reply-To: <CALfch1_cu_uurBGVG+GO_AtyyYxnuLU5GWf0ahHMre_HDqGDyw@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