public inbox for [email protected]  
help / color / mirror / Atom feed
From: Amit Kapila <[email protected]>
To: Rahila Syed <[email protected]>
Cc: Tomas Vondra <[email protected]>
Cc: Nazir Bilal Yavuz <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: Improve monitoring of shared memory allocations
Date: Tue, 13 May 2025 16:34:05 +0530
Message-ID: <CAA4eK1+GXW4uKzzT45Zd14QKFNQ34wtO7FOUMe2wZvJJONgdmA@mail.gmail.com> (raw)
In-Reply-To: <CAH2L28vgzvTUqNwQay=jx4w30sHMx_pC+emnZErv8oX0R+SALQ@mail.gmail.com>
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>
	<[email protected]>
	<CAH2L28uG_g1Ljo8aL-g1MupJXO4Y7-a-bUCriE7w2213+KSGdA@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CAH2L28tzCFEk2bxQ+oYv6zda=LFLfd_9cmq7HzsT4nj9KN1Yvg@mail.gmail.com>
	<[email protected]>
	<CAH2L28soW40DafNmLEEWMrNDTMWYZ9STWSS5axP910WQUT=JcQ@mail.gmail.com>
	<[email protected]>
	<CAH2L28vaktm0UXw19y22w+DThcqV80-UKtVfMxPeVw8ro2yTDg@mail.gmail.com>
	<[email protected]>
	<CAH2L28vgzvTUqNwQay=jx4w30sHMx_pC+emnZErv8oX0R+SALQ@mail.gmail.com>

On Fri, Apr 4, 2025 at 9:15 PM Rahila Syed <[email protected]> wrote:
>
> Please find attached the patch which removes the changes for non-shared hash tables
> and keeps them for shared hash tables.
>

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

-- 
With Regards,
Amit Kapila.





view thread (20+ 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: Improve monitoring of shared memory allocations
  In-Reply-To: <CAA4eK1+GXW4uKzzT45Zd14QKFNQ34wtO7FOUMe2wZvJJONgdmA@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