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 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.
However, I don't think using RecordAndGetPageWithFreeSpace rather than
GetPageWithFreeSpace is appropriate. We've already recorded free
space on that page, and recording it again is a bad idea. It's quite
possible that by the time we get the lock our old value is totally
inaccurate. If there's some advantage to searching in the more
targeted way that RecordAndGetPageWithFreeSpace does over
GetPageWithFreeSpace then we need a new API into the freespace stuff
that does the more targeted search without updating anything.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company