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.
Thanks. 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 necessary? > /* 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 http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
