| From: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
|---|---|
| To: | Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
| Cc: | Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> |
| Subject: | Re: [PATCH] Two remaining shmem attachment issues in single-user mode |
| Date: | 2026-09-23 12:23:33 |
| Message-ID: | 99bdbc90-2ecf-44ab-b545-606c585ef811@iki.fi |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 19/09/2026 22:12, Ayush Tiwari wrote:
> Hi,
>
> I found two more shmem attachment issues in single-user mode after
> the recent fixes. Patches attached.
>
> With ShmemInitStruct(), a second call for the same name and size errors
> out with "already initialized". We only look for the old allocation if
> IsUnderPostmaster is true, so a standalone backend goes straight down
> the allocation path again.
>
> 0001 drops that condition. It fixes postmaster-startup reattachment
> too, which I did hesitate over at first. But AFAICS that was supported
> before the refactoring, and it's what the legacy API still promises.
> So I'd lean towards restoring that behaviour in both places. Is there
> a reason not to? (The size and initialization checks are still there.)
Hmm, so the scenario is:
1. Start postgres in single-user mode
2. Load an extension that calls ShmemInitStruct() to alloc a shmem area
3. The extension calls ShmemInitStruct() again, to get a pointer to the
already-initialized area.
I didn't think of that scenario. There's no reason to not support that,
although it's pretty weird for an extension to do that. One use case
might be to have a shared struct between two cooperating extensions, so
that they both call ShmemInitStruct() to get a pointer to the same area.
I'm not aware of any extensions actually doing that, though.
I'll commit that fix, thanks!
> The other one is a bit odd: ask for an existing area with
> SHMEM_ATTACH_UNKNOWN_SIZE after startup in single-user mode, and we
> tell you it "cannot be used during startup".
>
> IIUC, the distinction we need here is whether we're still working out
> the initial shmem requirements, not whether we have a postmaster.
> 0002 adds SRS_REQUESTING_AFTER_STARTUP for that. I couldn't see a clean
> way to reuse SRS_REQUESTING without mixing those cases up.
>
> I'm a bit on the fence about adding another state just for this.
> With the PG19 release getting close, I thought I'd send this out for
> feedback before spending more time iterating on it. Does the extra
> state seem like the right approach?
Extra state sounds reasonable. Thanks, I'll take a closer look, and I'll
double-check all the other places in shmem.c where we use
IsUnderPostmaster, too.
The IsUnderPostmaster variable is deceptive. It's easy to forget about
single-user code, and incorrectly assume that IsUnderPostmaster == true
means you're a backend and IsUnderPostmaster == false means you're
postmaster. I think that's what happened to me here and with the
previous single-user mode bugs. I remember I've struggled to keep that
in mind in the past too. We should perhaps replace IsUnderPostmaster
with a three-valued enum or something (postmaster, backend, single-user
backend).
We got that wrong in pg_get_shmem_pagesize(), too, also in previous
versions:
backend> select * from pg_get_shmem_allocations_numa();
1: name (typeid = 25, len = -1, typmod = -1, byval = f)
2: numa_node (typeid = 23, len = 4, typmod = -1, byval = t)
3: size (typeid = 20, len = 8, typmod = -1, byval = t)
----
TRAP: failed Assert("IsUnderPostmaster"), File:
"../src/backend/storage/ipc/shmem.c", Line: 731, PID: 121579
- Heikki
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Dean Rasheed | 2026-09-23 12:28:47 | Re: SSI: ON CONFLICT DO SELECT takes no predicate lock on the returned row |
| Previous Message | shihao zhong | 2026-09-23 12:17:50 | Re: aio: worker: Free SMGR objects when idle |