Re: Better shared data structure management and resizable shared data structures - Mailing list pgsql-hackers

On Sat, Apr 4, 2026 at 6:19 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>
> On 02/04/2026 09:58, Ashutosh Bapat wrote:
> > On Wed, Apr 1, 2026 at 11:47 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> >>> + /*
> >>> + * Extra space to reserve in the shared memory segment, but it's not part
> >>> + * of the struct itself. This is used for shared memory hash tables that
> >>> + * can grow beyond the initial size when more buckets are allocated.
> >>> + */
> >>> + size_t extra_size;
> >>>
> >>> When we introduce resizable structures (where even the hash table
> >>> directly itself could be resizable), we will introduce a new field
> >>> max_size which is easy to get confused with extra_size. Maybe we can
> >>> rename extra_size to something like "auxilliary_size" to mean size of
> >>> the auxiliary parts of the structure which are not part of the main
> >>> struct itself.
> >>>
> >>> + /*
> >>> + * max_size is the estimated maximum number of hashtable entries. This is
> >>> + * not a hard limit, but the access efficiency will degrade if it is
> >>> + * exceeded substantially (since it's used to compute directory size and
> >>> + * the hash table buckets will get overfull).
> >>> + */
> >>> + size_t max_size;
> >>> +
> >>> + /*
> >>> + * init_size is the number of hashtable entries to preallocate. For a
> >>> + * table whose maximum size is certain, this should be equal to max_size;
> >>> + * that ensures that no run-time out-of-shared-memory failures can occur.
> >>> + */
> >>> + size_t init_size;
> >>>
> >>> Everytime I look at these two fields, I question whether those are the
> >>> number of entries (i.e. size of the hash table) or number of bytes
> >>> (size of the memory). I know it's the former, but it indicates that
> >>> something needs to be changed here, like changing the names to have
> >>> _entries instead of _size, or changing the type to int64 or some such.
> >>> Renaming to _entries would conflict with dynahash APIs since they use
> >>> _size, so maybe the latter?
> >>
> >> I hear you, but I didn't change these yet. If we go with the patches
> >> from the "Shared hash table allocations" thread, max_size and init_size
> >> will be merged into one. I'll try to settle that thread before making
> >> changes here.
> >
> > Will review those patches next.
>
> Those are now committed, and here's a new version rebased over those
> changes. The hash options is now called 'nelems', and the 'extra_size'
> in ShmemStructOpts is gone.
>

Thanks. Adjusted my resizable shared memory patch on top of this. The
result looks better.

> Plus a bunch of other fixes and cleanups. I also reordered and
> re-grouped the patches a little, into more logical increments I hope.

Some more comments

test_shmem declares MODULE_big and OBJS which seems to be old
fashioned, newer modules seem to be using MODULES. Also it should use
NO_INSTALLCHECK.

/*
* Alignment of the starting address. If not set, defaults to cacheline
* boundary. Must be a power of two.
*/
size_t alignment;

We don't seem to enforce the "must be a power of two" rule anywhere.
We should at least validate it.

I like the way buffer manager related changes untangle sub-sub-systems
of Buffer manager viz. StrategyControl and buffer look up table.
Simplifies code very much.

I also eyeballed some of the changes in 0014. If time permits, I will
review those closely soon. But the changes look ok.

Before this change, replication_states_ctl in origin.c was not
initialized explicitly when max_active_replication_origins = 0. With
this change, the structure is not registered and thus global static
pointer is not initialized. However, given that it's implicit, I
suggest adding Asserts as attached.

--
Best Wishes,
Ashutosh Bapat