On 4/27/21 7:34 AM, Masahiko Sawada wrote:
On Tue, Apr 27, 2021 at 8:07 AM Andres Freund <and...@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


Reply via email to