Re: Performance degradation of REFRESH MATERIALIZED VIEW
Tomas Vondra <tomas.vondra@enterprisedb.com>
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
To: Masahiko Sawada <sawada.mshk@gmail.com>,
Andres Freund <andres@anarazel.de>
Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>,
Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
PostgreSQL-development <pgsql-hackers@postgresql.org>,
Pavan Deolasee <pavan.deolasee@gmail.com>,
Anastasia Lubennikova <a.lubennikova@postgrespro.ru>,
Jeff Janes <jeff.janes@gmail.com>, Paul Guo <guopa@vmware.com>
Date: 2021-04-27T13:43:07Z
Lists: pgsql-hackers
On 4/27/21 7:34 AM, Masahiko Sawada wrote: > On Tue, Apr 27, 2021 at 8:07 AM Andres Freund <andres@anarazel.de> wrote: >> >> Hi, >> >> On 2021-04-26 23:59:17 +0200, Tomas Vondra wrote: >>> On 4/26/21 9:27 PM, Andres Freund wrote: >>>> On 2021-04-26 15:31:02 +0200, Tomas Vondra wrote: >>>>> I'm not sure what to do about this :-( I don't have any ideas about how to >>>>> eliminate this overhead, so the only option I see is reverting the changes >>>>> in heap_insert. Unfortunately, that'd mean inserts into TOAST tables won't >>>>> be frozen ... >>>> >>>> ISTM that the fundamental issue here is not that we acquire pins that we >>>> shouldn't, but that we do so at a much higher frequency than needed. >>>> >>>> It's probably too invasive for 14, but I think it might be worth exploring >>>> passing down a BulkInsertState in nodeModifyTable.c's table_tuple_insert() iff >>>> the input will be more than one row. >>>> >>>> And then add the vm buffer of the target page to BulkInsertState, so that >>>> hio.c can avoid re-pinning the buffer. >>>> >>> >>> Yeah. The question still is what to do about 14, though. Shall we leave the >>> code as it is now, or should we change it somehow? It seem a bit unfortunate >>> that a COPY FREEZE optimization should negatively influence other (more) >>> common use cases, so I guess we can't just keep the current code ... >> >> I'd suggest prototyping the use of BulkInsertState in nodeModifyTable.c >> and see whether that fixes the regression. > > Is this idea to have RelationGetBufferForTuple() skip re-pinning > vmbuffer? If so, is this essentially the same as the one in the v3 > patch? > I don't think it is the same approach - it's a bit hard to follow what exactly happens in RelationGetBufferForTuple, but AFAICS it always starts with vmbuffer = InvalidBuffer, so it may pin the vmbuffer quite often, no? What Andres is suggesting (I think) is to modify ExecInsert() to pass a valid bistate to table_tuple_insert, instead of just NULL, and store the vmbuffer in it. Not sure how to identify when inserting more than just a single row, though ... regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Commits
-
Fix pg_visibility regression failure with CLOBBER_CACHE_ALWAYS
- d1f0aa769691 14.0 landed
-
Revert most of 39b66a91bd
- 8e03eb92e9ad 14.0 landed
-
Fix COPY FREEZE with CLOBBER_CACHE_ALWAYS
- 39b66a91bdeb 14.0 cited