Re: Add PQsendSyncMessage() to libpq

Hello, On 17/01/2024 07:30, Michael Paquier wrote: Thanks for double-checking. Done. Thank you very much for taking care of my patch! One thing that I noticed is that the TODO list on the PostgreSQL Wiki still contained an entry ( https://wiki.postgresql.org/wiki/Todo#libpq ) about adding

Re: Add PQsendSyncMessage() to libpq

On Tue, Jan 16, 2024 at 02:55:12PM +0100, Alvaro Herrera wrote: > On 2024-Jan-16, Michael Paquier wrote: >> How about the attached to remove all that, then? > > Looks good, thank you. Thanks for double-checking. Done. -- Michael signature.asc Description: PGP signature

Re: Add PQsendSyncMessage() to libpq

On 2024-Jan-16, Michael Paquier wrote: > I've applied the patch with all these modifications to move on with > the subject. Thanks! > On Mon, Jan 15, 2024 at 10:49:56AM +0100, Alvaro Herrera wrote: > > Looking again at the largish comment that's now atop > > pqPipelineSyncInternal(), I think

Re: Add PQsendSyncMessage() to libpq

On Mon, Jan 15, 2024 at 10:49:56AM +0100, Alvaro Herrera wrote: > the new function pqPipelineSyncInternal is not a wrapper for these other > two functions -- the opposite is true actually. We tend to use the term > "workhorse" or "internal workhorse" for this kind of thing. Indeed, makes sense.

Re: Add PQsendSyncMessage() to libpq

On Mon, Jan 15, 2024 at 10:01:59AM +0100, Jelte Fennema-Nio wrote: > Error message should be "second SELECT" not "first SELECT". Same note > for the error message in the third pipeline, where it still says > "second SELECT". > > same issue: s/first/second/g (and s/second/third/g for the existing >

Re: Add PQsendSyncMessage() to libpq

On 2024-Jan-15, Michael Paquier wrote: Looks good! Just some small notes, > +/* > + * Wrapper for PQpipelineSync and PQsendPipelineSync. > * > * It's legal to start submitting more commands in the pipeline immediately, > * without waiting for the results of the current pipeline. There's

Re: Add PQsendSyncMessage() to libpq

On Mon, 15 Jan 2024 at 08:50, Michael Paquier wrote: > Yeah, I'll go with that after a second look. Attached is what I am > finishing with, and I have reproduced some numbers with the pgbench > metacommand mentioned upthread, which is reeeaaally nice. Code looks good to me. But one small notes

Re: Add PQsendSyncMessage() to libpq

On Wed, Jan 10, 2024 at 03:40:36PM +0900, Michael Paquier wrote: > Hence, as a whole, wouldn't it be more consistent if the new > PQsendPipelineSync() and the existing PQpipelineSync() call an > internal static routine (PQPipelineSyncInternal?) that can switch > between both modes? Let's just

Re: Add PQsendSyncMessage() to libpq

On Sun, Dec 31, 2023 at 09:37:31AM +0900, Michael Paquier wrote: > On Fri, Dec 29, 2023 at 12:52:30PM +0100, Jelte Fennema-Nio wrote: >> Those are some nice improvements. And I think once this patch is in, >> it would make sense to add the pgbench feature you're suggesting. >> Because indeed that

Re: Add PQsendSyncMessage() to libpq

On Fri, Dec 29, 2023 at 12:52:30PM +0100, Jelte Fennema-Nio wrote: > On Mon, 13 Nov 2023 at 09:20, Anthonin Bonnefoy > wrote: > > \syncpipeline > > tps = 2607.587877 (without initial connection time) > > ... > > \endpipeline > > \startpipeline > > tps = 2290.527462 (without initial connection

Re: Add PQsendSyncMessage() to libpq

On Mon, 13 Nov 2023 at 09:20, Anthonin Bonnefoy wrote: > \syncpipeline > tps = 2607.587877 (without initial connection time) > ... > \endpipeline > \startpipeline > tps = 2290.527462 (without initial connection time) Those are some nice improvements. And I think once this patch is in, it would

Re: Add PQsendSyncMessage() to libpq

On Sun, 12 Nov 2023 at 14:37, Anton Kirilov wrote: > Since there haven't been any comments from the other people who have > chimed in on this e-mail thread, I will assume that there is consensus > (we are doing a U-turn with the implementation approach after all), so > here is the updated version

Re: Add PQsendSyncMessage() to libpq

Hi, I've played a bit with the patch on my side. One thing that would be great would be to make this available in pgbench through a \syncpipeline meta command. That would make it easier for users to test whether there's a positive impact with their queries or not. I've wrote a patch to add it to

Re: Add PQsendSyncMessage() to libpq

Hello, Thanks for the feedback! On 07/11/2023 09:23, Jelte Fennema-Nio wrote: > But I think it's looking at the situation from the wrong direction. [...] we should look at it as an addition to our current list of PQsend functions for a new packet type. And none of those PQsend functions ever

Re: Add PQsendSyncMessage() to libpq

On 2023-Nov-07, Jelte Fennema-Nio wrote: > I think this function should be named something with the "PQsend" > prefix since that's the way we name all our public async message > sending functions in libpq. The "Put" word we only use in internal > libpq functions, so I feel it has no place in the

Re: Add PQsendSyncMessage() to libpq

On Fri, 28 Apr 2023 at 14:07, Robert Haas wrote: > I wonder whether this is the naming that we want. The two names are > significantly different. Something like PQpipelineSendSync() would be > more similar. > > I also wonder, really even more, whether it would be better to do > something like

Re: Add PQsendSyncMessage() to libpq

Hello, On 05/07/2023 21:45, Daniel Gustafsson wrote: Please rebase and send an updated version. Here it is (including the warning fix). Thanks, Anton KirilovFrom 3c2e064a151568830e4d9e4bf238739b458350b4 Mon Sep 17 00:00:00 2001 From: Anton Kirilov Date: Wed, 22 Mar 2023 20:39:57 +

Re: Add PQsendSyncMessage() to libpq

> On 21 May 2023, at 19:17, Anton Kirilov wrote: > .. here is an updated version of the patch This hunk here: - if (PQflush(conn) < 0) + const int ret = flags & PG_PIPELINEPUTSYNC_FLUSH ? PQflush(conn) : 0; + + if (ret < 0) ..is causing this compiler warning: fe-exec.c: In

Re: Add PQsendSyncMessage() to libpq

Hello, On 02/05/2023 00:55, Michael Paquier wrote: > Well, these are nice numbers. At ~1% I am ready to buy the noise > argument, but what would the range of the usual noise when it comes to > multiple runs under the same conditions> I managed to get my patch tested in the TechEmpower

Re: Add PQsendSyncMessage() to libpq

Hello, On 05/05/2023 16:02, Anton Kirilov wrote: On Thu, 4 May 2023, 11:36 Alvaro Herrera, > wrote: On 2023-May-04, Anton Kirilov wrote: If you want to make sure it's fully flushed, your only option is to have the call block. Surely PQflush()

Re: Add PQsendSyncMessage() to libpq

On Wed, May 03, 2023 at 12:03:57PM +0200, Alvaro Herrera wrote: > We already have 'int' flag masks in PQcopyResult() and > PQsetTraceFlags(). We were using bits32 initially for flag stuff in the > PQtrace facilities, until [1] reminded us that we shouldn't let c.h > creep into app-land, so that

Re: Add PQsendSyncMessage() to libpq

Hello, On Thu, 4 May 2023, 11:36 Alvaro Herrera, mailto:alvhe...@alvh.no-ip.org>> wrote: > On 2023-May-04, Anton Kirilov wrote: > If you want to make sure it's fully flushed, your only option is to have > the call block. Surely PQflush() returning 0 would signify that the output buffer has been

Re: Add PQsendSyncMessage() to libpq

On 2023-May-04, Anton Kirilov wrote: > Thank you all for the feedback! Do you have any thoughts on the other > issue with PQpipelineSync() I have mentioned in my previous message? Eh, I hadn't seen that one. > Am I just misunderstanding what the code comment means and how the API > is supposed

Re: Add PQsendSyncMessage() to libpq

Hello, > On 3 May 2023, at 11:03, Alvaro Herrera wrote: > > On 2023-May-02, Robert Haas wrote: > >> On Mon, May 1, 2023 at 8:42 PM Michael Paquier wrote: >>> Another thing that may matter in terms of extensibility? Would a >>> boolean argument really be the best design? Could it be better

Re: Add PQsendSyncMessage() to libpq

On 2023-May-02, Robert Haas wrote: > On Mon, May 1, 2023 at 8:42 PM Michael Paquier wrote: > > Another thing that may matter in terms of extensibility? Would a > > boolean argument really be the best design? Could it be better to > > have instead one API with a bits32 and some flags

Re: Add PQsendSyncMessage() to libpq

On Mon, May 1, 2023 at 8:42 PM Michael Paquier wrote: > Another thing that may matter in terms of extensibility? Would a > boolean argument really be the best design? Could it be better to > have instead one API with a bits32 and some flags controlling its > internals? I wondered that, too. If

Re: Add PQsendSyncMessage() to libpq

On Sat, Apr 29, 2023 at 05:06:03PM +0100, Anton Kirilov wrote: > In any case I am not particularly attached to any naming or the exact shape > of the new API, as long as it achieves the same goal (reducing the number of > system calls), but before I make any substantial changes to my patch, I >

Re: Add PQsendSyncMessage() to libpq

On Sun, Apr 30, 2023 at 01:59:17AM +0100, Anton Kirilov wrote: > I did a quick check using the TechEmpower Framework Benchmarks ( > https://www.techempower.com/benchmarks/ ) - they define 4 Web application > tests that are database-bound. Everything was running on a single machine, > and 3 of the

Re: Add PQsendSyncMessage() to libpq

Hello, On 28/04/2023 09:08, Denis Laxalde wrote: Michael Paquier a écrit : Speaking of which, what was the performance impact of your application once PQflush() was moved out of the pipeline sync?  Just asking for curiosity.. I have no metrics for that; but maybe Anton has some? I did a

Re: Add PQsendSyncMessage() to libpq

Hello, On 28/04/2023 13:06, Robert Haas wrote: On Fri, Mar 24, 2023 at 6:39 PM Anton Kirilov wrote: I have attached a patch that introduces PQsendSyncMessage()... I wonder whether this is the naming that we want. The two names are significantly different. Something like PQpipelineSendSync()

Re: Add PQsendSyncMessage() to libpq

On Fri, Mar 24, 2023 at 6:39 PM Anton Kirilov wrote: > I have attached a patch that introduces PQsendSyncMessage(), a > function that is equivalent to PQpipelineSync(), except that it does > not flush anything to the server; the user must subsequently call > PQflush() instead. Alternatively, the

Re: Add PQsendSyncMessage() to libpq

Michael Paquier a écrit : On Thu, Apr 27, 2023 at 01:06:27PM +0200, Denis Laxalde wrote: Thank you; this V2 looks good to me. Marking as ready for committer. Please note that we are in a stabilization period for v16 and that the first commit fest of v17 should start in July, so it will

Re: Add PQsendSyncMessage() to libpq

On Thu, Apr 27, 2023 at 01:06:27PM +0200, Denis Laxalde wrote: > Thank you; this V2 looks good to me. > Marking as ready for committer. Please note that we are in a stabilization period for v16 and that the first commit fest of v17 should start in July, so it will perhaps take some time before

Re: Add PQsendSyncMessage() to libpq

Hello, Anton Kirilov a écrit : On 25/04/2023 15:23, Denis Laxalde wrote: This sounds like a useful addition to me. I've played a bit with it in Psycopg and it works fine. Thank you very much for reviewing my patch! I have attached a new version of it that addresses your comments and that has

Re: Add PQsendSyncMessage() to libpq

Hello, On 25/04/2023 15:23, Denis Laxalde wrote: > This sounds like a useful addition to me. I've played a bit with it in > Psycopg and it works fine. Thank you very much for reviewing my patch! I have attached a new version of it that addresses your comments and that has been rebased on top of

Re: Add PQsendSyncMessage() to libpq

Anton Kirilov wrote: I would appeciate your thoughts on my proposal. This sounds like a useful addition to me. I've played a bit with it in Psycopg and it works fine. diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c index a16bbf32ef..e2b32c1379 100644 ---