Horton

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

Amit Kapila <amit.kapila16@gmail.com>

View thread View on postgr.es
From: Amit Kapila <amit.kapila16@gmail.com>
To: John Naylor <john.naylor@2ndquadrant.com>
Cc: Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2019-05-02T06:31:27Z
Lists: pgsql-hackers 
On Tue, Apr 30, 2019 at 11:42 AM John Naylor
<john.naylor@2ndquadrant.com> wrote:
>
> On Fri, Apr 26, 2019 at 11:52 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > As discussed above, we need to issue an
> > invalidation for following points:  (a) when vacuum finds there is no
> > FSM and page has more space now, I think you can detect this in
> > RecordPageWithFreeSpace
>
> I took a brief look and we'd have to know how much space was there
> before. That doesn't seem possible without first implementing the idea
> to save free space locally in the same way the FSM does. Even if we
> have consensus on that, there's no code for it, and we're running out
> of time.
>
> > (b) invalidation to notify the existence of
> > FSM, this can be done both by vacuum and backend.
>
> I still don't claim to be anything but naive in this area, but does
> the attached get us any closer?
>

@@ -776,7 +776,10 @@ fsm_extend(Relation rel, BlockNumber fsm_nblocks)
  if ((rel->rd_smgr->smgr_fsm_nblocks == 0 ||
  rel->rd_smgr->smgr_fsm_nblocks == InvalidBlockNumber) &&
  !smgrexists(rel->rd_smgr, FSM_FORKNUM))
+ {
  smgrcreate(rel->rd_smgr, FSM_FORKNUM, false);
+ fsm_clear_local_map(rel);
+ }

I think this won't be correct because when we call fsm_extend via
vacuum the local map won't be already existing, so it won't issue an
invalidation call.  Isn't it better to directly call
CacheInvalidateRelcache here to notify other backends that their local
maps are invalid now?

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

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.