Re: Better shared data structure management and resizable shared data structures

On Wed, Apr 8, 2026 at 1:08 AM Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:
>
> On Tue, 7 Apr 2026 at 16:47, Ashutosh Bapat
> <ashutosh.bapat.oss@gmail.com> wrote:
> >
> > On Tue, Apr 7, 2026 at 3:36 PM Ashutosh Bapat
> > <ashutosh.bapat.oss@gmail.com> wrote:
> > >
> > > On Mon, Apr 6, 2026 at 7:23 PM Ashutosh Bapat
> > > <ashutosh.bapat.oss@gmail.com> wrote:
> > > >
> > > > I have kept these two patches separate from the main patch so that I
> > > > can remove them if others feel they are not worth including in the
> > > > feature.
> > >
> > > Here are patches rebased on the latest HEAD. No conflicts just rebase.
> > >
> > > Here are differences from the previous patchset.
> > >
> > > o. There are two patches in this patchset now. a. 0001 which supports
> > > resizable shared memory and is equivalent to 0001 + 0002 + 0004 + 0005
> > > from the previous patchset. b. 0002 which is 0006 from the previous
> > > patchset and adds support for protecting resizable shared memory
> > > structures. 0003, which added diagnostics to investigate CFBot
> > > failure, from the previous patchset is not required anymore since all
> > > tests pass with CFBot.
> > >
> > > o. I have merged 0002 into 0001 from the previous patchset since with
> > > that patch all platforms are green on CFBot. The resizable shared
> > > memory test now uses /proc/self/smaps instead of /proc/self/status to
> > > find the amount of memory allocated in the main shared memory segment
> > > of PostgreSQL.
> > >
> > > o. Merged 0004, which supported minimum_size, into 0001. Minimum_size
> > > would be useful to protect against accidental shrinkage of the
> > > resizable structures. It will help additional support for minimum
> > > sizes of GUCs like shared_buffers. It also makes it easy and intuitive
> > > to distinguish between fixed-size and resizable structures, and will
> > > be useful to find the minimum size of the shared memory segment.
>
> I was thinking more along the lines of attached (incremental) patch
> 0003 for min/max sizing. I'd say it has a slightly more natural API,
> but YMMV.
>

Thanks for the proposal. There are some advantages and disadvantages
of that approach. Let me explain.

minimum_size = 0 seems more straightforward to me compared to the
introduction of SHMEM_RESIZE_TO_ZERO - a value other than 0 to mean 0.
That's confusing.

The thought to modify the options in place did cross my mind and I
started going that route. But soon realized that a. option is a caller
structure which is not designed to scribble upon, b. it is saved as a
request and used later. By scribbling upon it, we lose the intent of
the original request, thus the saved request may be susceptible to a
different interpretation in future. I would like to avoid scribbling
as much as possible. The code after scribbling doesn't look materially
improved than earlier.

I like the error handling refactoring, but need to pay close attention
to the details. I tried something similar that didn't work in all the
cases. I will try your changes in the next version.

0004 actually changes the error message we throw when the request is
opposite of the existing structure. Is that intentional? But I guess
some of it can be absorbed to simplify the code here. The macro
definition is confusing.

+#define CHECK_SIZE(size) \
+do { \
+ /* Check that the sizes in the index match the request. */ \
+ if (request->options->size != SHMEM_ATTACH_UNKNOWN_SIZE && \
+ index_entry->size != request->options->size) \
+ { \
+ ereport(ERROR, \
+ (errmsg("shared memory struct \"%s\" was created with" \
+ " different %s: existing %zu, requested %zu", \
+ name, CppAsString(size), index_entry->size, \
+ request->options->size))); \
+ } \
+} while (false)

Ideally size here should be in paranthesis. Its easy to confuse
request->options->size to mean request->options->size when it actually
means request->options->{maximum/minimum}_size. Is that right?
Possibly a static inline function where we pass corresponding members
of request->options and index_entry?

> -----
>
> I also noticed that it's probably not correct to "just" check and
> complain about the size of a resizable shmem segment when you attach:
> Without coordination about which startup size the shmem segment should
> have, how could you get the current size state correct? And
> cross-process coordination of size information before shmem is
> attached is not really possible, not when you may have to deal with a
> very slow to start backend.

SHMEM_ATTACH_UNKNOWN_SIZE can be used there. test_shmem module already
uses it that way.

-- 
Best Wishes,
Ashutosh Bapat