On 23/03/16 20:43, Robert Haas wrote:
> On Wed, Mar 23, 2016 at 3:33 PM, Petr Jelinek <petr@2ndquadrant.com> wrote:
>> Btw thinking about it some more, ISTM that not finding the block and just
>> doing the extension if the FSM wasn't extended correctly previously is
>> probably cleaner behavior than what we do now. The reasoning for that
>> opinion is that if the FSM wasn't extended, we'll fix it by doing relation
>> extension since we know we do both in this code path and also if we could
>> not find page before we'll most likely not find one even on retry and if
>> there was page added at the end by extension that we might reuse partially
>> here then there is no harm in adding new one anyway as the whole point of
>> this patch is that it does bigger extension that strictly necessary so
>> insisting on page reuse for something that seems like only theoretical
>> possibility that does not even exist in current code does not seem right.
>
> I'm not sure I completely follow this. The fact that the last
> sentence is 9 lines long may be related. :-)
>
I tend to do that sometimes :)
> I think it's pretty clearly important to re-check the FSM after
> acquiring the extension lock. Otherwise, imagine that 25 backends
> arrive at the exact same time. The first one gets the lock and
> extends the relation 500 pages; the next one, 480, and so on. In
> total, they extend the relation by 6500 pages, which is a bit rich.
> Rechecking the FSM after acquiring the lock prevents that from
> happening, and that's a very good thing. We'll do one 500-page
> extension and that's it.
>
Right, but that would only happen if all the backends did it using
different code which does not do the FSM extension because the current
code does FSM extension and the point of using
RecordAndGetPageWithFreeSpace seems to be "just in case" somebody is
doing extension differently (at least I don't see other reason). So
basically I am not saying we shouldn't do the search but that I agree
GetPageWithFreeSpace should be enough as the worst that can happen is
that we overextend the relation in case some theoretical code from
somewhere else also did extension of relation without extending FSM
(afaics).
But maybe Dilip had some other reason for using the
RecordAndGetPageWithFreeSpace that is not documented.
-- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services