public inbox for [email protected]  
help / color / mirror / Atom feed
From: Michael Paquier <[email protected]>
To: Jelte Fennema-Nio <[email protected]>
Cc: Anthonin Bonnefoy <[email protected]>
Cc: Anton Kirilov <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: [email protected]
Subject: Re: Add PQsendSyncMessage() to libpq
Date: Wed, 10 Jan 2024 15:40:36 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<CAO6_Xqo_H3-ykt_Hmip-TKNBVWZbRthLvFu4qTpQSSEgV4dWmA@mail.gmail.com>
	<CAGECzQQqkkvSko4fZUBAmVMd=VmC4nXdyOBO=s_42HzKJ=GaEA@mail.gmail.com>
	<[email protected]>

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 makes it see what perf improvements can be gained
>> for your workload.
> 
> Yeah, that sounds like a good idea seen from here.  (Still need to
> look at the core patch.)

 PQpipelineSync(PGconn *conn)
+{
+	return PQsendPipelineSync(conn) && pqFlush(conn) >= 0;
+}
[...]
+	 * Give the data a push if we're past the size threshold. In nonblock
+	 * mode, don't complain if we're unable to send it all; the caller is
+	 * expected to execute PQflush() at some point anyway.
 	 */
-	if (PQflush(conn) < 0)
+	if (pqPipelineFlush(conn) < 0)
 		goto sendFailed;

I was looking at this patch, and calling PQpipelineSync() would now
cause two calls of PQflush() to be issued when the output buffer
threshold has been reached.  Could that lead to regressions?

A second thing I find disturbing is that pqAppendCmdQueueEntry() would
be called before the final pqFlush(), which could cause the commands
to be listed in a queue even if the flush fails when calling
PQpipelineSync().

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 make the extra argument a boolean.
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
  download

view thread (15+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Add PQsendSyncMessage() to libpq
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox