Pipeline mode and PQpipelineSync()

On 2021-Jun-16, Boris Kolpackov wrote:

Specifically, the documentation[1]
makes it sound like the use of PQpipelineSync() is optional (34.5.1.1
"Issuing Queries"):

Hmm. My intention here was to indicate that you should have
PQpipelineSync *somewhere*, but that the server was free to start
executing some commands even before that, if the buffered commands
happened to reach the server somehow -- but not necessarily that the
results from those commands would reach the client immediately.

I'll experiment a bit more to be sure that what I'm saying is correct.
But if it is, then I think the documentation you quote is misleading:

"After entering pipeline mode, the application dispatches requests using
PQsendQuery, PQsendQueryParams, or its prepared-query sibling
PQsendQueryPrepared. These requests are queued on the client-side until
flushed to the server; this occurs when PQpipelineSync is used to establish a
synchronization point in the pipeline, or when PQflush is called. [...]

The server executes statements, and returns results, in the order the client
sends them. The server will begin executing the commands in the pipeline
immediately, not waiting for the end of the pipeline. [...]"

... because it'll lead people to do what you've done, only to discover
that it doesn't really work.

I think I should rephrase this to say that PQpipelineSync() is needed
where the user needs the server to start executing commands; and
separately indicate that it is possible (but not promised) that the
server would start executing commands ahead of time because $reasons.

Do I have it right that other than this documentation problem, you've
been able to use pipeline mode successfully?

So to me it looks like, contrary to the documentation, the server does
not start executing the statements immediately, instead waiting for the
synchronization point. Or am I missing something here?

I don't think you are.

The above tests were performed using libpq from 14beta1 running against
PostgreSQL server version 9.5. If you would like to take a look at the
actual code, you can find it here[2] (the PIPELINE_SYNC macro controls
whether PQpipelineSync() is used).

Thanks.

On a related note, I've been using libpq_pipeline.c[3] as a reference
and I believe it has a busy loop calling PQflush() repeatedly on line
721 since once everything has been sent and we are waiting for the
result, select() will keep returning with an indication that the socket
is writable

Oops, thanks, will look at fixing this too.

(you can find one way to fix this in [2]).
[2] https://git.codesynthesis.com/cgit/odb/libodb-pgsql/tree/odb/pgsql/statement.cxx?h=bulk#n771

Neat, can do likewise I suppose.

--
�lvaro Herrera 39�49'30"S 73�17'W

Alvaro Herrera <alvaro.herrera@2ndquadrant.com> writes:

I think I should rephrase this to say that PQpipelineSync() is needed
where the user needs the server to start executing commands; and
separately indicate that it is possible (but not promised) that the
server would start executing commands ahead of time because $reasons.

I think always requiring PQpipelineSync() is fine since it also serves
as an error recovery boundary. But the fact that the server waits until
the sync message to start executing the pipeline is surprising. To me
this seems to go contrary to the idea of a "pipeline".

In fact, I see the following ways the server could behave:

1. The server starts executing queries and sending their results before
receiving the sync message.

2. The server starts executing queries before receiving the sync message
but buffers the results until it receives the sync message.

3. The server buffers the queries and only starts executing them and
sending the results after receiving the sync message.

My observations suggest that the server behaves as (3) but it could
also be (2).

While it can be tempting to say that this is an implementation detail,
this affects the way one writes a client. For example, I currently have
the following comment in my code:

// Send queries until we get blocked. This feels like a better
// overall strategy to keep the server busy compared to sending one
// query at a time and then re-checking if there is anything to read
// because the results of INSERT/UPDATE/DELETE are presumably small
// and quite a few of them can get buffered before the server gets
// blocked.

This would be a good strategy for behavior (1) but not (3) (where it
would make more sense to queue the queries on the client side). So I
think it would be useful to clarify the server behavior and specify
it in the documentation.

Do I have it right that other than this documentation problem, you've
been able to use pipeline mode successfully?

So far I've only tried it in a simple prototype (single INSERT statement).
But I am busy plugging it into ODB's bulk operation support (that we
already have for Oracle and MSSQL) and once that's done I should be
able to exercise things in more meaningful ways.

On 2021-Jun-21, Boris Kolpackov wrote:

Alvaro Herrera <alvaro.herrera@2ndquadrant.com> writes:

I think I should rephrase this to say that PQpipelineSync() is needed
where the user needs the server to start executing commands; and
separately indicate that it is possible (but not promised) that the
server would start executing commands ahead of time because $reasons.

