On 23/03/16 20:43, Robert Haas wrote:
On Wed, Mar 23, 2016 at 3:33 PM, Petr Jelinek <p...@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

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to