Re: Error handling in after-startup shmem requests

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.

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).

--
Best Wishes,
Ashutosh Bapat