Re: [HACKERS] PATCH: Batch/pipelining support for libpq

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Justin Pryzby <pryzby(at)telsasoft(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Craig Ringer <craig(dot)ringer(at)enterprisedb(dot)com>, Matthieu Garrigues <matthieu(dot)garrigues(at)gmail(dot)com>, Zhihong Yu <zyu(at)yugabyte(dot)com>, Ibrar Ahmed <ibrar(dot)ahmad(at)gmail(dot)com>
Subject: Re: [HACKERS] PATCH: Batch/pipelining support for libpq
Date: 2021-03-13 04:28:15
Message-ID: 20210313042815.GA18243@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2021-Mar-11, Tom Lane wrote:

> I think the changes in pqParseInput3() are broken. You should have
> kept the else-structure as-is and inserted the check for "not really
> idle" inside the else-clause that reports an error. As it stands,
> after successfully processing an asynchronously-received error or
> ParameterStatus message, the added code will cause us to return without
> advancing inStart, creating an infinite loop of reprocessing that message.
>
> It's possible that we should redefine the way things happen so that if
> we're waiting for another pipeline event, we should hold off processing
> of async error & ParameterStatus; but in that case you should have added
> the pre-emptive return ahead of that if/else structure, where the existing
> "If not IDLE state, just wait ..." test is.

I think I agree that holding off 'E' and 'S' messages when in between
processing results for different queries in a pipeline, so keeping the
original if/else structure is correct. An error would be correctly
dealt with in the BUSY state immediately afterwards; and the fact that
we pass 'inError' false at that point causes the wrong reaction (namely
that the pipeline is not put in aborted state).

I made a number of other changes: documentation adjustments per comments
from David Johnston, some function renaming as previously noted, and
added test code for PQsendDescribePrepared, PQsendDescribePortal. Also
rebased on latest changes. I also absorbed one change that I already
had when I submitted v35, but hadn't done "git add" on (which caused a
compile failure for CF bot).

--
Álvaro Herrera Valdivia, Chile

Attachment Content-Type Size
v36-libpq-pipeline.patch text/x-diff 114.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2021-03-13 04:38:47 Re: Parallel INSERT (INTO ... SELECT ...)
Previous Message Thomas Munro 2021-03-13 04:23:22 Re: pgbench: option delaying queries till connections establishment?