Обсуждение: Libpq COPY optimization

The following is a suggestion for optimizing the libpq COPY FROM call for
better performance. I submitted a similar suggestion awhile ago, but it
wasn't safe enough. This one is better. It shows a pretty significant
improvement while maintaining deadlock prevention.

The change is localized to PQputCopyData, not requiring
an alternate version of pqPutMsgEnd.

The change is that before returning, PQputCopyData would process inbound
NOTICEs *only if* the buffer was flushed.  (and drop the other
PQconsumeInput/parseInput calls in PQputCopyData)

At the end of the copy operation, libpq's caller will call
PQputCopyEnd followed by PQgetResult.  PQputCopyEnd flushes
the buffer, but I don't think we need to add any PQconsumeInput/
parseInput there, because any remaining inbound NOTICEs will be
handled immediately afterward when libpq's caller calls PQgetResult.

Alon.

<font face="Verdana, Helvetica, Arial"><span style="font-size:14.0px">A note on this – we see a huge performance
benefitfrom this change on Solaris, so much so that it should be mandatory for that platform.  Solaris’ error handling
isdeeper, and so the repeated redundant interrupts that this patch avoids causes 60% of the CPU to be consumed during
COPY. After this patch, this work disappears from the profile and the COPY speed is much improved.<br /><br /> There
arealso big speed improvements on Linux, on the order of 40% at high speeds.  Using Bizgres MPP, this change allowed us
toincrease from 47 MB/s loading to 70+ MB/s loading speed (250GB/hour).<br /><br /> - Luke <br /><br /><br /> On 1/8/06
6:00AM, "Alon Goldshuv" <agoldshuv@greenplum.com> wrote:<br /><br /></span></font><blockquote><font
face="Verdana,Helvetica, Arial"><span style="font-size:14.0px">The following is a suggestion for optimizing the libpq
COPYFROM call for<br /> better performance. I submitted a similar suggestion awhile ago, but it<br /> wasn't safe
enough.This one is better. It shows a pretty significant<br /> improvement while maintaining deadlock prevention.<br
/><br/> The change is localized to PQputCopyData, not requiring<br /> an alternate version of pqPutMsgEnd.<br /><br />
Thechange is that before returning, PQputCopyData would process inbound<br /> NOTICEs *only if* the buffer was flushed.
 (anddrop the other<br /> PQconsumeInput/parseInput calls in PQputCopyData)<br /><br /> At the end of the copy
operation,libpq's caller will call<br /> PQputCopyEnd followed by PQgetResult.  PQputCopyEnd flushes<br /> the buffer,
butI don't think we need to add any PQconsumeInput/<br /> parseInput there, because any remaining inbound NOTICEs will
be<br/> handled immediately afterward when libpq's caller calls PQgetResult.<br /><br /> Alon.<br /><br /><br
/></span></font></blockquote><fontface="Verdana, Helvetica, Arial"><span style="font-size:14.0px"><br /></span></font> 

"Alon Goldshuv" <agoldshuv@greenplum.com> writes:
> The following is a suggestion for optimizing the libpq COPY FROM call for
> better performance. I submitted a similar suggestion awhile ago, but it
> wasn't safe enough. This one is better.

It's not enough better, because it will still deadlock given a
sufficiently large message-to-send.  I don't think you can postpone
the clearing-input action until after all the data is sent.
        regards, tom lane

Tom,

> It's not enough better, because it will still deadlock given a
> sufficiently large message-to-send.  I don't think you can postpone
> the clearing-input action until after all the data is sent.
> 
> regards, tom lane

Please help me understand this better. It appears to me that when the
client->backend pipe fills up, pqSendSome() consumes any incoming
NOTICE/WARNING messages before waiting, which should prevent deadlock. I'll
look at the code again, maybe I missed something.

Thx,
Alon.

"Alon Goldshuv" <agoldshuv@greenplum.com> writes:
> Please help me understand this better. It appears to me that when the
> client->backend pipe fills up, pqSendSome() consumes any incoming
> NOTICE/WARNING messages before waiting, which should prevent deadlock.

Hm, I had forgotten that the low-level pqSendSome routine does that.
That makes the PQconsumeInput call in PQputCopyData redundant (or
almost; see below).  The parseInput call is still needed, because it's
there to pull NOTICE messages out of the input buffer and get rid of
them, rather than possibly having the input buffer grow to exceed
memory.  But when there's nothing for it to do, parseInput is cheap
enough that there's no real need to bypass it.

