PATCH: Batch/pipelining support for libpq

Started by Craig Ringeralmost 10 years ago129 messageshackers
Jump to latest
#1Craig Ringer
craig@2ndquadrant.com

Hi all

Following on from the foreign table batch inserts thread[1]/messages/by-id/CAMsr+YFgDUiJ37DEfPRk8WDBuZ58psdAYJd8iNFSaGxtw=wU3g@mail.gmail.com, here's a patch
to add support for pipelining queries into asynchronous batches in libpq.

Attached, and also available at
https://github.com/2ndQuadrant/postgres/tree/dev/libpq-async-batch (subject
to rebasing and force pushes).

It's cleaned up over the draft I posted on that thread and has error
recovery implemented. I've written and included the SGML docs for it. The
test program is now pretty comprehensive, more so than for anything else in
libpq anyway. I'll submit it to the next CF as a 9.7/10.0 candidate.

I'm measuring 300x (not %) performance improvements doing batches on
servers over the Internet, so this seems pretty worthwhile. It turned out
to be way less invasive than I expected too.

(I intentionally didn't add any way for clients to annotate each work-item
in a batch with their own private data. I think that'd be really useful and
would make implementing clients easier, but should be a separate patch).

This should be very useful for optimising FDWs, Postgres-XC, etc.

[1]: /messages/by-id/CAMsr+YFgDUiJ37DEfPRk8WDBuZ58psdAYJd8iNFSaGxtw=wU3g@mail.gmail.com
/messages/by-id/CAMsr+YFgDUiJ37DEfPRk8WDBuZ58psdAYJd8iNFSaGxtw=wU3g@mail.gmail.com

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachments:

