MIME-Version: 1.0
References: 
 <CAH2L28vHzRankszhqz7deXURxKncxfirnuW68zD7+hVAqaS5GQ@mail.gmail.com>
 <k6f6ynjvb7lvebhygaiqsfrdohq672uughk3q4ve4q5jqljywz@7jbrz724epeq>
 <CAH2L28uL-8EQTRSeyTpW1DqAXsDXRkXCkT1dus2u6p4HYDrxAg@mail.gmail.com>
 <CAH2L28uGLhkXBKDWFKm5XZtp_0nNqpYQ3Hc35vG++mM7wuOhgg@mail.gmail.com>
 <CAN55FZ3cJxy0VkeXpuO3K4BpjzJo3S6oU+iMyc00P6gEjqPztw@mail.gmail.com>
 <CAH2L28uwxJREzB62UjRDBumE87hHWUJJvRwxqqbO+7qFmoZfTg@mail.gmail.com>
 <3e40eeec-d8bf-4496-854e-485dd901f6a2@vondra.me>
 <CAH2L28uG_g1Ljo8aL-g1MupJXO4Y7-a-bUCriE7w2213+KSGdA@mail.gmail.com>
 <6bf7194e-4c34-4e6d-8215-f6acf8903974@vondra.me>
 <6705dbd2-060b-4f3c-9fcb-1c7f10880b26@vondra.me>
 <CAH2L28tzCFEk2bxQ+oYv6zda=LFLfd_9cmq7HzsT4nj9KN1Yvg@mail.gmail.com>
 <66283922-0448-44e7-bbdd-38aaf2be34b9@vondra.me>
 <CAH2L28soW40DafNmLEEWMrNDTMWYZ9STWSS5axP910WQUT=JcQ@mail.gmail.com>
 <7987a0b5-9933-457a-b5ba-d28e9cce883b@vondra.me>
 <CAH2L28vaktm0UXw19y22w+DThcqV80-UKtVfMxPeVw8ro2yTDg@mail.gmail.com>
 <4e9a124d-29cb-4fd5-99fb-a5016e508f20@vondra.me>
 <CAH2L28vgzvTUqNwQay=jx4w30sHMx_pC+emnZErv8oX0R+SALQ@mail.gmail.com>
In-Reply-To: 
 <CAH2L28vgzvTUqNwQay=jx4w30sHMx_pC+emnZErv8oX0R+SALQ@mail.gmail.com>
From: Amit Kapila <amit.kapila16@gmail.com>
Date: Tue, 13 May 2025 16:34:05 +0530
Message-ID: 
 <CAA4eK1+GXW4uKzzT45Zd14QKFNQ34wtO7FOUMe2wZvJJONgdmA@mail.gmail.com>
Subject: Re: Improve monitoring of shared memory allocations
To: Rahila Syed <rahilasyed90@gmail.com>
Cc: Tomas Vondra <tomas@vondra.me>, Nazir Bilal Yavuz <byavuz81@gmail.com>,
	Andres Freund <andres@anarazel.de>,
 PostgreSQL-development <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAA4eK1%2BGXW4uKzzT45Zd14QKFNQ34wtO7FOUMe2wZvJJONgdmA%40mail.gmail.com>
Precedence: bulk

On Fri, Apr 4, 2025 at 9:15=E2=80=AFPM Rahila Syed <rahilasyed90@gmail.com>=
 wrote:
>
> Please find attached the patch which removes the changes for non-shared h=
ash tables
> and keeps them for shared hash tables.
>

  /* Allocate initial segments */
+ i =3D 0;
  for (segp =3D hashp->dir; hctl->nsegs < nsegs; hctl->nsegs++, segp++)
  {
- *segp =3D seg_alloc(hashp);
- if (*segp =3D=3D NULL)
- return false;
+ /* Assign initial segments, which are also pre-allocated */
+ if (hashp->isshared)
+ {
+ *segp =3D (HASHSEGMENT) HASH_SEGMENT_PTR(hashp, i++);
+ MemSet(*segp, 0, HASH_SEGMENT_SIZE(hashp));
+ }
+ else
+ {
+ *segp =3D seg_alloc(hashp);
+ i++;
+ }

In the non-shared hash table case, previously, we used to return false
from here when seg_alloc() fails, but now it seems to be proceeding
without returning false. Is there a reason for the same?

> I tested this by running make-check, make-check world and the reproducer =
script shared
> by David. I also ran pgbench to test creation and expansion of some of th=
e
> shared hash tables.
>

This covers the basic tests for this patch. I think we should do some
low-level testing of both shared and non-shared hash tables by having
a contrib module or such (we don't need to commit such a contrib
module, but it will give us confidence that the low-level data
structure allocation change is thoroughly tested). We also need to
focus on negative tests where there is insufficient memory in the
system.

--=20
With Regards,
Amit Kapila.