Re: Flushing large data immediately in pqcomm

From: Melih Mutlu <m(dot)melihmutlu(at)gmail(dot)com>
To: Jelte Fennema-Nio <postgres(at)jeltef(dot)nl>
Cc: David Rowley <dgrowleyml(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Robert Haas <robertmhaas(at)gmail(dot)com>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
Subject: Re: Flushing large data immediately in pqcomm
Date: 2024-04-04 14:28:35
Message-ID: CAGPVpCQWUz=VGAjycSw_1aRLRXj5wp8hubZHq992KBxp25veSQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Jelte Fennema-Nio <postgres(at)jeltef(dot)nl>, 4 Nis 2024 Per, 16:34 tarihinde
şunu yazdı:

> On Thu, 4 Apr 2024 at 13:08, Melih Mutlu <m(dot)melihmutlu(at)gmail(dot)com> wrote:
> > I changed internal_flush() to an inline function, results look better
> this way.
>
> It seems you also change internal_flush_buffer to be inline (but only
> in the actual function definition, not declaration at the top). I
> don't think inlining internal_flush_buffer should be necessary to
> avoid the perf regressions, i.e. internal_flush is adding extra
> indirection compared to master and is only a single line, so that one
> makes sense to inline.
>

Right. It was a mistake, forgot to remove that. Fixed it in v5.

> Other than that the code looks good to me.
>
> The new results look great.
>
> One thing that is quite interesting about these results is that
> increasing the buffer size results in even better performance (by
> quite a bit). I don't think we can easily choose a perfect number, as
> it seems to be a trade-off between memory usage and perf. But allowing
> people to configure it through a GUC like in your second patchset
> would be quite useful I think, especially because larger buffers could
> be configured for connections that would benefit most for it (e.g.
> replication connections or big COPYs).
>
> I think your add-pq_send_buffer_size-GUC.patch is essentially what we
> would need there but it would need some extra changes to actually be
> merge-able:
> 1. needs docs
> 2. rename PQ_SEND_BUFFER_SIZE (at least make it not UPPER_CASE, but
> maybe also remove the PQ_ prefix)
> 3. It's marked as PGC_USERSET, but there's no logic to grow/shrink it
> after initial allocation
>

I agree that the GUC patch requires more work to be in good shape. I
created that for testing purposes. But if we decide to make the buffer size
customizable, then I'll start polishing up that patch and address your
suggestions.

One case that could benefit from increased COPY performance is table sync
of logical replication. It might make sense letting users to configure
buffer size to speed up table sync. I'm not sure what kind of problems this
GUC would bring though.

Thanks,
--
Melih Mutlu
Microsoft

Attachment Content-Type Size
v5-0001-Flush-large-data-immediately-in-pqcomm.patch application/octet-stream 4.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2024-04-04 14:31:58 Re: Adding comments to help understand psql hidden queries
Previous Message Tom Lane 2024-04-04 14:26:42 Re: WIP Incremental JSON Parser