On 4/27/21 5:44 PM, Masahiko Sawada wrote:
On Wed, Apr 28, 2021 at 12:26 AM Masahiko Sawada <sawada.m...@gmail.com> wrote:
On Tue, Apr 27, 2021 at 10:43 PM Tomas Vondra
<tomas.von...@enterprisedb.com> wrote:
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?
With that patch, we pin the vmbuffer only when inserting a frozen
tuple into an empty page. That is, when inserting a frozen tuple into
an empty page, we pin the vmbuffer and heap_insert() will mark the
page all-visible and set all-frozen bit on vm. And from the next
insertion (into the same page) until the page gets full, since the
page is already all-visible, we skip pinning the vmbuffer. IOW, if the
target page is not empty but all-visible, we skip pinning the
vmbuffer. We pin the vmbuffer only once per heap page used during
insertion.
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.
Understood. This approach keeps using the same vmbuffer until we need
another vm page corresponding to the target heap page, which seems
better.
But how is ExecInsert() related to REFRESH MATERIALIZED VIEW?
TBH I haven't looked into the details, but Andres talked about
nodeModifyTable and table_tuple_insert, and ExecInsert is the only place
calling it. But maybe I'm just confused and Andres meant something else?
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
