Ildus Kurbangaliev wrote:

> Thanks for the review. I've attached a new version of SLRU patch. I've
> removed add_postfix and fixed EXEC_BACKEND case.


Please do not use "committs" in commit_ts.c; people didn't like the
abbreviated name without the underscore.  But then, why are we
abbreviating here?  We could keep it complete and with a space instead
of underscore, so why not use just "commit timestamp", because it's just
a string, right?

In multixact.c, is there a reason to have underscore in the strings?  We
could substitute it with a space and it'd look prettier; but really, we
could also keep those names parallel to subdirectory names by using the
already existing string parameter as name here, and not add another one.

I also imagined that the Slru's ControlLock itself would be part of the
tranche, and not just the per-buffer locks.  That requires a bit more
churn, but seems reasonable.

Why do we have two per-buffer loops in SimpleLruInit?  I mean, why not
add the LWLockInitialize call to the second one?

I'm up to speed on how the LWLockTranche API works -- does assigning to
tranche_name a pstrdup string work okay?  Is the pstrdup really

>       /* Initialize our shared state struct */
> diff --git a/src/backend/access/transam/slru.c 
> b/src/backend/access/transam/slru.c
> index 90c7cf5..868b35a 100644
> --- a/src/backend/access/transam/slru.c
> +++ b/src/backend/access/transam/slru.c
> @@ -157,6 +157,8 @@ SimpleLruShmemSize(int nslots, int nlsns)
>       if (nlsns > 0)
>               sz += MAXALIGN(nslots * nlsns * sizeof(XLogRecPtr));    /* 
> group_lsn[] */
> +     sz += MAXALIGN(nslots * sizeof(LWLockPadded)); /* lwlocks[] */
> +
>       return BUFFERALIGN(sz) + BLCKSZ * nslots;
>  }

What is the "lwlocks[]" comment supposed to mean?  I don't think there's
a struct member with that name, is there?

Uhm, actually, why do we keep buffer_locks[] at all?  This arrangement
seems pretty odd, where if I understand correctly we have one array
which is the tranche and another array which points to each item in the
tranche ...

Álvaro Herrera      
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Sent via pgsql-hackers mailing list (
To make changes to your subscription:

Reply via email to