Re: autoprewarm is fogetting to register a tranche.

On Fri, Dec 15, 2017 at 3:32 AM, Kyotaro HORIGUCHI
<horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> Hello, I noticed while an investigation that pg_prewarm is
> forgetting to register a tranche.

Oops.

> In passing, I found it hard to register a tranche in
> apw_init_shmem() because a process using the initialized shared
> memory struct cannot find the tranche id (without intruding into
> the internal of LWLock strcut). So I'd like to propose another
> tranche registering interface LWLockRegisterTrancheOfLock(LWLock
> *, int). The interface gets rid of the necessity of storing
> tranche id separately from LWLock. (in patch 0001)

I don't think we really need this. If it suits a particular caller to
to pass somelock->tranche to LWLockRegisterTranche, fine, but they can
do that with or without this function. It's basically a one-line
function, so I don't see the point.

> The comment cited above looks a bit different from how the
> interface is actually used. So I rearranged the comment following
> a typical use I have seen in several cases. (in patch 0001)

I don't really see a need to change this. It's true that what's
currently #3 could be done before #2, but I hesitate to call that a
typical practice. Also, I'm worried that it could create the
impression that it's OK to use an LWLock before registering the
tranche, and it's really not.

> The final patch 0003 should be arguable, or anticipated to be
> rejected. It cannot be detect that a tranche is not registered
> until its name is actually accessed (or many eewon't care even if
> the name were printed as '(null)' in an error message that is the
> result of the developer's own bug). On the other hand there's no
> chance to detect that before the lock is actually used. By the
> last patch I added an assertion in LWLockAcquire() but it must
> hit performance in certain dgree (even if it is only in
> --enable-cassert build) so I don't insisit that it must be there.

Actually, I think this is a good idea as long as it doesn't hurt
performance too much. It catches something that would otherwise be
hard to check.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company