I think always requiring PQpipelineSync() is fine since it also serves
as an error recovery boundary. But the fact that the server waits until
the sync message to start executing the pipeline is surprising. To me
this seems to go contrary to the idea of a "pipeline".

But does that actually happen? There's a very easy test we can do by
sending queries that sleep. If my libpq program sends a "SELECT
pg_sleep(2)", then PQflush(), then sleep in the client program two more
seconds without sending the sync; and *then* send the sync, I find that
the program takes 2 seconds, not four. This shows that both client and
server slept in parallel, even though I didn't send the Sync until after
the client was done sleeping.

In order to see this, I patched libpq_pipeline.c with the attached, and
ran it under time:

time ./libpq_pipeline simple_pipeline -t simple.trace
simple pipeline... sent and flushed the sleep. Sleeping 2s here:
client sleep done
ok

real 0m2,008s
user 0m0,000s
sys 0m0,003s

So I see things happening as you describe in (1):

In fact, I see the following ways the server could behave:

1. The server starts executing queries and sending their results before
receiving the sync message.

I am completely at a loss on how to explain a server that behaves in any
other way, given how the protocol is designed. There is no buffering on
the server side.

While it can be tempting to say that this is an implementation detail,
this affects the way one writes a client. For example, I currently have
the following comment in my code:

// Send queries until we get blocked. This feels like a better
// overall strategy to keep the server busy compared to sending one
// query at a time and then re-checking if there is anything to read
// because the results of INSERT/UPDATE/DELETE are presumably small
// and quite a few of them can get buffered before the server gets
// blocked.

This would be a good strategy for behavior (1) but not (3) (where it
would make more sense to queue the queries on the client side).

Agreed, that's the kind of strategy I would have thought was the most
reasonable, given my understanding of how the protocol works.

I wonder if your program is being affected by something else. Maybe the
socket is nonblocking (though I don't quite understand how that would
affect the client behavior in just this way), or your program is
buffering elsewhere. I don't do C++ much so I can't help you with that.

So I think it would be useful to clarify the server behavior and
specify it in the documentation.

I'll see about improving the docs on these points.

Do I have it right that other than this documentation problem, you've
been able to use pipeline mode successfully?

So far I've only tried it in a simple prototype (single INSERT statement).
But I am busy plugging it into ODB's bulk operation support (that we
already have for Oracle and MSSQL) and once that's done I should be
able to exercise things in more meaningful ways.

Fair enough.

--
�lvaro Herrera 39�49'30"S 73�17'W

On 2021-Jun-22, Alvaro Herrera wrote:

So I think it would be useful to clarify the server behavior and
specify it in the documentation.

I'll see about improving the docs on these points.

So I started to modify the second paragraph to indicate that the client
would send data on PQflush/buffer full/PQpipelineSync, only to realize
that the first paragraph already explains this. So I'm not sure if any
changes are needed.

Maybe your complaint is only based on disagreement about what does libpq
do regarding queueing commands; and as far as I can tell in quick
experimentation with libpq, it works as the docs state already.

--
�lvaro Herrera Valdivia, Chile

Alvaro Herrera <alvaro.herrera@2ndquadrant.com> writes:

I think always requiring PQpipelineSync() is fine since it also serves
as an error recovery boundary. But the fact that the server waits until
the sync message to start executing the pipeline is surprising. To me
this seems to go contrary to the idea of a "pipeline".

But does that actually happen? There's a very easy test we can do by
sending queries that sleep. If my libpq program sends a "SELECT
pg_sleep(2)", then PQflush(), then sleep in the client program two more
seconds without sending the sync; and *then* send the sync, I find that
the program takes 2 seconds, not four. This shows that both client and
server slept in parallel, even though I didn't send the Sync until after
the client was done sleeping.

Thanks for looking into it. My experiments were with INSERT and I now
was able to try things with larger pipelines. I can now see the server
starts sending results after ~400 statements. So I think you are right,
the server does start executing the pipeline before receiving the sync
message, though there is still something strange going on (but probably
on the client side):

I have a pipeline of say 500 INSERTs. If I "execute" this pipeline by first
sending all the statements and then reading the results, then everything
works as expected. This is the call sequence I am talking about:

