Horton

Re: Unhappy about API changes in the no-fsm-for-small-rels patch

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Amit Kapila <amit.kapila16@gmail.com>, John Naylor <john.naylor@2ndquadrant.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2019-04-23T17:39:13Z
Lists: pgsql-hackers 
Hi,

On 2019-04-23 13:31:25 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2019-04-23 15:46:17 +0530, Amit Kapila wrote:
> >> If we invalidate it only when there's no space on the page, then when
> >> should we set it back to available, because if we don't do that, then
> >> we might miss the space due to concurrent deletes.
> 
> > Well, deletes don't traditionally (i.e. with an actual FSM) mark free
> > space as available (for heap). I think RecordPageWithFreeSpace() should
> > issue a invalidation if there's no FSM, and the block goes from full to
> > empty (as there's no half-full IIUC).
> 
> Why wouldn't we implement this just as a mini four-entry FSM stored in
> the relcache, and update the entries according to the same rules we'd
> use for regular FSM entries?

I mean the big difference is that it's not shared memory. So there needs
to be some difference. My suggestion to handle that is to just issue an
invalidation when *increasing* the amount of space.

And sure, leaving that aside we could store one byte per block - it's
just not what the patch has done so far (or rather, it used one byte per
block, but only utilized one bit of it).  It's possible that'd come with
some overhead - I've not thought sufficiently about it: I assume we'd
still start out in each backend assuming each page is empty, and we'd
then rely on RelationGetBufferForTuple() to update that. What I wonder
is if we'd need to check if an on-disk FSM has been created every time
the space on a page is reduced?  I think not, if we use invalidations to
notify others of the existance of an on-disk FSM. There's a small race,
but that seems ok.

Greetings,

Andres Freund

Commits

  1. Revert "Avoid the creation of the free space map for small heap relations".

  2. Improve code comments in b0eaa4c51b.

  3. During pg_upgrade, conditionally skip transfer of FSMs.

  4. Add more tests for FSM.

  5. Fix typos in commit 8586bf7ed8.

  6. tableam: introduce table AM infrastructure.

  7. Clear the local map when not used.

  8. Doc: Update the documentation for FSM behavior for small tables.

  9. Make FSM test portable.

  10. Avoid creation of the free space map for small heap relations, take 2.