On Fri, Nov 24, 2023 at 7:26 AM Michael Paquier <michael@paquier.xyz> wrote:
> If you wish to use a combination of N points with a sleep callback and
> different sleep times, one can just register a second shmem area in
> the extension holding the callbacks that links the point names with
> the sleep time to use.
>
Interesting idea. For that the callback needs to know the injection
point name. At least we should pass that to the callback. It's trivial
thing to do.
> > shmem hash size won't depend upon the number of functions we write in
> > the core. Yes it will add to the core code and may add maintenance
> > burden. So I understand your inclination to keep the core minimal.
>
> Yeah, without a clear benefit, my point is just to throw the
> responsibility to extension developers for now, which would mean the
> addition of tests that depend on test_injection_points/, or just
> install this extension optionally in other code path that need it.
> Maybe 0004 should be in src/test/recovery/ and do that, actually..
That might work, but in order to run tests in that directory one has
to also install the extension. Do we have precedence for such kind of
dependency?
>
> > If the session which attaches to an injection point is same as the
> > session where the injection point is triggered (most of the ERROR and
> > NOTICE injections will see this pattern), we don't need shared memory.
> > There's a performance penalty to it since injection_run will look up
> > shared memory. For WAIT we may or may not need shared memory. But
> > let's see what other think and what usage patterns we see.
>
> The first POC of the patch that you can find at the top of this thread
> did that, actually, but this is too limited. IMO, linking things to a
> central table is just *much* more useful.
>
> I've implemented a v5 that switches the cache to use a seconf hash
> table on TopMemoryContext for the cache instead of an array. This
> makes the cache handling slightly cleaner, so your suggestion was
> right.
glad that you liked the outcome.
> 0003 and 0004 are the same as previously, passing or failing
> under the same conditions. I'm wondering if folks have other comments
> about 0001 and 0002? It sounds to me like the consensus is that this
> stuff is useful
I think so.
> and that there are no string objections, so feel free
> to comment.
Let's get some more opinions on the design. I will review the detailed
code then.
>
> I don't want to propose 0003 in the tree, just an improved version of
> 0004 for the test coverage (still need to improve that).
Are you working on v6 already?
--
Best Wishes,
Ashutosh Bapat