PQsendQueryPrepared() # INSERT #1
PQflush()
PQsendQueryPrepared() # INSERT #2
PQflush()
...
PQsendQueryPrepared() # INSERT #500
PQpipelineSync()
PQflush()
PQconsumeInput()
PQgetResult() # INSERT #1
PQgetResult() # NULL
PQgetResult() # INSERT #2
PQgetResult() # NULL
...
PQgetResult() # INSERT #500
PQgetResult() # NULL
PQgetResult() # PGRES_PIPELINE_SYNC

If, however, I execute it by checking for results before sending the
next INSERT, I get the following call sequence:

PQsendQueryPrepared() # INSERT #1
PQflush()
PQsendQueryPrepared() # INSERT #2
PQflush()
...
PQsendQueryPrepared() # INSERT #~400
PQflush()
PQconsumeInput() # At this point select() indicates we can read.
PQgetResult() # NULL (???)
PQgetResult() # INSERT #1
PQgetResult() # NULL
PQgetResult() # INSERT #2
PQgetResult() # NULL
...

What's strange here is that the first PQgetResult() call (marked with ???)
returns NULL instead of result for INSERT #1 as in the first call sequence.
Interestingly, if I skip it, the rest seems to progress as expected.

Any idea what might be going on here? My hunch is that there is an issue
with libpq's state machine. In particular, in the second case, PQgetResult()
is called before the sync message is sent. Did you have a chance to test
such a scenario (i.e., a large pipeline where the first result is processed
before the PQpipelineSync() call)? Of course, this could very well be a bug
on my side or me misunderstanding something.

Alvaro Herrera <alvaro.herrera@2ndquadrant.com> writes:

On 2021-Jun-22, Alvaro Herrera wrote:

So I think it would be useful to clarify the server behavior and
specify it in the documentation.

I'll see about improving the docs on these points.

So I started to modify the second paragraph to indicate that the client
would send data on PQflush/buffer full/PQpipelineSync, only to realize
that the first paragraph already explains this. So I'm not sure if any
changes are needed.

Maybe your complaint is only based on disagreement about what does libpq
do regarding queueing commands; and as far as I can tell in quick
experimentation with libpq, it works as the docs state already.

I think one change that is definitely needed is to make it clear that
the PQpipelineSync() call is not optional.

I would also add a note saying that while the server starts processing
the pipeline immediately, it may buffer the results and the only way
to flush them out is to call PQpipelineSync().

On 2021-Jun-23, Boris Kolpackov wrote:

I think one change that is definitely needed is to make it clear that
the PQpipelineSync() call is not optional.

I would also add a note saying that while the server starts processing
the pipeline immediately, it may buffer the results and the only way
to flush them out is to call PQpipelineSync().

Curious -- I just noticed that the server understands a message 'H' that
requests a flush of the server buffer. However, libpq has no way to
generate that message as far as I can see. I think you could use that
to request results from the pipeline, without the sync point.

I wonder if it's worth adding an entry point to libpq to allow access to
this. PQrequestFlush() or something like that ... Prior to pipeline
mode this has no use (since everything ends with ReadyForQuery which
involves a flush) but it does seem to have use in pipeline mode.

--
�lvaro Herrera 39�49'30"S 73�17'W

On 2021-Jun-23, Boris Kolpackov wrote:

I think one change that is definitely needed is to make it clear that
the PQpipelineSync() call is not optional.

I would also add a note saying that while the server starts processing
the pipeline immediately, it may buffer the results and the only way
to flush them out is to call PQpipelineSync().

Aren't those two things one and the same? I propose the attached.

--
�lvaro Herrera Valdivia, Chile
"El hombre nunca sabe de lo que es capaz hasta que lo intenta" (C. Dickens)

Boris Kolpackov <boris@codesynthesis.com> writes:

What's strange here is that the first PQgetResult() call (marked with ???)
returns NULL instead of result for INSERT #1 as in the first call sequence.

I've hit another similar case except now an unexpected NULL result is
returned in the middle of PGRES_PIPELINE_ABORTED result sequence. The
call sequence is as follows:

PQsendQueryPrepared() # INSERT #1
PQflush()
PQsendQueryPrepared() # INSERT #2
PQflush()
...
PQsendQueryPrepared() # INSERT #251 -- insert duplicate PK
PQflush()
...
PQsendQueryPrepared() # INSERT #343
PQflush()
PQconsumeInput() # At this point select() indicates we can read.
PQgetResult() # NULL -- unexpected but skipped (see prev. email)
PQgetResult() # INSERT #1
PQgetResult() # NULL
PQgetResult() # INSERT #2
PQgetResult() # NULL
...
PQgetResult() # INSERT #251 error result, SQLSTATE 23505
PQgetResult() # NULL
PQgetResult() # INSERT #252 PGRES_PIPELINE_ABORTED
PQgetResult() # NULL
PQgetResult() # INSERT #253 PGRES_PIPELINE_ABORTED
PQgetResult() # NULL
...
PQgetResult() # INSERT #343 NULL (???)