In short, if you just remove the PQconsumeInput call I think you'll find
that it does what you want.

The only case where it's helpful to have it there is if there's a
incomplete message in the input buffer, as parseInput isn't quite so
fast if it has to determine that the message is incomplete.  Without
the PQconsumeInput call, the incomplete-message state could persist
for a long time, and you'd pay the parseInput overhead each time through
PQputCopyData.  However, that's certainly not the normal situation,
so I think we could leave that case slightly pessimal.  It's certainly
true that that path in parseInput is a lot faster than a kernel call,
so it'd still be better than it is now.
        regards, tom lane

Can I get an updated patch for this?

---------------------------------------------------------------------------

Tom Lane wrote:
> "Alon Goldshuv" <agoldshuv@greenplum.com> writes:
> > Please help me understand this better. It appears to me that when the
> > client->backend pipe fills up, pqSendSome() consumes any incoming
> > NOTICE/WARNING messages before waiting, which should prevent deadlock.
> 
> Hm, I had forgotten that the low-level pqSendSome routine does that.
> That makes the PQconsumeInput call in PQputCopyData redundant (or
> almost; see below).  The parseInput call is still needed, because it's
> there to pull NOTICE messages out of the input buffer and get rid of
> them, rather than possibly having the input buffer grow to exceed
> memory.  But when there's nothing for it to do, parseInput is cheap
> enough that there's no real need to bypass it.
> 
> In short, if you just remove the PQconsumeInput call I think you'll find
> that it does what you want.
> 
> The only case where it's helpful to have it there is if there's a
> incomplete message in the input buffer, as parseInput isn't quite so
> fast if it has to determine that the message is incomplete.  Without
> the PQconsumeInput call, the incomplete-message state could persist
> for a long time, and you'd pay the parseInput overhead each time through
> PQputCopyData.  However, that's certainly not the normal situation,
> so I think we could leave that case slightly pessimal.  It's certainly
> true that that path in parseInput is a lot faster than a kernel call,
> so it'd still be better than it is now.
> 
>             regards, tom lane
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 9: In versions below 8.0, the planner will ignore your desire to
>        choose an index scan if your joining column's datatypes do not
>        match
> 

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
359-1001+  If your life is a hard drive,     |  13 Roberts Road +  Christ can be your backup.        |  Newtown Square,
Pennsylvania19073
 

I'll send it to -patches shortly

Alon.


On 1/23/06 10:20 PM, "Bruce Momjian" <pgman@candle.pha.pa.us> wrote:

> 
> Can I get an updated patch for this?
> 
> ---------------------------------------------------------------------------
> 
> Tom Lane wrote:
>> "Alon Goldshuv" <agoldshuv@greenplum.com> writes:
>>> Please help me understand this better. It appears to me that when the
>>> client->backend pipe fills up, pqSendSome() consumes any incoming
>>> NOTICE/WARNING messages before waiting, which should prevent deadlock.
>> 
>> Hm, I had forgotten that the low-level pqSendSome routine does that.
>> That makes the PQconsumeInput call in PQputCopyData redundant (or
>> almost; see below).  The parseInput call is still needed, because it's
>> there to pull NOTICE messages out of the input buffer and get rid of
>> them, rather than possibly having the input buffer grow to exceed
>> memory.  But when there's nothing for it to do, parseInput is cheap
>> enough that there's no real need to bypass it.
>> 
>> In short, if you just remove the PQconsumeInput call I think you'll find
>> that it does what you want.
>> 
>> The only case where it's helpful to have it there is if there's a
>> incomplete message in the input buffer, as parseInput isn't quite so
>> fast if it has to determine that the message is incomplete.  Without
>> the PQconsumeInput call, the incomplete-message state could persist
>> for a long time, and you'd pay the parseInput overhead each time through
>> PQputCopyData.  However, that's certainly not the normal situation,
>> so I think we could leave that case slightly pessimal.  It's certainly
>> true that that path in parseInput is a lot faster than a kernel call,
>> so it'd still be better than it is now.
>> 
>> regards, tom lane
>> 
>> ---------------------------(end of broadcast)---------------------------
>> TIP 9: In versions below 8.0, the planner will ignore your desire to
>>        choose an index scan if your joining column's datatypes do not
>>        match
>>