On Tue, May 11, 2021 at 11:07 PM Tomas Vondra <tomas.von...@enterprisedb.com> wrote: > > On 5/11/21 11:04 AM, Masahiko Sawada wrote: > > On Tue, May 11, 2021 at 4:37 PM Michael Paquier <mich...@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/