Notice that result #343 corresponds to the last PQsendQueryPrepared()
call made before the socket became readable (it's not always 343 but
around there).

For completeness, the statement in question is:

INSERT INTO pgsql_bulk_object (id, idata, sdata) VALUES ($1, $2, $3)

The table:

CREATE TABLE pgsql_bulk_object (
id BIGINT NOT NULL PRIMARY KEY,
idata BIGINT NOT NULL,
sdata TEXT NOT NULL);

And the data inserted is in the form:

1, 1, "1"
2, 2, "2"
...

Alvaro Herrera <alvaro.herrera@2ndquadrant.com> writes:

Curious -- I just noticed that the server understands a message 'H' that
requests a flush of the server buffer. However, libpq has no way to
generate that message as far as I can see. I think you could use that
to request results from the pipeline, without the sync point.

I wonder if it's worth adding an entry point to libpq to allow access to
this.

Yes, I think this can be useful. For example, an application may wish
to receive the result as soon as possible in case it's used as input
to some further computation.

PQrequestFlush() or something like that ...

I think I would prefer PQflushResult() or something along these
lines ("request" is easy to misinterpret as "client request").

Alvaro Herrera <alvaro.herrera@2ndquadrant.com> writes:

Subject: [PATCH] Clarify that pipeline sync is mandatory

---
doc/src/sgml/libpq.sgml | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 441cc0da3a..0217f8d8c7 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -5103,10 +5103,12 @@ int PQflush(PGconn *conn);
The server executes statements, and returns results, in the order the
client sends them.  The server will begin executing the commands in the
pipeline immediately, not waiting for the end of the pipeline.
+     Do note that results are buffered on the server side; a synchronization
+     point, establshied with <function>PQpipelineSync</function>, is necessary
+     in order for all results to be flushed to the client.

s/establshied/established/

Otherwise, LGTM, thanks!

On 2021-Jun-23, Boris Kolpackov wrote:

If, however, I execute it by checking for results before sending the
next INSERT, I get the following call sequence:

PQsendQueryPrepared() # INSERT #1
PQflush()
PQsendQueryPrepared() # INSERT #2
PQflush()
...
PQsendQueryPrepared() # INSERT #~400
PQflush()
PQconsumeInput() # At this point select() indicates we can read.
PQgetResult() # NULL (???)
PQgetResult() # INSERT #1
PQgetResult() # NULL
PQgetResult() # INSERT #2
PQgetResult() # NULL
...

What's strange here is that the first PQgetResult() call (marked with ???)
returns NULL instead of result for INSERT #1 as in the first call sequence.
Interestingly, if I skip it, the rest seems to progress as expected.

Yeah, I agree that there's a problem in the libpq state machine. I'm
looking into it now.

--
�lvaro Herrera 39�49'30"S 73�17'W

On 2021-Jun-23, Boris Kolpackov wrote:

If, however, I execute it by checking for results before sending the
next INSERT, I get the following call sequence:

PQsendQueryPrepared() # INSERT #1
PQflush()
PQsendQueryPrepared() # INSERT #2
PQflush()
...
PQsendQueryPrepared() # INSERT #~400
PQflush()
PQconsumeInput() # At this point select() indicates we can read.
PQgetResult() # NULL (???)
PQgetResult() # INSERT #1
PQgetResult() # NULL
PQgetResult() # INSERT #2
PQgetResult() # NULL

IIUC the problem is that PQgetResult is indeed not prepared to deal with
a result the first time until after the queue has been "prepared", and
this happens on calling PQpipelineSync. But I think the formulation in
the attached patch works too, and the resulting code is less surprising.

I wrote a test case that works as you describe, and indeed with the
original code it gets a NULL initially; that disappears with the
attached patch. Can you give it a try?

Thanks

--
�lvaro Herrera Valdivia, Chile
"Escucha y olvidar�s; ve y recordar�s; haz y entender�s" (Confucio)

Alvaro Herrera <alvaro.herrera@2ndquadrant.com> writes:

IIUC the problem is that PQgetResult is indeed not prepared to deal with
a result the first time until after the queue has been "prepared", and
this happens on calling PQpipelineSync. But I think the formulation in
the attached patch works too, and the resulting code is less surprising.

