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

Andres Freund <andres@anarazel.de>

From: Andres Freund <andres@anarazel.de>
To: Bruce Momjian <bruce@momjian.us>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Amit Kapila <amit.kapila16@gmail.com>, John Naylor <john.naylor@2ndquadrant.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2019-05-01T16:08:54Z
Lists: pgsql-hackers
Hi,

On 2019-05-01 11:28:11 -0400, Bruce Momjian wrote:
> On Wed, May  1, 2019 at 08:24:25AM -0700, Andres Freund wrote:
> > Hi,
> >
> > On 2019-04-18 14:10:29 -0700, Andres Freund wrote:
> > > 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.
> >
> > I think decision time has come. My tentative impression is that we're
> > not there yet, and should revert the improvements in v12, and apply the
> > improved version early in v13. As a second choice, we should live with
> > the current approach, if John and Amit "promise" further effort to clean
> > this up for v13.
>
> My ignorant opinion is that I have been surprised by the churn caused by
> this change, and have therefore questioned the value of it.

Hm, I don't think there has been that much churn? Sure, there was a
revert to figure out a regression test instability, but that doesn't
seem that bad. Relevant commits in date order are:


andres-classification: cleanup
commit 06c8a5090ed9ec188557a86d4de11384f5128ec0
Author: Amit Kapila <akapila@postgresql.org>
Date:   2019-03-16 06:55:56 +0530

    Improve code comments in b0eaa4c51b.

    Author: John Naylor
    Discussion: https://postgr.es/m/CACPNZCswjyGJxTT=mxHgK=Z=mJ9uJ4WEx_UO=bNwpR_i0EaHHg@mail.gmail.com


andres-classification: incremental improvement
commit 13e8643bfc29d3c1455c0946281cdfc24758ffb6
Author: Amit Kapila <akapila@postgresql.org>
Date:   2019-03-15 08:25:57 +0530

    During pg_upgrade, conditionally skip transfer of FSMs.


andres-classification: additional tests
commit 6f918159a97acf76ee2512e44f5ed5dcaaa0d923
Author: Amit Kapila <akapila@postgresql.org>
Date:   2019-03-12 08:14:28 +0530

    Add more tests for FSM.


andres-classification: cleanup
commit a6e48da08844eeb5a72c8b59dad3aaab6e891fac
Author: Amit Kapila <akapila@postgresql.org>
Date:   2019-03-11 08:16:14 +0530

    Fix typos in commit 8586bf7ed8.


andres-classification: bugfix
commit 9c32e4c35026bd52aaf340bfe7594abc653e42f0
Author: Amit Kapila <akapila@postgresql.org>
Date:   2019-03-01 07:38:47 +0530

    Clear the local map when not used.


andres-classification: docs addition
commit 29d108cdecbe918452e70041d802cc515b2d56b8
Author: Amit Kapila <akapila@postgresql.org>
Date:   2019-02-20 17:37:39 +0530

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


andres-classification: regression test stability
commit 08ecdfe7e5e0a31efbe1d58fefbe085b53bc79ca
Author: Amit Kapila <akapila@postgresql.org>
Date:   2019-02-04 10:08:29 +0530

    Make FSM test portable.


andres-classification: feature
commit b0eaa4c51bbff3e3c600b11e5d104d6feb9ca77f
Author: Amit Kapila <akapila@postgresql.org>
Date:   2019-02-04 07:49:15 +0530

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


So sure, there's a few typo fixes, one bugfix, and one buildfarm test
stability issue. Doesn't seem crazy for a nontrivial improvement.


> Frankly, there has been so much churn I am unclear if it can be easily reverted.

Doesn't seem that hard? There's some minor conflicts, but nothing bad?

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.