| From: | Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com> |
|---|---|
| To: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
| Cc: | Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, chaturvedipalak1911(at)gmail(dot)com |
| Subject: | Re: Better shared data structure management and resizable shared data structures |
| Date: | 2026-04-05 19:58:52 |
| Message-ID: | CAEze2Wio0oudQatHmCRYzurnmBv2_p_muACP2=VkRwiMuJPiMw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Sun, 5 Apr 2026 at 17:07, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>
> On 05/04/2026 02:17, Matthias van de Meent wrote:
> > Is this malformatting caused by pgindent? If so, could you see if
> > there's a better way of defining ShmemRequestHash/Struct that doesn't
> > have this indent as output?
>
> Yeah, that's pgindent. Matter of taste, but I think that looks fine. An
> alternative is to put the closing bracket on the same line with the last
> argument and drop the trailing comma:
>
> ShmemRequestStruct(.name = "pg_stat_statements",
> .size = sizeof(pgssSharedState),
> .ptr = (void **) &pgss);
>
> That looks OK to me too.
Then let's keep it as per the v11 patch with ugly closing indents --
hopefully someone will fix pgindent in the future, but that's not the
job of this patch.
> > You could make sure that this isn't an issue by maintaining a flag
> > in lwlock.c that's set when the shmem request is made (and reset on
> > shmem exit), which must be false when RequestNamedLWLockTranche() is
> > called, and if not then it should throw an error.
> I'll change it so that the number of locks calculated in
> LWLockShmemRequest() is stored in a global variable, and
> LWLockShmemInit() has an Assert() to cross-checks with that. That
> catches the bug and seems like a good cross-check in general.
Thanks for fixing this!
> > 0010: Not looked at everything yet, but a few comment:
> >
> >> +++ b/src/include/access/slru.h
> >
> > With the changes in the signatures for most/all SLRU functions from a
> > hidden-by-typedef pointer to a visible pointer type, maybe this could
> > be an opportunity to swap them to `const SlruDesc *ctl` wherever
> > possible? I don't think there are many backend-local changes that
> > happen to SlruDescs once we've properly started the backend. I'm happy
> > to provide an incremental patch if you'd like me to spend cycles on it
> > if you're busy.
>
> Yeah sounds like a good idea.
Attached 2 incremental patches:
0001 constifies the expected function arguments, which I propose to include; and
0002 adds 'type* const struct fields' in SlruShared.
0002 was not requested, but it looked feasible to at least try it out
in this subsystem. It might be interesting, but you're free to drop
it.
Kind regards,
Matthias van de Meent
Databricks (https://www.databricks.com)
| Attachment | Content-Type | Size |
|---|---|---|
| nocfbot.v11-0002-Const-qualify-SlruShared-s-fields-and-i.patch | application/octet-stream | 6.7 KB |
| nocfbot.v11-0001-Add-const-qualification-in-SLRU-subsyst.patch | application/octet-stream | 15.3 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Andres Freund | 2026-04-05 20:03:49 | Re: pg_get__*_ddl consolidation |
| Previous Message | Álvaro Herrera | 2026-04-05 19:44:48 | Re: tid_blockno() and tid_offset() accessor functions |