public inbox for [email protected]
help / color / mirror / Atom feedFrom: Ashutosh Bapat <[email protected]>
To: Heikki Linnakangas <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: [email protected]
Subject: Re: Better shared data structure management and resizable shared data structures
Date: Sat, 4 Apr 2026 22:02:47 +0530
Message-ID: <CAExHW5v746TWa3s6gJ6eKBu-9FNkYdUTXAoRCdkeKbXeBD9YAQ@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAExHW5vM1bneLYfg0wGeAa=52UiJ3z4vKd3AJ72X8Fw6k3KKrg@mail.gmail.com>
<CA+TgmobqwCHp1w6xkTJmZiq4C8Pr0O3tNUvSFZcAgYLkvF45bg@mail.gmail.com>
<[email protected]>
<CA+Tgmob8ot7qam7a650nx_4jyEoFLs8s_kPMc1+EB7-dr56=WA@mail.gmail.com>
<[email protected]>
<CA+TgmoauqYXm8iA3FGRAVKxYShUxWBiS_MSLmQpTrmO7wNHamw@mail.gmail.com>
<[email protected]>
<CAExHW5uTNWOSxJDWQAUnS0tZawob2_J3dRAtc67NHNZ98X4_xA@mail.gmail.com>
<CAExHW5t439y61YD9bc7d5wZWHp6J=M43Qu3eEZOBPguZML7o2A@mail.gmail.com>
<CAExHW5v5FVZbsO9sLzztMZ11C3hgGStE=HkkV2bQkCyncess4w@mail.gmail.com>
<[email protected]>
<CAExHW5tCC0T1ky=Jnq-AvMxa67Adaw7aQ4iQAO=BSdHcbSNBVg@mail.gmail.com>
<[email protected]>
<CAExHW5tS7GncN90oJWOSzW_3F1EHL9xwe59L7Req3nUVgmObUw@mail.gmail.com>
<[email protected]>
<CAExHW5sYgt=XekOoAE-Tu_Dv8obWZCjCqAPT9vtN2D4m=M=drQ@mail.gmail.com>
<[email protected]>
On Sat, Apr 4, 2026 at 6:19 AM Heikki Linnakangas <[email protected]> wrote:
>
> On 02/04/2026 09:58, Ashutosh Bapat wrote:
> > On Wed, Apr 1, 2026 at 11:47 PM Heikki Linnakangas <[email protected]> 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
Attachments:
[application/octet-stream] 0014_edits.diff.nocibot (696B, 2-0014_edits.diff.nocibot)
download
view thread (82+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Better shared data structure management and resizable shared data structures
In-Reply-To: <CAExHW5v746TWa3s6gJ6eKBu-9FNkYdUTXAoRCdkeKbXeBD9YAQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox