Re: Potential GIN vacuum bug

On Sun, Aug 30, 2015 at 6:57 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> But we would still have to deal with the
>> fact that unconditional acquire attempt by the backends will cause a vacuum
>> to cancel itself, which is undesirable.
>
> Good point.
>
>> If we define a new namespace for
>> this lock (like the relation extension lock has its own namespace) then
>> perhaps the cancellation code could be made to not cancel on that
>> condition. But that too seems like a lot of work to backpatch.
>
> We could possibly teach the autocancel logic to distinguish this lock type
> from others without using a new namespace. That seems a bit klugy, but
> maybe better than adding a new namespace. (For example, there are
> probably only a couple of modes in which we take page-level locks at
> present. Choosing a currently unused, but self-exclusive, mode for taking
> such a lock might serve.)

That seems like a pretty big kludge to me; it will work until it doesn't.

Adding a new lock type (similar to "relation extension") would address
a lot of my concern with the heavyweight lock approach. It strikes me
that trying to grab a lock on the index in what's basically a pretty
low-level operation here could have a variety of unintended
consequences. The autovacuum thing is one; but consider also the risk
of new deadlock scenarios resulting from a lock upgrade. Those
deadlocks would likely be, to use Peter Geoghegan's term,
unprincipled. The locking regime around indexes is already pretty
complicated, and I'm skeptical about the idea that we can complicate
it further without any blowback.

If we use a new lock type, it's a lot easier to reason about the
interactions, I think. We know all of the things that will take that
lock type. And we can be reasonably confident that future code
changes won't introduce any new ones, or that the current ones will be
considered before making changes.

It's not great to have to back-patch such a change, but in practice
the blast radius should be pretty limited. People who are looking at
pg_locks might start to see a new lock type show up that they're not
expecting, but a lot of people either aren't looking at that data at
all, or are looking at it but not doing anything programmatic with it
and therefore won't really be impacted by something new showing up.

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