I wrote a test case that works as you describe, and indeed with the
original code it gets a NULL initially; that disappears with the
attached patch. Can you give it a try?

Yes, I can confirm this appears to have addressed the first issue,
thanks! The second issue [1]/messages/by-id/boris.20210624103805@codesynthesis.com, however, is still there even with
this patch.

[1]: /messages/by-id/boris.20210624103805@codesynthesis.com

On 2021-Jun-24, Boris Kolpackov wrote:

Boris Kolpackov <boris@codesynthesis.com> writes:

What's strange here is that the first PQgetResult() call (marked with ???)
returns NULL instead of result for INSERT #1 as in the first call sequence.

I've hit another similar case except now an unexpected NULL result is
returned in the middle of PGRES_PIPELINE_ABORTED result sequence. The
call sequence is as follows:

I haven't been able to get this to break for me yet, and I probably
won't today. In the meantime, here's patches for the first one. The
test added by 0003 fails, and then 0004 fixes it.

--
�lvaro Herrera 39�49'30"S 73�17'W

On 2021-Jun-25, Alvaro Herrera wrote:

From 071757645ee0f9f15f57e43447d7c234deb062c0 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Fri, 25 Jun 2021 16:02:00 -0400
Subject: [PATCH v2 2/4] Add PQrequestFlush()

I forgot to mention:

+/*
+ * Send request for server to flush its buffer
+ */
+int
+PQrequestFlush(PGconn *conn)
+{
+	if (!conn)
+		return 0;
+
+	/* Don't try to send if we know there's no live connection. */
+	if (conn->status != CONNECTION_OK)
+	{
+		appendPQExpBufferStr(&conn->errorMessage,
+							 libpq_gettext("no connection to the server\n"));
+		return 0;
+	}
+
+	/* Can't send while already busy, either, unless enqueuing for later */
+	if (conn->asyncStatus != PGASYNC_IDLE &&
+		conn->pipelineStatus == PQ_PIPELINE_OFF)
+	{
+		appendPQExpBufferStr(&conn->errorMessage,
+							 libpq_gettext("another command is already in progress\n"));
+		return false;
+	}
+
+	if (pqPutMsgStart('H', conn) < 0 ||
+		pqPutMsgEnd(conn) < 0)
+	{
+		return 0;
+	}
+	/* XXX useless without a flush ...? */
+	pqFlush(conn);
+
+	return 1;
+}

I'm not sure if it's a good idea for PQrequestFlush to itself flush
libpq's buffer. We can just document that PQflush is required ...
opinions?

(I didn't try PQrequestFlush in any scenarios other than the test case I
added.)

--
�lvaro Herrera Valdivia, Chile
Voy a acabar con todos los humanos / con los humanos yo acabar�
voy a acabar con todos (bis) / con todos los humanos acabar� �acabar�! (Bender)

It hadn't occurred to me that I should ask the release management team
about adding a new function to libpq this late in the cycle.

Please do note that the message type used in the new routine is currenly
unused and uncovered -- see line 4660 here:

https://coverage.postgresql.org/src/backend/tcop/postgres.c.gcov.html

--
�lvaro Herrera Valdivia, Chile
"I'm impressed how quickly you are fixing this obscure issue. I came from
MS SQL and it would be hard for me to put into words how much of a better job
you all are doing on [PostgreSQL]."
Steve Midgley, http://archives.postgresql.org/pgsql-sql/2008-08/msg00000.php

On Sat, Jun 26, 2021 at 05:40:15PM -0400, Alvaro Herrera wrote:

It hadn't occurred to me that I should ask the release management team
about adding a new function to libpq this late in the cycle.

I have not followed the thread in details, but if you think that this
improves the feature in the long term even for 14, I have no
personally no objections to the addition of a new function, or even a
change of behavior in one of the existing functions. The beta cycle
is here for such adjustments.
--
Michael

Alvaro Herrera <alvaro.herrera@2ndquadrant.com> writes:

I forgot to mention:

+	/* XXX useless without a flush ...? */
+	pqFlush(conn);
+
+	return 1;
+}

I'm not sure if it's a good idea for PQrequestFlush to itself flush
libpq's buffer. We can just document that PQflush is required ...
opinions?

Yes, I think not calling PQflush() gives more flexibility. For example,
an application may "insert" them periodically after a certain number of
queries but call PQflush() at different intervals.