Re: Error handling in after-startup shmem requests

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Error handling in after-startup shmem requests
Date: 2026-08-25 16:33:46
Message-ID: 4516f077-2671-472e-af84-204f131407e2@iki.fi
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 18/08/2026 13:37, Ayush Tiwari wrote:
> On Wed, 12 Aug 2026 at 15:12, Ashutosh Bapat
> <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com <mailto:ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>> wrote:
>
> On Wed, Aug 12, 2026 at 2:37 PM Ayush Tiwari
> <ayushtiwari(dot)slg01(at)gmail(dot)com <mailto:ayushtiwari(dot)slg01(at)gmail(dot)com>>
> wrote:
>
> > Thanks for the latest patch, it looks good to me.
> >
>
> Thanks.
>
> > 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.
> >
>
> Attached patch fixes those two as well.

Thanks! And sorry for not picking this up earlier.

Here's my take with some small changes. This is all based on things you
already discussed, I just picked the mix I liked best:

* I added the 'initialized' flag to each shmem index entry. It feels a
little heavy-handed just to deal with failed initializations after
postmaster startup, which is a rare codepath to begin with, but then
again, it's just a boolean. You still can't really retry or recover from
a failed initialization, the entry stays in the broken state until
restart, but at least you get a nicer error message and you don't need
worry about detecting it in your extension. As evidenced by smaller
changes needed to the docs to describe it :-).

* I kept the pending_shmem_requests list in the caller's memory context.
Instead of freeing it in PG_FINALLY, I just set pending_shmem_requests =
NIL, and rely on the memory context release to free it. (I was on the
fence on this, TopMemoryContext would also be quite reasonable)

* I structured the PG_TRY/CATCH block more like you had it in the
earlier patches, adding a little helper function.

What do you think?

- Heikki

Attachment Content-Type Size
0001-Fix-backend-state-after-a-failed-after-startup-shmem.patch text/x-patch 4.4 KB
0002-Track-which-shmem-areas-have-been-fully-initialized.patch text/x-patch 7.9 KB
0003-Add-tests-for-failing-shmem-allocations-after-startu.patch text/x-patch 6.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bharath Rupireddy 2026-08-25 16:34:00 Re: Report index currently being vacuumed in pg_stat_progress_vacuum
Previous Message Jacob Champion 2026-08-25 16:23:03 Re: PSA: Planning to grease protocol connections during 19beta