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

From: Andres Freund <andres(at)anarazel(dot)de>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, John Naylor <john(dot)naylor(at)2ndquadrant(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Unhappy about API changes in the no-fsm-for-small-rels patch
Date: 2019-05-01 16:08:54
Message-ID: 20190501160854.zs2sdeiriwuxpmkq@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
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(at)postgresql(dot)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(at)postgresql(dot)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(at)postgresql(dot)org>
Date: 2019-03-12 08:14:28 +0530

Add more tests for FSM.

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

Fix typos in commit 8586bf7ed8.

andres-classification: bugfix
commit 9c32e4c35026bd52aaf340bfe7594abc653e42f0
Author: Amit Kapila <akapila(at)postgresql(dot)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(at)postgresql(dot)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(at)postgresql(dot)org>
Date: 2019-02-04 10:08:29 +0530

Make FSM test portable.

andres-classification: feature
commit b0eaa4c51bbff3e3c600b11e5d104d6feb9ca77f
Author: Amit Kapila <akapila(at)postgresql(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2019-05-01 16:11:51 Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
Previous Message Andres Freund 2019-05-01 15:59:12 Re: hyrax vs. RelationBuildPartitionDesc