Re: Persist injection points across server restarts

On Wed, May 21, 2025 at 12:17:55PM +0300, Andrey Borodin wrote:
> I've looked into the patch set and have some more questions:
> 1. What if we flush many times? Is it supposed to overwrite injection_points.data?

Yes.

> 2. What if we restart many times? first startup will remove the file... maybe remove it explicitly?

Yes. The point is to persist across one restart. That's the same
behavior as what we do for the stats, implying that an extra flush
would be required across multiple restarts. As this behavior is
within the extension, I don't see why we could not change the
internals at some point if we're unhappy with what it does. The only
case where I can see us do a node restart with point persistence is
TAP tests, where local points don't really make sense. It is true
that we could introduce at some point tests with more advanced
filtering conditions, like backend types where flushes of the private
data could make sense. We don't have that now.

> 3. InjectionPoint private data is not handled, is it OK?

Because I don't have a case for that in core yet. We could add it, of
course, but I've always defined the bar of what gets into the backend
code as of something that is used by the core tests. I've just not
needed it for this test case.

> 4. How session-local points are expected to be flushed? into which
> session they will be loaded? Maybe let's document that they are not
> saved?

Yes, I was wondering about that, but finished by discarding it based
on the same argument as the previous point. Again, we could do that.
Flushes with restarts would be part of TAP tests, where we don't care
about concurrent test sessions, so I'm not sure it is worth worrying
at this stage. That feels like bloat in the backend interface than
actually required.
--
Michael