Re: [PATCH] Two remaining shmem attachment issues in single-user mode

From: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
To: Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, 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-20 08:27:26
Message-ID: CB18DE9D-7461-47D3-B51C-E4ADD515EDA4@gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Sep 20, 2026, at 03:12, Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com> 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.)

I think it’s reasonable to remove the IsUnderPostmaster check for two reasons:

* IsUnderPostmaster is always false in single-user mode, so the current check prevents reattachment there.
* The current logic feels unnecessarily asymmetric. The natural pattern for ShmemInitStruct() is to first look for an existing allocation and create one only if none exists. With the IsUnderPostmaster check, however, the postmaster always goes directly to creation, while a child process first looks for an existing allocation and creates one if it is not found. That makes the code a little confusing to reason about. It suggests that the create-or-attach behavior depends on whether the process is under the postmaster, even though I don't see why that distinction is needed here. In practice, the postmaster initializes shared memory before child processes are started anyway.

So, removing the check makes the behavior simpler and more consistent, always look for an existing allocation first, and create one only if it doesn't exist.

>
> 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?
>

I’m not very keen on adding a new state just to allow SHMEM_ATTACH_UNKNOWN_SIZE in single-user mode. While reading 0002, I had a few concerns:

1
```
- if (IsUnderPostmaster)
+ if (IsUnderPostmaster || shmem_request_state == SRS_REQUESTING_AFTER_STARTUP)
{
if (options->size <= 0 && options->size != SHMEM_ATTACH_UNKNOWN_SIZE)
elog(ERROR, "invalid size %zd for shared memory request for \"%s\"",
```

Now that we have SRS_REQUESTING_AFTER_STARTUP, my first thought was whether the IsUnderPostmaster check could be removed entirely.

After digging into the code, I don't think so. IsUnderPostmaster is still needed for cases such as EXEC_BACKEND children, so the new state seems to be added specifically to cover the single-user after-startup case.

That makes me wonder whether introducing another state is the simplest way to express this.

2
```
- * DONE -> REQUESTING -> AFTER_STARTUP_ATTACH_OR_INIT -> DONE
+ * DONE -> REQUESTING_AFTER_STARTUP -> AFTER_STARTUP_ATTACH_OR_INIT -> DONE
```

The main reason for introducing REQUESTING_AFTER_STARTUP seems to be to allow SHMEM_ATTACH_UNKNOWN_SIZE. But the next state is still AFTER_STARTUP_ATTACH_OR_INIT, so the new state is not really describing an attach-only phase.

That makes the state machine a little harder for me to reason about, because REQUESTING_AFTER_STARTUP is mainly distinguishing the context in which the request callback runs rather than representing a fundamentally different shared-memory lifecycle state.

If the only additional case we need to distinguish here is a late request in single-user mode, would it be simpler to check MyBackendType == B_STANDALONE_BACKEND instead of adding a new request state?

> Regards,
> Ayush
> <v1-0001-Fix-legacy-shmem-reattachment-in-single-user-mode.patch><v1-0002-Allow-unknown-size-shmem-attachments-in-single-user-mode.patch>

Also, for v1-0001, I personally don't feel that the new test is really necessary. This doesn't seem like particularly fragile logic that's likely to regress accidentally, and I've noticed that Tom generally seems reluctant to increase test runtime, even by a very small amount, unless the test provides clear value.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Henson Choi 2026-09-20 09:00:46 Re: Row pattern recognition
Previous Message Alexandre Felipe 2026-09-20 07:52:05 Re: [patch] Cache invalidation for I/O Workers