Re: Potential GIN vacuum bug - Mailing list pgsql-hackers

Jeff Janes <jeff.janes@gmail.com> writes:
> On Sun, Aug 30, 2015 at 11:11 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Your earlier point about how the current design throttles insertions to
>> keep the pending list from growing without bound seems like a bigger deal
>> to worry about.  I think we'd like to have some substitute for that.
>> ...

> If the goal is to not change existing behavior (like for back patching) the
> margin should be 1, always wait.

The current behavior is buggy, both as to performance and correctness,
so I'm not sure how come "don't change the behavior" should be a
requirement.

> 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.)

> Would we bother to back-patch a theoretical bug which there is no evidence
> is triggering in the field?

What's theoretical about it?  You seemed pretty sure that the issue in
http://www.postgresql.org/message-id/flat/CA+bfosGVGVQhMAa=0-mUE6cOo7dBSgAYxb-XsnR5vm-S39hpNg@mail.gmail.com
was exactly this.

> If we want to improve the current behavior rather than fix a bug, then I
> think that if the list is greater than threshold*2 and the cleaning lock is
> unavailable, what it should do is proceed to insert the tuple's keys into
> the index itself, as if fastupdate = off.  That would require some major
> surgery to the existing code, as by the time it invokes the clean up, it is
> too late to not insert into the pending list.

Meh.  That's introducing a whole new behavioral regime, which quite aside
from correctness bugs might introduce new performance bugs of its own.
It certainly doesn't sound like a better back-patch candidate than the
other thing.
        regards, tom lane