0001-Pipelining-batch-support-for-libpq.patchtext/x-patch; charset=US-ASCII; name=0001-Pipelining-batch-support-for-libpq.patchDownload+2360-44
#2Andres Freund
andres@anarazel.de
In reply to: Craig Ringer (#1)
Re: PATCH: Batch/pipelining support for libpq

Hi,

On 2016-05-23 17:19:09 +0800, Craig Ringer wrote:

Hi all
Following on from the foreign table batch inserts thread[1], here's a patch
to add support for pipelining queries into asynchronous batches in libpq.

Yay!

I'm measuring 300x (not %) performance improvements doing batches on
servers over the Internet, so this seems pretty worthwhile. It turned out
to be way less invasive than I expected too.

yay^2.

(I intentionally didn't add any way for clients to annotate each work-item
in a batch with their own private data. I think that'd be really useful and
would make implementing clients easier, but should be a separate patch).

This should be very useful for optimising FDWs, Postgres-XC, etc.

And optimizing normal clients.

Not easy, but I'd be very curious how much psql's performance improves
when replaying a .sql style dump, and replaying from a !tty fd, after
hacking it up to use the batch API.

- Andres

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Michael Paquier
michael@paquier.xyz
In reply to: Andres Freund (#2)
Re: PATCH: Batch/pipelining support for libpq

On Mon, May 23, 2016 at 8:50 AM, Andres Freund <andres@anarazel.de> wrote:

On 2016-05-23 17:19:09 +0800, Craig Ringer wrote:

Following on from the foreign table batch inserts thread[1], here's a patch
to add support for pipelining queries into asynchronous batches in libpq.

Yay!

I'm measuring 300x (not %) performance improvements doing batches on
servers over the Internet, so this seems pretty worthwhile. It turned out
to be way less invasive than I expected too.

yay^2.

I'll follow this mood. Yeha.

(I intentionally didn't add any way for clients to annotate each work-item
in a batch with their own private data. I think that'd be really useful and
would make implementing clients easier, but should be a separate patch).

This should be very useful for optimising FDWs, Postgres-XC, etc.

And optimizing normal clients.

Not easy, but I'd be very curious how much psql's performance improves
when replaying a .sql style dump, and replaying from a !tty fd, after
hacking it up to use the batch API.

Did you consider the use of simple_list.c instead of introducing a new
mimic as PGcommandQueueEntry? It would be cool avoiding adding new
list emulations on frontends.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Craig Ringer
craig@2ndquadrant.com
In reply to: Michael Paquier (#3)
Re: PATCH: Batch/pipelining support for libpq

On 24 May 2016 at 00:00, Michael Paquier <michael.paquier@gmail.com> wrote:

On Mon, May 23, 2016 at 8:50 AM, Andres Freund <andres@anarazel.de>
wrote:

This should be very useful for optimising FDWs, Postgres-XC, etc.

And optimizing normal clients.

Not easy, but I'd be very curious how much psql's performance improves
when replaying a .sql style dump, and replaying from a !tty fd, after
hacking it up to use the batch API.

I didn't, but agree it'd be interesting. So would pg_restore, for that
matter, though its use of COPY for the bulk of its work means it wouldn't
make tons of difference.

I think it'd be safe to enable it automatically in psql's
--single-transaction mode. It's also safe to send anything after an
explicit BEGIN and until the next COMMIT as a batch from libpq, and since
it parses the SQL enough to detect statement boundaries already that
shouldn't be too hard to handle.

However: psql is synchronous, using the PQexec family of blocking calls.
It's all fairly well contained in SendQuery and PSQLexec, but making it use
batching still require restructuring those to use the asynchronous
nonblocking API and append the query to a pending-list, plus the addition
of a select() loop to handle results and dispatch more work. MainLoop()
isn't structured around a select or poll, it loops over gets. So while it'd
be interesting to see what difference batching made the changes to make
psql use it would be a bit more invasive. Far from a rewrite, but to avoid
lots of code duplication it'd have to change everything to use nonblocking
mode and a select loop, which is a big change for such a core tool.

This is a bit of a side-project and I've got to get back to "real work" so
I don't expect to do a proper patch for psql any time soon. I'd rather not
try to build too much on this until it's seen some review and I know the
API won't need a drastic rewrite anyway. I'll see if I can do a hacked-up
version one evening to see what it does for performance though.

Did you consider the use of simple_list.c instead of introducing a new

mimic as PGcommandQueueEntry? It would be cool avoiding adding new
list emulations on frontends.

Nope. I didn't realise it was there; I've done very little on the C client
and library side so far. So thanks, I'll update it accordingly.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

#5Craig Ringer
craig@2ndquadrant.com
In reply to: Michael Paquier (#3)
Re: PATCH: Batch/pipelining support for libpq

On 24 May 2016 at 00:00, Michael Paquier <michael.paquier@gmail.com> wrote:

On Mon, May 23, 2016 at 8:50 AM, Andres Freund <andres@anarazel.de> wrote:

yay^2.

I'll follow this mood. Yeha.

BTW, I've publushed the HTML-ified SGML docs to
http://2ndquadrant.github.io/postgres/libpq-batch-mode.html as a preview.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

#6Craig Ringer
craig@2ndquadrant.com
In reply to: Michael Paquier (#3)
Re: PATCH: Batch/pipelining support for libpq

On 24 May 2016 at 00:00, Michael Paquier <michael.paquier@gmail.com> wrote:

Did you consider the use of simple_list.c instead of introducing a new
mimic as PGcommandQueueEntry? It would be cool avoiding adding new
list emulations on frontends.

I'd have to extend simple_list to add a generic object version, like

struct my_list_elem
{
PG_SIMPLE_LIST_ATTRS;
mytype mycol;
myothertype myothercol;
}

Objections?

I could add a void* version that's a simple clone of the string version,
but having to malloc both a list cell and its contents separately is
annoying.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Craig Ringer (#6)
Re: PATCH: Batch/pipelining support for libpq

Craig Ringer <craig@2ndquadrant.com> writes:

On 24 May 2016 at 00:00, Michael Paquier <michael.paquier@gmail.com> wrote:

Did you consider the use of simple_list.c instead of introducing a new
mimic as PGcommandQueueEntry? It would be cool avoiding adding new
list emulations on frontends.

I'd have to extend simple_list to add a generic object version, like

struct my_list_elem
{
PG_SIMPLE_LIST_ATTRS;
mytype mycol;
myothertype myothercol;
}

Objections?

That doesn't look exactly "generic".

I could add a void* version that's a simple clone of the string version,
but having to malloc both a list cell and its contents separately is
annoying.

I'd be okay with a void* version, but I'm not sure about this.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#8Jim Nasby
Jim.Nasby@BlueTreble.com
In reply to: Craig Ringer (#1)
Re: PATCH: Batch/pipelining support for libpq

On 5/23/16 4:19 AM, Craig Ringer wrote:

+ Batching less useful when information from one operation is required by the

SB "Batching is less useful".
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532) mobile: 512-569-9461

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#9Tsunakawa, Takayuki
tsunakawa.takay@jp.fujitsu.com
In reply to: Craig Ringer (#5)
Re: PATCH: Batch/pipelining support for libpq

From: pgsql-hackers-owner@postgresql.org [mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Craig Ringer
I'll follow this mood. Yeha.

BTW, I've publushed the HTML-ified SGML docs to http://2ndquadrant.github.io/postgres/libpq-batch-mode.html as a preview.

Sorry for my late reply. Fantastic performance improvement, nice documentation, and amazing rapid development! I think I’ll join the review & testing in 2016/9 CF.

Regards
Takayuki Tsunakawa

#10Dmitriy Igrishin
dmitigr@gmail.com
In reply to: Craig Ringer (#5)
Re: PATCH: Batch/pipelining support for libpq

2016-05-24 5:01 GMT+03:00 Craig Ringer <craig@2ndquadrant.com>:

On 24 May 2016 at 00:00, Michael Paquier <michael.paquier@gmail.com> wrote:

On Mon, May 23, 2016 at 8:50 AM, Andres Freund <andres@anarazel.de> wrote:

yay^2.

I'll follow this mood. Yeha.

BTW, I've publushed the HTML-ified SGML docs to
http://2ndquadrant.github.io/postgres/libpq-batch-mode.html as a preview.

Typo detected: "Returns 1 if the batch curently being received" -- "curently".

--
// Dmitry.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#11Michael Paquier
michael@paquier.xyz
In reply to: Dmitriy Igrishin (#10)
Re: PATCH: Batch/pipelining support for libpq

On Fri, Jun 3, 2016 at 8:51 PM, Dmitry Igrishin <dmitigr@gmail.com> wrote:

BTW, I've publushed the HTML-ified SGML docs to
http://2ndquadrant.github.io/postgres/libpq-batch-mode.html as a preview.

Typo detected: "Returns 1 if the batch curently being received" -- "curently".

I am looking a bit more seriously at this patch and assigned myself as
a reviewer.

testlibpqbatch.c:1239:73: warning: format specifies type 'long' but
the argument has type '__darwin_suseconds_t' (aka 'int') [-Wformat]
printf("batch insert elapsed: %ld.%06lds\n",
elapsed_time.tv_sec, elapsed_time.tv_usec);
macos complains here. You may want to replace %06lds by just %06d.

(lldb) bt
* thread #1: tid = 0x0000, 0x0000000108bcdd56
libpq.5.dylib`closePGconn(conn=0x00007fd642d002d0) + 438 at
fe-connect.c:3010, stop reason = signal SIGSTOP
* frame #0: 0x0000000108bcdd56
libpq.5.dylib`closePGconn(conn=0x00007fd642d002d0) + 438 at
fe-connect.c:3010
frame #1: 0x0000000108bc9db0
libpq.5.dylib`PQfinish(conn=0x00007fd642d002d0) + 32 at
fe-connect.c:3072
frame #2: 0x0000000108bc9ede
libpq.5.dylib`PQping(conninfo="dbname=postgres port=5432 host='/tmp'
connect_timeout=5") + 46 at fe-connect.c:539
frame #3: 0x0000000108bb5210
pg_ctl`test_postmaster_connection(pm_pid=78218, do_checkpoint='\0') +
976 at pg_ctl.c:681
frame #4: 0x0000000108bb388e pg_ctl`do_start + 302 at pg_ctl.c:915
frame #5: 0x0000000108bb29b4 pg_ctl`main(argc=5,
argv=0x00007fff5704e5c0) + 2836 at pg_ctl.c:2416
frame #6: 0x00007fff8b8b65ad libdyld.dylib`start + 1
(lldb) down 1
frame #0: 0x0000000108bcdd56
libpq.5.dylib`closePGconn(conn=0x00007fd642d002d0) + 438 at
fe-connect.c:3010
3007 queue = conn->cmd_queue_recycle;
3008 {
3009 PGcommandQueueEntry *prev = queue;
-> 3010 queue = queue->next;
3011 free(prev);
3012 }
3013 conn->cmd_queue_recycle = NULL;
This patch generates a core dump, use for example pg_ctl start -w and
you'll bump into the trace above. There is something wrong with the
queue handling.

Do you have plans for a more generic structure for the command queue list?

+ <application>libpq</application> supports queueing up mulitiple queries into
s/mulitiple/multiple/.

+  <para>
+   An example of batch use may be found in the source distribution in
+   <filename>src/test/examples/libpqbatch.c</filename>.
+  </para>
You mean testlibpqbatch.c here.
+   <para>
+    Batching less useful when information from one operation is required by the
+    client before it knows enough to send the next operation. The client must
"Batching *is* less useful".

src/test/examples/.gitignore needs a new entry for the new test binary.

+           fprintf(stderr, "internal error, COPY in batch mode");
+           abort();
I don't think that's a good idea. defaultNoticeProcessor can be
overridden to allow applications to have error messages sent
elsewhere. Error messages should also use libpq_gettext, and perhaps
be stored in conn->errorMessage as we do so for OOMs happening on
client-side and reporting them back even if they are not expected
(those are blocked PQsendQueryStart in your patch).

src/test/examples is a good idea to show people what this new API can
do, but this is never getting compiled. It could as well be possible
to include tests in src/test/modules/, in the same shape as what
postgres_fdw is doing by connecting to itself and link it to libpq. As
this patch complicates quote a lot fe-exec.c, I think that this would
be worth it. Thoughts?
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#12Craig Ringer
craig@2ndquadrant.com
In reply to: Michael Paquier (#11)
Re: PATCH: Batch/pipelining support for libpq

On 10 August 2016 at 14:44, Michael Paquier <michael.paquier@gmail.com>
wrote:

On Fri, Jun 3, 2016 at 8:51 PM, Dmitry Igrishin <dmitigr@gmail.com> wrote:

BTW, I've publushed the HTML-ified SGML docs to
http://2ndquadrant.github.io/postgres/libpq-batch-mode.html as a

preview.

Typo detected: "Returns 1 if the batch curently being received" --

"curently".

I am looking a bit more seriously at this patch and assigned myself as
a reviewer.

Much appreciated.

testlibpqbatch.c:1239:73: warning: format specifies type 'long' but
the argument has type '__darwin_suseconds_t' (aka 'int') [-Wformat]
printf("batch insert elapsed: %ld.%06lds\n",
elapsed_time.tv_sec, elapsed_time.tv_usec);
macos complains here. You may want to replace %06lds by just %06d.

Yeah, or cast to a type known to be big enough. Will amend.

This patch generates a core dump, use for example pg_ctl start -w and
you'll bump into the trace above. There is something wrong with the
queue handling.

Huh. I didn't see that here (Fedora 23). I'll look more closely.

Do you have plans for a more generic structure for the command queue list?

No plans, no. This was a weekend experiment that turned into a useful patch
and I'm having to scrape up time for it amongst much more important things
like logical failover / sequence decoding and various other replication
work.

Thanks for the docs review too, will amend.

+           fprintf(stderr, "internal error, COPY in batch mode");
+           abort();
I don't think that's a good idea. defaultNoticeProcessor can be
overridden to allow applications to have error messages sent
elsewhere. Error messages should also use libpq_gettext, and perhaps
be stored in conn->errorMessage as we do so for OOMs happening on
client-side and reporting them back even if they are not expected
(those are blocked PQsendQueryStart in your patch).

src/test/examples is a good idea to show people what this new API can
do, but this is never getting compiled. It could as well be possible
to include tests in src/test/modules/, in the same shape as what
postgres_fdw is doing by connecting to itself and link it to libpq. As
this patch complicates quote a lot fe-exec.c, I think that this would
be worth it. Thoughts?

I didn't think it added much complexity to fe-exec.c personally. A lot of
the appeal is that it has very minor impact on anything that isn't using it.

I think it makes sense to (ab)use the recovery module tests for this,
invoking the test program from there.

Ideally I'd like to teach pgsql and pg_restore how to use async mode, but
that's a whole separate patch.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

#13Craig Ringer
craig@2ndquadrant.com
In reply to: Craig Ringer (#12)
Re: PATCH: Batch/pipelining support for libpq

On 23 August 2016 at 08:27, Craig Ringer <craig@2ndquadrant.com> wrote:

On 10 August 2016 at 14:44, Michael Paquier <michael.paquier@gmail.com>
wrote:

I am looking a bit more seriously at this patch and assigned myself as

a reviewer.

Much appreciated.

macos complains here. You may want to replace %06lds by just %06d.

Yeah, or cast to a type known to be big enough. Will amend.

I used an upcast to (long), because on Linux it's a long. I don't see the
point of messing about adding a configure test for something this trivial.

This patch generates a core dump, use for example pg_ctl start -w and

you'll bump into the trace above. There is something wrong with the
queue handling.

Huh. I didn't see that here (Fedora 23). I'll look more closely.

Do you have plans for a more generic structure for the command queue list?

No plans, no. This was a weekend experiment that turned into a useful
patch and I'm having to scrape up time for it amongst much more important
things like logical failover / sequence decoding and various other
replication work.

Thanks for the docs review too, will amend.

+           fprintf(stderr, "internal error, COPY in batch mode");
+           abort();
I don't think that's a good idea.

My thinking there was that it's a "shouldn't happen" case. It's a problem
in library logic. I'd use an Assert() here in the backend.

I could printfPQExpBuffer(...) an error and return failure instead if you
think that's more appropriate. I'm not sure how the app would handle it
correctly, but OTOH it's generally better for libraries not to call
abort(). So I'll do that, but since it's an internal error that's not meant
to happen I'll skip the gettext calls.

Error messages should also use libpq_gettext, and perhaps

be stored in conn->errorMessage as we do so for OOMs happening on
client-side and reporting them back even if they are not expected
(those are blocked PQsendQueryStart in your patch).

I didn't get that last part, re PQsendQueryStart.

src/test/examples is a good idea to show people what this new API can

do, but this is never getting compiled. It could as well be possible
to include tests in src/test/modules/, in the same shape as what
postgres_fdw is doing by connecting to itself and link it to libpq. As
this patch complicates quote a lot fe-exec.c, I think that this would
be worth it. Thoughts?

I think it makes sense to use the TAP framework. Added
src/test/modules/test_libpq/ with a test for async mode. Others can be
added/migrated based on that. I thought it made more sense for the tests to
live there than in src/interfaces//libpq/ since they need test client
programs and shouldn't pollute the library directory.

I've made the docs changes too. Thanks.

I fixed the list handling error. I'm amazed it appears to run fine, and
without complaint from valgrind, here, since it was an accidentally
_deleted_ line.

Re lists, I looked at simple_list.c and it's exceedingly primitive. Using
it would mean more malloc()ing since we'll have a list cell and then a
struct pointed to it, and won't recycle members, but... whatever. It's not
going to matter a great deal. The reason I did it with an embedded list
originally was because that's how it's done for PGnotify, but that's not
exactly new code

The bigger problem is that simple_list also uses pg_malloc, which won't set
conn->errorMessage, it'll just fprintf() and exit(1). I'm not convinced
it's appropriate to use that for libpq.

For now I've left list handling unchanged. If it's to move to a generic
list, it should probably be one that knows how to use
pg_malloc_extended(size, MCXT_ALLOC_NO_OOM) and emit its own
libpq-error-handling-aware error. I'm not sure whether that list should use
cell heads embedded in the structures it manages or pointing to them,
either.

Updated patch attached.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachments:

0001-Pipelining-batch-support-for-libpq-v2.patchtext/x-patch; charset=US-ASCII; name=0001-Pipelining-batch-support-for-libpq-v2.patchDownload+2571-44
#14Daniel Verite
daniel@manitou-mail.org
In reply to: Craig Ringer (#13)
Re: PATCH: Batch/pipelining support for libpq

Craig Ringer wrote:

Updated patch attached.

Please find attached a couple fixes for typos I've came across in
the doc part.

Also it appears that PQqueriesInBatch() doesn't work as documented.
It says:
"Returns the number of queries still in the queue for this batch"
but in fact it's implemented as a boolean:

+/* PQqueriesInBatch
+ *   Return true if there are queries currently pending in batch mode
+ */+int
+PQqueriesInBatch(PGconn *conn)
+{
+	if (!PQisInBatchMode(conn))
+		return false;
+
+	return conn->cmd_queue_head != NULL;
+}

However, is this function really needed? It doesn't seem essential to
the API. You don't call it in the test program either.

Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite

Attachments:

diff-typos.txttext/plain; name=diff-typos.txtDownload+4-4
#15Craig Ringer
craig@2ndquadrant.com
In reply to: Daniel Verite (#14)
Re: PATCH: Batch/pipelining support for libpq

On 6 September 2016 at 16:10, Daniel Verite <daniel@manitou-mail.org> wrote:

Craig Ringer wrote:

Updated patch attached.

Please find attached a couple fixes for typos I've came across in
the doc part.

Thanks, will apply and post a rebased patch soon, or if someone picks
this up in the mean time they can apply your diff on top of the patch.

Also it appears that PQqueriesInBatch() doesn't work as documented.
It says:
"Returns the number of queries still in the queue for this batch"
but in fact it's implemented as a boolean:

Whoops. Will fix.

I think the function is useful and necessary. There's no reason not to
expose that, but also it's good for when your query dispatch isn't as
tightly coupled to your query handling as in the example, so your app
might need to keep processing until it sees the end of queued results.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#16Michael Paquier
michael@paquier.xyz
In reply to: Craig Ringer (#15)
Re: PATCH: Batch/pipelining support for libpq

On Tue, Sep 6, 2016 at 8:01 PM, Craig Ringer <craig@2ndquadrant.com> wrote:

On 6 September 2016 at 16:10, Daniel Verite <daniel@manitou-mail.org> wrote:

Craig Ringer wrote:

Updated patch attached.

Please find attached a couple fixes for typos I've came across in
the doc part.

Thanks, will apply and post a rebased patch soon, or if someone picks
this up in the mean time they can apply your diff on top of the patch.

Could you send an updated patch then? At the same time I am noticing
that git --check is complaining... This patch has tests and a
well-documented feature, so I'll take a look at it soon at the top of
my list. Moved to next CF for now.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#17Craig Ringer
craig@2ndquadrant.com
In reply to: Michael Paquier (#16)
Re: PATCH: Batch/pipelining support for libpq

On 3 October 2016 at 10:10, Michael Paquier <michael.paquier@gmail.com> wrote:

On Tue, Sep 6, 2016 at 8:01 PM, Craig Ringer <craig@2ndquadrant.com> wrote:

On 6 September 2016 at 16:10, Daniel Verite <daniel@manitou-mail.org> wrote:

Craig Ringer wrote:

Updated patch attached.

Please find attached a couple fixes for typos I've came across in
the doc part.

Thanks, will apply and post a rebased patch soon, or if someone picks
this up in the mean time they can apply your diff on top of the patch.

Could you send an updated patch then? At the same time I am noticing
that git --check is complaining... This patch has tests and a
well-documented feature, so I'll take a look at it soon at the top of
my list. Moved to next CF for now.

Thanks.

I'd really like to teach psql in non-interactive mode to use it, but
(a) I'm concerned about possible subtle behaviour differences arising
if we do that and (b) I won't have the time. I think it's mostly of
interest to app authors and driver developers and that's what it's
aimed at. pg_restore could benefit a lot too.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#18Daniel Verite
daniel@manitou-mail.org
In reply to: Craig Ringer (#17)
Re: PATCH: Batch/pipelining support for libpq

Craig Ringer wrote:

I think it's mostly of interest to app authors and driver developers
and that's what it's aimed at. pg_restore could benefit a lot too.

Wouldn't pgbench benefit from it?
It was mentioned some time ago [1]/messages/by-id/alpine.DEB.2.20.1607140925400.1962@sto, in relationship to the
\into construct, how client-server latency was important enough to
justify the use of a "\;" separator between statements, to send them
as a group.

But with the libpq batch API, maybe this could be modernized
with meta-commands like this:
\startbatch
...
\endbatch
which would essentially call PQbeginBatchMode() and PQsendEndBatch().
Inside the batch section, collecting results would be interleaved
with sending queries.
Interdepencies between results and subsequent queries could
be handled or ignored, depending on how sophisticated we'd want
this.

This might also draw more users to the batch API, because
it would make it easier to check how exactly it affects
the performance of specific sequences of SQL statements to be
grouped in a batch.
For instance it would make sense for programmers to benchmark mock-ups
of their code with pgbench with/without batching, before embarking on
adapting it from blocking mode to asynchronous/non-blocking mode.

[1]: /messages/by-id/alpine.DEB.2.20.1607140925400.1962@sto
/messages/by-id/alpine.DEB.2.20.1607140925400.1962@sto

Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#19Michael Paquier
michael@paquier.xyz
In reply to: Daniel Verite (#18)
Re: PATCH: Batch/pipelining support for libpq

On Mon, Oct 3, 2016 at 11:52 PM, Daniel Verite <daniel@manitou-mail.org> wrote:

Wouldn't pgbench benefit from it?
It was mentioned some time ago [1], in relationship to the
\into construct, how client-server latency was important enough to
justify the use of a "\;" separator between statements, to send them
as a group.

But with the libpq batch API, maybe this could be modernized
with meta-commands like this:
\startbatch
...
\endbatch

Or just \batch [on|off], which sounds like a damn good idea to me for
some users willing to test some workloads before integrating it in an
application.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#20Craig Ringer
craig@2ndquadrant.com
In reply to: Michael Paquier (#19)
Re: PATCH: Batch/pipelining support for libpq

On 4 Oct. 2016 15:15, "Michael Paquier" <michael.paquier@gmail.com> wrote:

On Mon, Oct 3, 2016 at 11:52 PM, Daniel Verite <daniel@manitou-mail.org>

wrote:

Wouldn't pgbench benefit from it?
It was mentioned some time ago [1], in relationship to the
\into construct, how client-server latency was important enough to
justify the use of a "\;" separator between statements, to send them
as a group.

But with the libpq batch API, maybe this could be modernized
with meta-commands like this:
\startbatch
...
\endbatch

Or just \batch [on|off], which sounds like a damn good idea to me for
some users willing to test some workloads before integrating it in an
application.

A batch jsnt necessarily terminated by a commit, so I'm more keen on
start/end batch. It's more in line with begin/commit. Batch is not only a
mode, you also have to delineate batches.

#21Gavin Flower
GavinFlower@archidevsys.co.nz
In reply to: Michael Paquier (#19)
#22Michael Paquier
michael@paquier.xyz
In reply to: Craig Ringer (#17)
#23Shay Rojansky
roji@roji.org
In reply to: Craig Ringer (#1)
#24Jim Nasby
Jim.Nasby@BlueTreble.com
In reply to: Michael Paquier (#22)
#25Craig Ringer
craig@2ndquadrant.com
In reply to: Shay Rojansky (#23)
#26Shay Rojansky
roji@roji.org
In reply to: Craig Ringer (#25)
#27Craig Ringer
craig@2ndquadrant.com
In reply to: Shay Rojansky (#26)
#28Shay Rojansky
roji@roji.org
In reply to: Craig Ringer (#27)
#29Craig Ringer
craig@2ndquadrant.com
In reply to: Shay Rojansky (#28)
#30Tsunakawa, Takayuki
tsunakawa.takay@jp.fujitsu.com
In reply to: Craig Ringer (#17)
#31Craig Ringer
craig@2ndquadrant.com
In reply to: Tsunakawa, Takayuki (#30)
#32Haribabu Kommi
kommi.haribabu@gmail.com
In reply to: Craig Ringer (#31)
#33Craig Ringer
craig@2ndquadrant.com
In reply to: Haribabu Kommi (#32)
#34Iwata, Aya
iwata.aya@jp.fujitsu.com
In reply to: Craig Ringer (#33)
#35Prabakaran, Vaishnavi
vaishnavip@fast.au.fujitsu.com
In reply to: Craig Ringer (#33)
#36Michael Paquier
michael@paquier.xyz
In reply to: Prabakaran, Vaishnavi (#35)
#37Vaishnavi Prabakaran
vaishnaviprabakaran@gmail.com
In reply to: Michael Paquier (#36)
#38Craig Ringer
craig@2ndquadrant.com
In reply to: Vaishnavi Prabakaran (#37)
#39Daniel Verite
daniel@manitou-mail.org
In reply to: Vaishnavi Prabakaran (#37)
#40Craig Ringer
craig@2ndquadrant.com
In reply to: Daniel Verite (#39)
#41Vaishnavi Prabakaran
vaishnaviprabakaran@gmail.com
In reply to: Daniel Verite (#39)
#42Daniel Verite
daniel@manitou-mail.org
In reply to: Vaishnavi Prabakaran (#41)
#43Daniel Verite
daniel@manitou-mail.org
In reply to: Vaishnavi Prabakaran (#41)
#44Vaishnavi Prabakaran
vaishnaviprabakaran@gmail.com
In reply to: Daniel Verite (#43)
#45Craig Ringer
craig@2ndquadrant.com
In reply to: Vaishnavi Prabakaran (#44)
#46Daniel Verite
daniel@manitou-mail.org
In reply to: Vaishnavi Prabakaran (#44)
#47Vaishnavi Prabakaran
vaishnaviprabakaran@gmail.com
In reply to: Daniel Verite (#46)
#48Vaishnavi Prabakaran
vaishnaviprabakaran@gmail.com
In reply to: Vaishnavi Prabakaran (#47)
#49Daniel Verite
daniel@manitou-mail.org
In reply to: Vaishnavi Prabakaran (#47)
#50Vaishnavi Prabakaran
vaishnaviprabakaran@gmail.com
In reply to: Daniel Verite (#49)
#51David Steele
david@pgmasters.net
In reply to: Vaishnavi Prabakaran (#50)
#52Craig Ringer
craig@2ndquadrant.com
In reply to: David Steele (#51)
#53Michael Paquier
michael@paquier.xyz
In reply to: Craig Ringer (#52)
#54Craig Ringer
craig@2ndquadrant.com
In reply to: Michael Paquier (#53)
#55Michael Paquier
michael@paquier.xyz
In reply to: Craig Ringer (#54)
#56Daniel Verite
daniel@manitou-mail.org
In reply to: Vaishnavi Prabakaran (#50)
#57Daniel Verite
daniel@manitou-mail.org
In reply to: Vaishnavi Prabakaran (#50)
#58Vaishnavi Prabakaran
vaishnaviprabakaran@gmail.com
In reply to: Daniel Verite (#57)
#59Michael Paquier
michael@paquier.xyz
In reply to: Vaishnavi Prabakaran (#58)
#60Daniel Verite
daniel@manitou-mail.org
In reply to: Michael Paquier (#59)
#61Vaishnavi Prabakaran
vaishnaviprabakaran@gmail.com
In reply to: Daniel Verite (#60)
#62Michael Paquier
michael@paquier.xyz
In reply to: Vaishnavi Prabakaran (#61)
#63Vaishnavi Prabakaran
vaishnaviprabakaran@gmail.com
In reply to: Michael Paquier (#62)
#64Daniel Verite
daniel@manitou-mail.org
In reply to: Vaishnavi Prabakaran (#63)
#65David Steele
david@pgmasters.net
In reply to: Daniel Verite (#64)
#66Vaishnavi Prabakaran
vaishnaviprabakaran@gmail.com
In reply to: David Steele (#65)
#67Andres Freund
andres@anarazel.de
In reply to: Vaishnavi Prabakaran (#66)
#68Vaishnavi Prabakaran
vaishnaviprabakaran@gmail.com
In reply to: Andres Freund (#67)
#69Andres Freund
andres@anarazel.de
In reply to: Vaishnavi Prabakaran (#68)
#70Michael Paquier
michael@paquier.xyz
In reply to: Andres Freund (#69)
#71Andres Freund
andres@anarazel.de
In reply to: Michael Paquier (#70)
#72Vaishnavi Prabakaran
vaishnaviprabakaran@gmail.com
In reply to: Andres Freund (#71)
#73Andres Freund
andres@anarazel.de
In reply to: Vaishnavi Prabakaran (#72)
#74Andres Freund
andres@anarazel.de
In reply to: Andres Freund (#73)
#75Vaishnavi Prabakaran
vaishnaviprabakaran@gmail.com
In reply to: Andres Freund (#74)
#76Craig Ringer
craig@2ndquadrant.com
In reply to: Andres Freund (#74)
#77Michael Paquier
michael@paquier.xyz
In reply to: Craig Ringer (#76)
#78Daniel Verite
daniel@manitou-mail.org
In reply to: Andres Freund (#74)
#79Andres Freund
andres@anarazel.de
In reply to: Daniel Verite (#78)
#80Craig Ringer
craig@2ndquadrant.com
In reply to: Andres Freund (#79)
#81Andres Freund
andres@anarazel.de
In reply to: Andres Freund (#79)
#82Craig Ringer
craig@2ndquadrant.com
In reply to: Andres Freund (#81)
#83Andres Freund
andres@anarazel.de
In reply to: Craig Ringer (#82)
#84Craig Ringer
craig@2ndquadrant.com
In reply to: Andres Freund (#83)
#85Andres Freund
andres@anarazel.de
In reply to: Andres Freund (#83)
#86Daniel Verite
daniel@manitou-mail.org
In reply to: Craig Ringer (#82)
#87Daniel Verite
daniel@manitou-mail.org
In reply to: Andres Freund (#85)
#88Andres Freund
andres@anarazel.de
In reply to: Daniel Verite (#87)
#89Daniel Verite
daniel@manitou-mail.org
In reply to: Andres Freund (#88)
#90Vaishnavi Prabakaran
vaishnaviprabakaran@gmail.com
In reply to: Daniel Verite (#89)
#91Andres Freund
andres@anarazel.de
In reply to: Vaishnavi Prabakaran (#90)
#92Vaishnavi Prabakaran
vaishnaviprabakaran@gmail.com
In reply to: Andres Freund (#91)
#93Craig Ringer
craig@2ndquadrant.com
In reply to: Vaishnavi Prabakaran (#92)
#94Vaishnavi Prabakaran
vaishnaviprabakaran@gmail.com
In reply to: Craig Ringer (#93)
#95Craig Ringer
craig@2ndquadrant.com
In reply to: Vaishnavi Prabakaran (#94)
#96Daniel Gustafsson
daniel@yesql.se
In reply to: Vaishnavi Prabakaran (#94)
#97Vaishnavi Prabakaran
vaishnaviprabakaran@gmail.com
In reply to: Daniel Gustafsson (#96)
#98Michael Paquier
michael@paquier.xyz
In reply to: Vaishnavi Prabakaran (#97)
#99Vaishnavi Prabakaran
vaishnaviprabakaran@gmail.com
In reply to: Michael Paquier (#98)
#100Vaishnavi Prabakaran
vaishnaviprabakaran@gmail.com
In reply to: Vaishnavi Prabakaran (#99)
#101Kyotaro Horiguchi
horikyota.ntt@gmail.com
In reply to: Vaishnavi Prabakaran (#100)
#102Daniel Verite
daniel@manitou-mail.org
In reply to: Kyotaro Horiguchi (#101)
#103Michael Paquier
michael@paquier.xyz
In reply to: Daniel Verite (#102)
#104Dmitry Dolgov
9erthalion6@gmail.com
In reply to: Michael Paquier (#103)
#105Nikhil Sontakke
nikhils@2ndquadrant.com
In reply to: Dmitry Dolgov (#104)
#106Amit Kapila
amit.kapila16@gmail.com
In reply to: Nikhil Sontakke (#105)
#107Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Amit Kapila (#106)
#108Andres Freund
andres@anarazel.de
In reply to: Alvaro Herrera (#107)
#109Ibrar Ahmed
ibrar.ahmad@gmail.com
In reply to: Andres Freund (#108)
#110Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Ibrar Ahmed (#109)
#111Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Alvaro Herrera (#110)
#112Ibrar Ahmed
ibrar.ahmad@gmail.com
In reply to: Alvaro Herrera (#111)
#113Justin Pryzby
pryzby@telsasoft.com
In reply to: Alvaro Herrera (#111)
#114Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Justin Pryzby (#113)
#115Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Alvaro Herrera (#114)
#116Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Alvaro Herrera (#115)
#117Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Alvaro Herrera (#116)
#118Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Alvaro Herrera (#117)
#119Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Alvaro Herrera (#118)
#120Zhihong Yu
zyu@yugabyte.com
In reply to: Alvaro Herrera (#119)
#121Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Alvaro Herrera (#119)
#122Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Alvaro Herrera (#121)
#123Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#122)
#124Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Tom Lane (#123)
#125Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Alvaro Herrera (#124)
#126Justin Pryzby
pryzby@telsasoft.com
In reply to: Alvaro Herrera (#125)
#127Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Justin Pryzby (#126)
#128Andres Freund
andres@anarazel.de
In reply to: Alvaro Herrera (#121)
#129Matthieu Garrigues
matthieu.garrigues@gmail.com
In reply to: Andres Freund (#128)