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-02T08:56:52Z
Lists: pgsql-hackers

Attachments

On Thu, May 2, 2019 at 12:39 PM John Naylor <john.naylor@2ndquadrant.com> wrote:
>
> On Thu, May 2, 2019 at 2:31 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > @@ -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?
>
> Yes, you're quite correct.
>

Okay,  I have updated the patch to incorporate your changes and call
relcache invalidation at required places. I have updated comments at a
few places as well.  The summarization of this patch is that (a) it
moves the local map to relation cache (b) performs the cache
invalidation whenever we create fsm (either via backend or vacuum),
when some space in a page is freed by vacuum (provided fsm doesn't
exist) or whenever the local map is cleared (c) additionally, we clear
the local map when we found a block from FSM, when we have already
tried all the blocks present in cache or when we are allowed to create
FSM.

If we agree on this, then we can update the README accordingly.

Can you please test/review?

-- 
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.