On Tue, May 11, 2021 at 11:07 PM Tomas Vondra
<tomas.vondra@enterprisedb.com> wrote:
>
> On 5/11/21 11:04 AM, Masahiko Sawada wrote:
> > On Tue, May 11, 2021 at 4:37 PM Michael Paquier <michael@paquier.xyz> wrote:
> >>
> >> On Wed, May 05, 2021 at 03:04:53PM +0200, Tomas Vondra wrote:
> >>> Thanks, that looks promising. I repeated the tests I did on 26/4, and the
> >>> results look like this:
> >>>
> >>> old (0c7d3bb99): 497ms
> >>> master: 621ms
> >>> patched: 531ms
> >>>
> >>> So yeah, that's a bit improvement - it does not remove the regression
> >>> entirely, but +5% is much better than +25%.
> >>
> >> Hmm. Is that really something we should do after feature freeze? A
> >> 25% degradation for matview refresh may be a problem for a lot of
> >> users and could be an upgrade stopper. Another thing we could do is
> >> also to revert 7db0cd2 and 39b66a9 from the v14 tree, and work on a
> >> proper solution for this performance problem for matviews for 15~.
> >
> > I think the approach proposed by Andres eliminates the extra vmbuffer
> > reads as much as possible. But even with the patch, there still is 5%
> > degradation (and there is no way to disable inserting frozen tuples at
> > matview refresh). Which could be a problem for some users. I think
> > it’s hard to completely eliminate the overhead so we might need to
> > consider another approach like having matview refresh use
> > heap_multi_insert() instead of heap_insert().
> >
>
> I think it's way too late to make such significant change (switching to
> heap_multi_insert) for v14 :-(
Right.
> Moreover, I doubt it affects just matview
> refresh - why wouldn't it affect other similar use cases? More likely
> it's just the case that was discovered.
I've not tested yet but I guess COPY FROM … FREEZE using heap_insert
would similarly be affected since it also uses heap_insert() with
TABLE_INSERT_FROZEN.
>
> > I think the changes for heap_multi_insert() are fine so we can revert
> > only heap_insert() part if we revert something from the v14 tree,
> > although we will end up not inserting frozen tuples into toast tables.
> >
>
> I'd be somewhat unhappy about reverting just this bit, because it'd mean
> that we freeze rows in the main table but not rows in the TOAST tables
> (that was kinda why we concluded we need the heap_insert part too).
>
> I'm still a bit puzzled where does the extra overhead (in cases when
> freeze is not requested) come from, TBH.
Which cases do you mean? Doesn't matview refresh always request to
freeze tuples even after applying the patch proposed on this thread?
Regards,
--
Masahiko Sawada
EDB: https://www.enterprisedb.com/
