Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> Tom Lane wrote:
>> I noticed that several places in brin_pageops.c do this sort of thing:
>> ...
>> ie they put *one* page into the index's free space map and then invoke a
>> scan of the index's entire FSM to ensure that that info is bubbled up to
>> the top immediately. I wonder how carefully this was thought through.
>> We don't generally think that it's worth doing an FSM vacuum for a single
>> page worth of free space.
> Yeah, there was no precedent for doing that, but it was actually
> intentional: there were enough cases where we would extend the relation,
> only to have the operation fail for unrelated reasons (things like a
> concurrent heap insertion), so it was possible to lose free space very
> fast, and since BRIN indexes are slow to grow it would become
> noticeable. We could have waited for FreeSpaceMapVacuum to occur
> naturally, but this would cause the index to bloat until next vacuum,
> which could happen a long time later. Also, the idea behind BRIN is
> that the indexed tables are very large, so vacuuming is infrequent.
Fair enough. It seemed like an odd thing, but as long as it was
intentional I'm good with it.
>> some places that do RecordPageWithFreeSpace follow it up with an immediate
>> FreeSpaceMapVacuum, and some don't.
> Hmm I analyzed the callsites carefully to ensure it wasn't possible to
> bail out without vacuuming in the normal corner cases. Maybe you
> spotted some gap in my analysis -- or worse, maybe I had to change
> nearby code for unrelated reasons and broke it afterwards inadvertently.
Well, I did not trace the logic fully, but there are places that record
free space and don't have an obviously connected FreeSpaceMapVacuum
call. Maybe those all expect the caller to do it.
> I agree that brin_getinsertbuffer should have a better comment to
> explain the intention (and that routine seems to carry the most guilt in
> failing to do the FSM vacuuming).
Got a proposal?
I can take care of the other stuff, unless you wanted to.
regards, tom lane