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-18T21:10:29Z
Lists: pgsql-hackers 
Hi,

On 2019-04-17 12:20:29 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2019-04-17 15:49:29 +0530, Amit Kapila wrote:
> >> OTOH, if we want to extend it later for whatever reason to a relation
> >> level cache, it shouldn't be that difficult as the implementation is
> >> mostly contained in freespace.c (fsm* functions) and I think the
> >> relation is accessible in most places.  We might need to rip out some
> >> calls to clearlocalmap.
> 
> > But it really isn't contained to freespace.c. That's my primary
> > concern. You added new parameters (undocumented ones!),
> > FSMClearLocalMap() needs to be called by callers and xlog, etc.
> 
> Given where we are in the release cycle, and the major architectural
> concerns that have been raised about this patch, should we just
> revert it and try again in v13, rather than trying to fix it under
> time pressure?  It's not like there's not anything else on our
> plates to fix before beta.

Hm. I'm of split mind here:

It's a nice improvement, and the fixes probably wouldn't be that
hard. And we could have piped up a bit earlier about these concerns (I
only noticed this when rebasing zheap onto the newest version of
postgres).

But as you it's also late, and there's other stuff to do. Although I
think neither Amit nor John is heavily involved in any...

My compromise suggestion would be to try to give John and Amit ~2 weeks
to come up with a cleanup proposal, and then decide whether to 1) revert
2) apply the new patch, 3) decide to live with the warts for 12, and
apply the patch in 13. As we would already have a patch, 3) seems like
it'd be more tenable than without.

Regards,

Andres

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.