public inbox for [email protected]  

help / color / mirror / Atom feed

From: Sami Imseih <[email protected]>
To: Michael Paquier <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: Daniil Davydov <[email protected]>
Subject: Re: Allow a condition string in an injection point
Date: Thu, 9 Apr 2026 18:05:02 -0500
Message-ID: <CAA5RZ0sbaw7=WdztVm71p0jsVnTVvn7SMYaO+hWR474bTiaGeQ@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAA5RZ0tsGHu2h6YLnVu4HiK05q+gTE_9WVUAqihW2LSscAYS-g@mail.gmail.com>
	<[email protected]>

> > A follow-up to the discussion here [0], here is a patch that allows
> > for an arbitrary string in injection points to be able to apply more
> > granular filters for running an injection point. This will be useful
> > for autovacuum testing as discussed in the referenced thread,
> > and perhaps in some other places.
>
> Are the patches under discussion required for v19 or is that something
> that can wait before v20 opens for business?  We have always required
> a use-case in core before adding a new API in this module, to justify
> its existence.

This is v20. One of the use-case is discussed here [1]. When testing of
autovacuum for a specific table, we need a way to run the injection point
for that table only, else we end up running the point it for all tables. This
is especially true for check-world where other non-related tables are
being autovacuumed. So this gives more granular control.

> > Worth noting, the condition types were changed to bit flags since
> > we may need to combine conditions such as local injection point
> > and string.
>
> It's definitely more useful to allow combinations of them in an AND
> fashion (if two conditions are defined then check both, and allow the
> point to run if both conditions pass).  Just to say that what you are
> doing looks sensible for me.  And I find that pretty cool for its
> simplicity and what it could provide for future tests.

yes, I did test with some other new hypothetical condition in the future and
it was simple to plug it in to the code at that point.

> @@ -322,6 +322,7 @@ InjectionPointAttach(const char *name,
>         strlcpy(entry->name, name, sizeof(entry->name));
>         strlcpy(entry->library, library, sizeof(entry->library));
>         strlcpy(entry->function, function, sizeof(entry->function));
> +       memset(entry->private_data, 0, INJ_PRIVATE_MAXLEN);
>         if (private_data != NULL)
>                 memcpy(entry->private_data, private_data, private_data_size);
>
> Hmm, this could be qualified as a bug, and surely it's a good practice
> to clean things on attach.  I'll go backpatch that.

It did not matter before this new condition type since the private_data
was never NULL. Not the case with this patch, as it caused inection_points test
regressions with this change. So, yeah, it's a latent bug.

--
Sami

[1] [https://www.postgresql.org/message-id/CAA5RZ0tExiffcu7qvrUbpq_qqz%3DzCD2aJ5_Qigo6eP2kgTx3eQ%40mail.g...]

view thread (3+ messages)

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]
  Subject: Re: Allow a condition string in an injection point
  In-Reply-To: <CAA5RZ0sbaw7=WdztVm71p0jsVnTVvn7SMYaO+hWR474bTiaGeQ@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