Re: Add Pipelining support in psql

From: Anthonin Bonnefoy <anthonin(dot)bonnefoy(at)datadoghq(dot)com>
To: Jelte Fennema-Nio <postgres(at)jeltef(dot)nl>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Add Pipelining support in psql
Date: 2024-11-28 09:22:49
Message-ID: CAO6_XqrseE1UGxMEHu3CARdn5HW_C4rqR3Xy=Vg2sfHjPRa1og@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Nov 27, 2024 at 11:46 AM Kirill Reshke <reshkekirill(at)gmail(dot)com> wrote:
> I'm very doubtful about the \syncpipeline . Maybe we should instead
> support \sync meta-command in psql? This will be a useful contribution
> itself.

\syncpipeline is useful to cover regression tests involving implicit
transaction blocks within a pipeline where a sync acts as an implicit
COMMIT. What would be the use of sending a \sync outside of a
pipeline? All extended queries meta-commands sent by psql already send
a sync if you're not in a pipeline, so the only use of a \sync
meta-command would be to send a single sync (which could be achieved
with \startpipeline followed by a \endpipeline).

On Wed, Nov 27, 2024 at 1:54 PM Jelte Fennema-Nio <postgres(at)jeltef(dot)nl> wrote:
> This behaviour is expected, it also happens if you send "SELECT 1"
> instead of "begin" in the parse message:
> db1=# \startpipeline
> db1=# SELECT 1 \parse p1
> db1=*#
>
> The reason this happens is that the first command in a pipeline sent
> to the server opens an implicit transaction. See the "implicit COMMIT"
> wording here[1], or look at this code in exec_parse_message:

I don't think that's the case here. Nothing was sent to the server so
there's no active transaction yet. From prompt.c, psql will show a '*'
when PQtransactionStatus is either PQTRANS_ACTIVE or PQTRANS_INTRANS.

Looking at the state of the PGConn, after a \startpipeline, we have:
db->xactStatus: (PGTransactionStatusType) PQTRANS_IDLE
db->asyncStatus: (PGAsyncStatusType) PGASYNC_IDLE
db->pipelineStatus: (PGpipelineStatus) PQ_PIPELINE_ON

After the first command is sent to the pipeline:
db->asyncStatus: (PGAsyncStatusType) PGASYNC_BUSY
db->xactStatus: (PGTransactionStatusType) PQTRANS_IDLE
db->pipelineStatus: (PGpipelineStatus) PQ_PIPELINE_ON

The xactStatus is idle, however, since asyncStatus reports busy, we
fall in the "if (conn->asyncStatus != PGASYNC_IDLE) return
PQTRANS_ACTIVE;" and psql display '*' in the prompt. This might be a
bit misleading since there's no ongoing transaction block on the
server and maybe it's worth having a new state? I've updated the patch
to display a tentative '|' when there's an ongoing pipeline. On the
other hand, a pipeline will start an implicit transaction block even
without BEGIN so leaving the '*' may be a good way to reflect that?

On Wed, Nov 27, 2024 at 1:45 PM Jelte Fennema-Nio <postgres(at)jeltef(dot)nl> wrote:
> I played around quickly with this patch and it works quite well. A few
> things that would be nice improvements I think. Feel free to change
> the command names:
> 1. Add a \flush command that calls PQflush
> 2. Add a \flushrequest command that calls PQsendFlushRequest
> 3. Add a \getresult command so you can get a result from a query
> without having to close the pipeline
>
> To be clear, not having those additional commands isn't a blocker for
> this patch imho, but I'd definitely miss them if they weren't there
> when I would be using it.

I will need a bit of time to check those and how they can be included
in the regression tests.

Attachment Content-Type Size
v02-0001-Add-pipelining-support-in-psql.patch application/octet-stream 32.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kirill Reshke 2024-11-28 09:31:27 Pass ParseState as down to utility functions.
Previous Message vignesh C 2024-11-28 09:13:50 Re: Introduce XID age and inactive timeout based replication slot invalidation