MIME-Version: 1.0
References: 
 <CAExHW5vM1bneLYfg0wGeAa=52UiJ3z4vKd3AJ72X8Fw6k3KKrg@mail.gmail.com>
 <CA+TgmobqwCHp1w6xkTJmZiq4C8Pr0O3tNUvSFZcAgYLkvF45bg@mail.gmail.com>
 <8a6799be-bd42-49fb-8914-856c97bb1977@iki.fi>
 <CA+Tgmob8ot7qam7a650nx_4jyEoFLs8s_kPMc1+EB7-dr56=WA@mail.gmail.com>
 <cc3ff976-c303-45cd-9a8d-a90483170e37@iki.fi>
 <CA+TgmoauqYXm8iA3FGRAVKxYShUxWBiS_MSLmQpTrmO7wNHamw@mail.gmail.com>
 <113724ab-0028-493f-9605-6e8570f0939f@iki.fi>
 <CAExHW5uTNWOSxJDWQAUnS0tZawob2_J3dRAtc67NHNZ98X4_xA@mail.gmail.com>
 <CAExHW5t439y61YD9bc7d5wZWHp6J=M43Qu3eEZOBPguZML7o2A@mail.gmail.com>
 <CAExHW5v5FVZbsO9sLzztMZ11C3hgGStE=HkkV2bQkCyncess4w@mail.gmail.com>
 <b2451225-0a10-4ded-abef-4e9efbde7a8e@iki.fi>
 <CAExHW5tCC0T1ky=Jnq-AvMxa67Adaw7aQ4iQAO=BSdHcbSNBVg@mail.gmail.com>
 <791c3f18-f4de-4d84-ac6b-c7ccc074dd38@iki.fi>
 <CAExHW5tS7GncN90oJWOSzW_3F1EHL9xwe59L7Req3nUVgmObUw@mail.gmail.com>
 <9d919bd9-94dd-4bda-8ccf-ebced4178c53@iki.fi>
 <CAExHW5sYgt=XekOoAE-Tu_Dv8obWZCjCqAPT9vtN2D4m=M=drQ@mail.gmail.com>
 <470e7ebe-0971-49f6-8e46-9b8f6395f88b@iki.fi>
In-Reply-To: <470e7ebe-0971-49f6-8e46-9b8f6395f88b@iki.fi>
From: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Date: Sat, 4 Apr 2026 22:02:47 +0530
Message-ID: 
 <CAExHW5v746TWa3s6gJ6eKBu-9FNkYdUTXAoRCdkeKbXeBD9YAQ@mail.gmail.com>
Subject: Re: Better shared data structure management and resizable shared data
 structures
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Robert Haas <robertmhaas@gmail.com>, Andres Freund <andres@anarazel.de>,
	pgsql-hackers <pgsql-hackers@postgresql.org>, chaturvedipalak1911@gmail.com
Content-Type: multipart/mixed; boundary="0000000000009efbcc064ea4fc39"
Archived-At: 
 <https://www.postgresql.org/message-id/CAExHW5v746TWa3s6gJ6eKBu-9FNkYdUTXAoRCdkeKbXeBD9YAQ%40mail.gmail.com>
Precedence: bulk

--0000000000009efbcc064ea4fc39
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Sat, Apr 4, 2026 at 6:19=E2=80=AFAM Heikki Linnakangas <hlinnaka@iki.fi>=
 wrote:
>
> On 02/04/2026 09:58, Ashutosh Bapat wrote:
> > On Wed, Apr 1, 2026 at 11:47=E2=80=AFPM Heikki Linnakangas <hlinnaka@ik=
i.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. Th=
is 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 o=
ccur.
> >>> + */
> >>> + size_t init_size;
> >>>
> >>> Everytime I look at these two fields, I question whether those are th=
e
> >>> 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_siz=
e
> >> 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 =3D 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.

--=20
Best Wishes,
Ashutosh Bapat

--0000000000009efbcc064ea4fc39
Content-Type: application/octet-stream; name="0014_edits.diff.nocibot"
Content-Disposition: attachment; filename="0014_edits.diff.nocibot"
Content-Transfer-Encoding: base64
Content-ID: <f_mnkjuf6d0>
X-Attachment-Id: f_mnkjuf6d0

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL3JlcGxpY2F0aW9uL2xvZ2ljYWwvb3JpZ2luLmMgYi9z
cmMvYmFja2VuZC9yZXBsaWNhdGlvbi9sb2dpY2FsL29yaWdpbi5jCmluZGV4IGRhYTk4NDMzMGYx
Li5iZDBiN2MzNGNkMCAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvcmVwbGljYXRpb24vbG9naWNh
bC9vcmlnaW4uYworKysgYi9zcmMvYmFja2VuZC9yZXBsaWNhdGlvbi9sb2dpY2FsL29yaWdpbi5j
CkBAIC01NzMsNyArNTczLDEwIEBAIHN0YXRpYyB2b2lkCiBSZXBsaWNhdGlvbk9yaWdpblNobWVt
SW5pdCh2b2lkICphcmcpCiB7CiAJaWYgKG1heF9hY3RpdmVfcmVwbGljYXRpb25fb3JpZ2lucyA9
PSAwKQorCXsKKwkJQXNzZXJ0KCFyZXBsaWNhdGlvbl9zdGF0ZXNfY3RsKTsKIAkJcmV0dXJuOwor
CX0KIAogCXJlcGxpY2F0aW9uX3N0YXRlcyA9IHJlcGxpY2F0aW9uX3N0YXRlc19jdGwtPnN0YXRl
czsKIApAQCAtNTkxLDcgKzU5NCwxMCBAQCBzdGF0aWMgdm9pZAogUmVwbGljYXRpb25PcmlnaW5T
aG1lbUF0dGFjaCh2b2lkICphcmcpCiB7CiAJaWYgKG1heF9hY3RpdmVfcmVwbGljYXRpb25fb3Jp
Z2lucyA9PSAwKQorCXsKKwkJQXNzZXJ0KCFyZXBsaWNhdGlvbl9zdGF0ZXNfY3RsKTsKIAkJcmV0
dXJuOworCX0KIAogCXJlcGxpY2F0aW9uX3N0YXRlcyA9IHJlcGxpY2F0aW9uX3N0YXRlc19jdGwt
PnN0YXRlczsKIH0K
--0000000000009efbcc064ea4fc39--