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

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: John Naylor <john(dot)naylor(at)2ndquadrant(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, 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-05 13:25:30
Message-ID: CAA4eK1JaUnHHZSTn-1mjEo1OLRvy2ysFyi4rKr8dKG_GaKu7QA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, May 4, 2019 at 2:55 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Fri, May 3, 2019 at 2:14 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Fri, May 3, 2019 at 11:43 AM John Naylor <john(dot)naylor(at)2ndquadrant(dot)com> wrote:
>
> Attached is a revert patch. John, can you please once double-check to
> ensure I have not missed anything?
>
> To summarize for everyone: This patch avoids the fsm creation for
> small relations (which is a small but good improvement as it saves
> space). This patch was using a process local map to track the first
> few blocks and was reset as soon as we get the block with enough free
> space. It was discussed in this thread that it would be better to
> track the local map in relcache and then invalidate it whenever vacuum
> frees up space in the page or when FSM is created. There is a
> prototype patch written for the same, but it is not 100% clear to me
> that the new idea would be a win in all cases (like code complexity or
> API design-wise) especially because resetting the map is not
> straight-forward. As time was not enough, we couldn't complete the
> patch from all aspects to see if it is really better in all cases.
>
> We have two options (a) revert this patch and try the new approach in
> next release, (b) keep the current patch and replace with the new
> approach if it turns out to be better in next release.
>
> So, do we want to keep this feature for this release?
>
> I am fine going with option (a), that's why I have prepared a revert
> patch, but I have a slight fear that the other option might not turn
> out to be better and even if it is then we can anyway replace it as
> shown in the prototype, so going with option (b) doesn't sound to be
> dumb.
>
> Anybody else wants to weigh in?
>

I understand that we have to take a call here shortly, but as there is
a weekend so I would like to wait for another day to see if anyone
else wants to share his opinion.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2019-05-05 14:16:45 Re: Inconsistent error message wording for REINDEX CONCURRENTLY
Previous Message Bruce Momjian 2019-05-05 12:09:02 Re: Logging the feature of SQL-level read/write commits