> On 24 Mar 2025, at 20:31, Rahila Syed <rahilasyed90@gmail.com> wrote:
> Please find the attached updated and rebased patch.
Thanks for this rebase, as was mentioned in the other thread I plan to get this
committed fairly soon. A few comments on the code, all performed in the
attached v3.
+ else
+ {
+ /* Log failure of unpinning */
+ elog(DEBUG2, "unable to unpin the segment %u as CurrentResourceOwner is NULL or releasing",
+ seg);
+ seg->resowner = NULL;
+ }
I removed the elog() calls since I can't see it adding enough value, and the
assignment to NULL can be removed as well since we've already asserted that
seg->resowner is NULL.
+ INJECTION_POINT("dsa_create_on_res_release");
This feels like a name which limits its use to this one test, whereas it is a
general purpose injection point. Renamed, and also moved to using dashes
rather than underscore as the former is project style.
+void
+test_dsa_inj(const char *name, const void *private_data)
Rather than duplicating the code I created an internal function for this test
which can be called from the existing basic test as well as this new test.
I also did a little bit of renaming to make it more readable.
As it can only really be tested with an injection point I plan on only
backpatching to 17 initially. Searching the archives I didn't find any mention
of this bug ever being hit so it seems safe to let it prove itself in testable
versions before going further back with it.
--
Daniel Gustafsson