Re: Potential GIN vacuum bug

Jeff Janes <jeff(dot)janes(at)gmail(dot)com> writes:
> Attached is a patch to deal with this without the heavyweight locks.
> I realized that using the clean up lock on the meta page, rather than the
> pending head page, would be easier to implement as a pin was already held
> on the meta page throughout, and the pending head page can change during
> the cleanup if we need multiple passes.
> ...
> I still prefer the heavy-weight approach. The buffer clean up lock for
> vacuuming seems fragile to start with, and abusing it for other purposes
> doesn't improve on that.

FWIW, I would go with the heavyweight lock approach as well. The extra
cycles needed for a heavyweight lock don't seem significant in this
context, and you have far more control over which other operations
conflict or don't conflict with the lock. Taking a buffer cleanup lock on
the metapage sounds really scary from that viewpoint; it's basically going
to conflict with everything else, even if the other operations only take
it for short intervals, and you have no freedom to adjust that.

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.
Perhaps we could make the logic in insertion be something like

if (pending-list-size > threshold)
{
if (conditional-lock-acquire(...))
{
do-pending-list-cleanup;
lock-release;
}
else if (pending-list-size > threshold * 2)
{
unconditional-lock-acquire(...);
if (pending-list-size > threshold)
do-pending-list-cleanup;
lock-release;
}
}

so that once the pending list got too big, incoming insertions would wait
for it to be cleared. Whether to use a 2x safety margin or something else
could be a subject for debate, of course.

regards, tom lane