Re: Add cassert-only checks against unlocked use of relations

On 2013-11-05 16:45:49 -0500, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > On 2013-11-05 16:25:53 -0500, Tom Lane wrote:
> >> If we're sufficiently worried by this type of bug, ISTM we'd be better off
> >> just disallowing heap_open(NoLock). At the time we invented that, every
> >> lock request went to shared memory; but now that we have the local lock
> >> table, re-locking just requires a local hash lookup followed by
> >> incrementing a local counter. That's probably pretty cheap --- certainly
> >> a lot cheaper than what you've got here.
>
> > Hm. That only works though if we're using the same lockmode as before -
> > often enough the *_open(NoLock) checks would use a weaker locklevel than
> > the previous lock. So I think the cost of doing so would probably be
> > noticeable.
>
> If you're not using the same lockmode as before, it's probably a bug anyway.
> As I said already, the entire NoLock coding technique is dependent on
> having a very clear idea of which previous lock-taking you're riding
> on the coattails of. Why wouldn't you duplicate that lock level,
> if we say you can't use NoLock anymore?

Hm. That doesn't really seem to match my reading of the code. There's
quite some code around that relies the fact that the parser has taken an
appropriately strong lock already. E.g. the parser chooses the lockmode
in addRangeTableEntry() - I don't think we want to add duplicate
isLockedRefname() calls everywhere.
Other users of that relation will likely just use AccessShareLock since
that's sufficient for their own use.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services