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 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