From: Heikki Linnakangas <hlinnaka@iki.fi>
To: Michael Paquier <michael@paquier.xyz>
Cc: feichanghong <feichanghong@qq.com>
Cc: pgsql-bugs <pgsql-bugs@lists.postgresql.org>
Subject: Re: MarkBufferDirty Assert held LW_EXCLUSIVE lock fail when ginFinishSplit
Date: Tue, 23 Jan 2024 09:12:25 +0200
Message-ID: <f5eb802a-e98d-438e-9fa8-1694f767e5ea@iki.fi> (raw)
In-Reply-To: <Za78YVcqNA8nAnn-@paquier.xyz>
References: <tencent_A3CE810F59132D8E230475A5F0F7A08C8307@qq.com>
<053bc845-3cb5-445e-8ea4-00f6c678ee25@iki.fi>
<tencent_0F9D2F03B4F3E2C3DE4483E31B4147C9F605@qq.com>
<d8f0b068-0e6e-4b2c-8932-62507eb7e1c6@iki.fi>
<tencent_BEFB72867BA7CF879B492487284296D2F606@qq.com>
<f8fa079c-c88c-4a4f-9c36-3854d12ddf70@iki.fi>
<Za78YVcqNA8nAnn-@paquier.xyz>
On 23/01/2024 01:38, Michael Paquier wrote:
> On Mon, Jan 22, 2024 at 10:47:17PM +0200, Heikki Linnakangas wrote:
>> Michael, it was a pleasure to write this test with the injection points,
>> compared to trying to set up PITR at just the right moment. Thank you! Since
>> this is the first test that uses it, I didn't have any precedence to
>> copy-paste; can you take a look and verify if this is how you imagined the
>> facility to be used?
>
> I was wondering how many weeks it would take for somebody to begin
> playing with that, and here you are 12 hours later..
:-D
> +EXTRA_INSTALL = src/test/modules/injection_points
> [...]
> +#ifdef USE_INJECTION_POINTS
> + if (GinPageIsLeaf(BufferGetPage(stack->buffer)))
> + INJECTION_POINT("gin-leave-leaf-split-incomplete");
> + else
> + INJECTION_POINT("gin-leave-internal-split-incomplete");
> +#endif
>
> Yes, that would be one way of doing it. It makes the code a bit more
> invasive but that would work. Now, you expect this point to be
> conditional depending on the state of the buffer. One thing I was
> thinking of was to extend the APIs in injection_point.c to be able to
> handle a set of arguments as a set of (void*) so as callbacks could
> internally decide what they want to do depending on the running state.
> I didn't have a use for it, but well, you do, so perhaps we could just
> bite the bullet and do it.
>
> That reduces the footprint on the backend code, moving more logic
> behind USE_INJECTION_POINTS in injection_point.c. At the end, you
> should be able to patch the core backend with something like that,
> without using some ifdefs on USE_INJECTION_POINTS:
> INJECTION_POINT_1ARG("gin-leave-leaf-split", (void *) buffer)
>
> And INJECTION_POINT_1ARG() would be just a wrapper around something
> like:
> extern void InjectionPointRun1(const char *name, void *arg1);
I can see that being useful in general. It requires a bespoken callback
function, though. One nice thing about this test was that I didn't need
to write a new C module, only that snippet at the injection point and
some SQL. In more complicated tests a new C module is warranted, but I
think a lot of tests - like this gin test - can be written without it.
Perhaps if the argument was uint64 instead of a pointer, so that the
'injection_point' module could contain ready-made functions that do
things with the argument.
Overall I feel we should not bother with injection point arguments for
now, though. Let's wait until we have a few more tests that would
benefit from that, and then we'll have more information on what the
ideal interface would look like.
--
Heikki Linnakangas
Neon (https://neon.tech)
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: pgsql-bugs@postgresql.org
Cc: hlinnaka@iki.fi, michael@paquier.xyz, feichanghong@qq.com, pgsql-bugs@lists.postgresql.org
Subject: Re: MarkBufferDirty Assert held LW_EXCLUSIVE lock fail when ginFinishSplit
In-Reply-To: <f5eb802a-e98d-438e-9fa8-1694f767e5ea@iki.fi>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox