On Sat, May 4, 2019 at 2:55 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Fri, May 3, 2019 at 2:14 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Fri, May 3, 2019 at 11:43 AM John Naylor <john.nay...@2ndquadrant.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