Re: Error handling in after-startup shmem requests

Hi,

On Wed, 12 Aug 2026 at 13:11, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
wrote:

> On Tue, Aug 11, 2026 at 11:35 PM Ayush Tiwari
> <ayushtiwari(dot)slg01(at)gmail(dot)com> wrote:
> >
> >
> > One detail caught my attention: the no-request branch now returns from
> inside
> > PG_TRY. Could that skip PG_FINALLY/PG_END_TRY and leave the saved error
> stack
> > unrestored? Would it be safer to guard the allocation work with
> > `pending_shmem_requests != NIL`, allowing every path to reach the common
> > cleanup instead?
> >
>
> You are right. Thanks for the catch. Fixed in the attached version.
>
> >> >>
> >>
> >> Node creation is an expensive operation. We should reuse it as much as
> >> possible, like attached.
> >
> >
> > Agreed on reusing the node. Since the stale list is backend-local, do
> separate
> > `$node->psql()` calls use different backends and miss the retry path?
>
> You are right again. We need the same session to retry. In the
> attached version, I have changed the sequence of tests so that the
> first test leaves a partially initialized but allocated area behind
> and demonstrates how to handle such a case. The next test fails during
> request and thus can be retried in the same psql session. That should
> cover the stale state issues. Let me know if something is still
> missing.
>
> > I tried
> > the test with one background psql session. With the init_fn injection,
> the
> > area was already indexed but uninitialized, and the later CREATE
> EXTENSION
> > failed in test_shmem_attach(), consistent with the new documentation.
> Would
> > an injection immediately after request_fn be closer to the original
> failure?
> >
>
> The place where the injection point was placed earlier could never
> have a failure. The failure can be either when request callbacks are
> called or in the init callbacks not in-between. With the current
> injection point placement both the cases, failure immediately after
> request and also a failure in initialization are covered.
>
> >>
> >> >
> >> > Pre-setting test_shmem.area_size as a placeholder gave the same
> result;
> >> > init_custom_variable() performs this check before placeholder
> replacement.
> >> > I have therefore kept it PGC_USERSET as a test-only control for the
> size
> >> > passed by request_fn.
> >> >
> >>
> >> Thanks for the explanation. Why do we need test_shmem_guc_defined?
> >
> >
> > It is needed for the same-backend retry. A failed CREATE EXTENSION can
> leave
> > the library mapped, but it is not added to the successfully-loaded
> library
> > list until _PG_init() returns, so the retry invokes _PG_init() again.
> When I
> > removed the guard, the second attempt failed with `attempt to redefine
> > parameter "test_shmem.area_size"` before reaching the shmem retry path.
> >
>
> Hmm. Let's leave it there then.
>
> > With an injection immediately after request_fn and fail/fail/succeed in
> one
> > backend, your cleanup fixed the original retry failure in my testing.
> Please
> > let me know if I have misunderstood any of the points above.
>
> Your points are correct. Please review the latest patch and see if it
> covers all the scenarios.
>

Thanks for the latest patch, it looks good to me.

I just have 2 minor editorial nits:

- The commit message mentions CallShmemCallbacksAfterStartupCleanup(), but I
think that should be CallShmemCallbacksAfterStartup().

- "unitialized" in xfunc.sgml should be "uninitialized".

These can be adjusted while committing so I'm not sending an updated patch.

> Also please add this thread to commitfest so that it's not forgotten
> and also it gets tested by CI (especially the EXEC_BACKEND case).
>

Here's the CF entry I had created:
https://commitfest.postgresql.org/patch/7115/

And also added it to the open list item for PG 19.

Regards,
Ayush