Backpatching injection point core facilities to REL_17_STABLE

Started by Michael Paquier10 months ago7 messageshackers
Jump to latest
#1Michael Paquier
michael@paquier.xyz

Hi all,

As mentioned during my talk about injection points at the last
pgconf.dev, I think that we should be more aggressive with the
backpatching of the core APIs of this facility, to ease the porting of
tests across more branches and the maintenance of this stuff itself,
so as it evolves in a consistent way across all stable branches.

Backporting that down to v16 would require more work, like in the area
of custom wait events for the injection point category for isolation
tests. The story is different for v17, where most of the basics are
in place. Please see the attached, that adds the following parts to
REL_17_STABLE:
- Cached points and loading, for critical sections.
- IS_INJECTION_POINT_ATTACHED(), for stack manipulations.
- Runtime arguments.

Thoughts or comments?
--
Michael

Attachments:

0001-Make-injection-point-facilities-consistent-with-HEAD.patchtext/x-diff; charset=us-asciiDownload+344-48
#2Andrey Borodin
amborodin@acm.org
In reply to: Michael Paquier (#1)
Re: Backpatching injection point core facilities to REL_17_STABLE

On 27 Jun 2025, at 14:25, Michael Paquier <michael@paquier.xyz> wrote:

Thoughts or comments?

I'm +1 on having full-fledged injection points in back branches where possible. Right now I'm debugging a PG-17(probably) problem where injection preloading would be handy (though functionality is available via hacks, just a little more work).

But are you going to backpatch all new features of injection points in future? It's potentially x6 more work.

Thanks!

Best regards, Andrey Borodin.

#3Michael Paquier
michael@paquier.xyz
In reply to: Andrey Borodin (#2)
Re: Backpatching injection point core facilities to REL_17_STABLE

On Fri, Jun 27, 2025 at 02:45:58PM +0500, Andrey Borodin wrote:

I'm +1 on having full-fledged injection points in back branches
where possible. Right now I'm debugging a PG-17(probably) problem
where injection preloading would be handy (though functionality is
available via hacks, just a little more work).

But are you going to backpatch all new features of injection points
in future? It's potentially x6 more work.

That may be nice, but I'd be interested in seeing how things evolve
across v17 first before taking a decision for older branches.
--
Michael

#4Andrey Borodin
amborodin@acm.org
In reply to: Michael Paquier (#3)
Re: Backpatching injection point core facilities to REL_17_STABLE

On 28 Jun 2025, at 05:38, Michael Paquier <michael@paquier.xyz> wrote:

On Fri, Jun 27, 2025 at 02:45:58PM +0500, Andrey Borodin wrote:

I'm +1 on having full-fledged injection points in back branches
where possible. Right now I'm debugging a PG-17(probably) problem
where injection preloading would be handy (though functionality is
available via hacks, just a little more work).

But are you going to backpatch all new features of injection points
in future? It's potentially x6 more work.

That may be nice, but I'd be interested in seeing how things evolve
across v17 first before taking a decision for older branches.

FWIW both multixact problem [0]/messages/by-id/172e5723-d65f-4eec-b512-14beacb326ce@yandex.ru and my recent corruption finding [1]/messages/by-id/B3C69B86-7F82-4111-B97F-0005497BB745@yandex-team.ru would benefit a lot from having ability to do injection points down to PG 12.
And while [0]/messages/by-id/172e5723-d65f-4eec-b512-14beacb326ce@yandex.ru is a bug that is detectable with several pgbenches, I have a good sounding proof that [1]/messages/by-id/B3C69B86-7F82-4111-B97F-0005497BB745@yandex-team.ru can't happen at all and no way to detect it without waiting injection point (or similar hand-hacked functionality).

Best regards, Andrey Borodin.

[0]: /messages/by-id/172e5723-d65f-4eec-b512-14beacb326ce@yandex.ru
[1]: /messages/by-id/B3C69B86-7F82-4111-B97F-0005497BB745@yandex-team.ru

#5Michael Paquier
michael@paquier.xyz
In reply to: Andrey Borodin (#4)
Re: Backpatching injection point core facilities to REL_17_STABLE

On Thu, Aug 07, 2025 at 12:53:12PM +0300, Andrey Borodin wrote:

FWIW both multixact problem [0] and my recent corruption finding [1]
would benefit a lot from having ability to do injection points down
to PG 12.
And while [0] is a bug that is detectable with several pgbenches, I
have a good sounding proof that [1] can't happen at all and no way
to detect it without waiting injection point (or similar hand-hacked
functionality).

Thanks for the input.

It is v18 and HEAD are on par in terms of features, with only
4eca711bc991 and 7b2eb72b1b8c requiring a cherry-pick, so perhaps we
should begin with that, before moving with more cherry-picks down to
v17 as the second step?

Any thoughts from others? Would it be useful to get that down to the
back-branches?
--
Michael

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paquier (#5)
Re: Backpatching injection point core facilities to REL_17_STABLE

Michael Paquier <michael@paquier.xyz> writes:

It is v18 and HEAD are on par in terms of features, with only
4eca711bc991 and 7b2eb72b1b8c requiring a cherry-pick, so perhaps we
should begin with that, before moving with more cherry-picks down to
v17 as the second step?

Any thoughts from others? Would it be useful to get that down to the
back-branches?

I don't have a strong opinion about whether this is worth getting into
the back branches. I do have a strong opinion that just before a
release freeze is not the time to be pushing such changes into stable
branches. Even after the release, I wouldn't touch v13: we have
learned repeatedly that inessential changes made in a branch's final
release are often sources of regret.

regards, tom lane

#7Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#6)
Re: Backpatching injection point core facilities to REL_17_STABLE

On Thu, Aug 07, 2025 at 09:34:05PM -0400, Tom Lane wrote:

I do have a strong opinion that just before a
release freeze is not the time to be pushing such changes into stable
branches.

Don't worry. I have no plans to do any of that before the next minor
release. Sorry if that was not clear. I was actually counting on the
one-week table branch freeze to gather more opinions.

Even after the release, I wouldn't touch v13: we have
learned repeatedly that inessential changes made in a branch's final
release are often sources of regret.

Sure.
--
Michael