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
signature.asc
Description: PGP signature
