public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v1 2/2] WIP: buffer alloc specialized for relation extension
15+ messages / 6 participants
[nested] [flat]

* [PATCH v1 2/2] WIP: buffer alloc specialized for relation extension
@ 2020-12-31 12:20 Luc Vlaming <[email protected]>
  0 siblings, 0 replies; 15+ messages in thread

From: Luc Vlaming @ 2020-12-31 12:20 UTC (permalink / raw)

---
 src/backend/storage/buffer/bufmgr.c | 280 +++++++++++++++++++++++++++-
 src/include/storage/buf_internals.h |   2 +-
 2 files changed, 275 insertions(+), 7 deletions(-)

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index d1ce88fee5..14f0923bb5 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -474,6 +474,10 @@ static BufferDesc *BufferAlloc(SMgrRelation smgr,
 							   BlockNumber blockNum,
 							   BufferAccessStrategy strategy,
 							   bool *foundPtr);
+static BufferDesc *BufferAllocExtend(SMgrRelation smgr, 
+			char relpersistence, ForkNumber forkNum,
+			BlockNumber blockNum, BufferAccessStrategy strategy,
+			LWLock** lastPartitionLock);
 static void FlushBuffer(BufferDesc *buf, SMgrRelation reln);
 static void AtProcExit_Buffers(int code, Datum arg);
 static void CheckForBufferLeaks(void);
@@ -996,7 +1000,7 @@ ReadBufferExtendBulk(Relation reln, struct BulkInsertStateData* bistate)
 	BlockNumber	firstBlock,
 				blockNum;
 	Block		bufBlock;
-	bool		found;
+	LWLock*		lastPartitionLock = NULL;
 
 	/* Open it at the smgr level if not already done */
 	RelationOpenSmgr(reln);
@@ -1007,7 +1011,7 @@ ReadBufferExtendBulk(Relation reln, struct BulkInsertStateData* bistate)
 
 	LockRelationForExtension(reln, ExclusiveLock);
 	firstBlock = smgrnblocks(smgr, MAIN_FORKNUM);
-	smgrextend_count(smgr, MAIN_FORKNUM, firstBlock, bistate->empty_buffer, false, BULK_INSERT_BATCH_SIZE);
+	smgrextend_count(smgr, MAIN_FORKNUM, firstBlock, bistate->empty_buffer, true, BULK_INSERT_BATCH_SIZE);
 	UnlockRelationForExtension(reln, ExclusiveLock);
 
 	for (int i=0; i<BULK_INSERT_BATCH_SIZE; ++i)
@@ -1017,12 +1021,10 @@ ReadBufferExtendBulk(Relation reln, struct BulkInsertStateData* bistate)
 		pgstat_count_buffer_read(reln);
 		/* Make sure we will have room to remember the buffer pin */
 		ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
-		bufHdr = BufferAlloc(smgr, relpersistence, MAIN_FORKNUM, blockNum,
-							 bistate->strategy, &found);
+		bufHdr = BufferAllocExtend(smgr, relpersistence, MAIN_FORKNUM, blockNum,
+							 bistate->strategy, &lastPartitionLock);
 		pgBufferUsage.shared_blks_written++;
 
-		Assert(!found);
-
 		bufBlock = BufHdrGetBlock(bufHdr);
 
 		/* new buffers are zero-filled */
@@ -1033,6 +1035,9 @@ ReadBufferExtendBulk(Relation reln, struct BulkInsertStateData* bistate)
 		bistate->local_buffers[i] = BufferDescriptorGetBuffer(bufHdr);
 	}
 
+	Assert(lastPartitionLock);
+	LWLockRelease(lastPartitionLock);
+
 	bistate->local_buffers_idx = 0;
 
 	VacuumPageMiss += BULK_INSERT_BATCH_SIZE;
@@ -1408,6 +1413,269 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
 	return buf;
 }
 
+static BufferDesc *
+BufferAllocExtend(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
+			BlockNumber blockNum, BufferAccessStrategy strategy,
+			LWLock** lastPartitionLock)
+{
+	BufferTag	newTag;			/* identity of requested block */
+	uint32		newHash;		/* hash value for newTag */
+	LWLock	   *newPartitionLock;	/* buffer partition lock for it */
+	BufferTag	oldTag;			/* previous identity of selected buffer */
+	uint32		oldHash;		/* hash value for oldTag */
+	LWLock	   *oldPartitionLock;	/* buffer partition lock for it */
+	uint32		oldFlags;
+	int			buf_id;
+	BufferDesc *buf;
+	uint32		buf_state;
+
+	Assert(lastPartitionLock);
+
+	/* create a tag so we can lookup the buffer */
+	INIT_BUFFERTAG(newTag, smgr->smgr_rnode.node, forkNum, blockNum);
+
+	/* determine its hash code and partition lock ID */
+	newHash = BufTableHashCode(&newTag);
+	newPartitionLock = BufMappingPartitionLock(newHash);
+
+	/* Loop here in case we have to try another victim buffer */
+	for (;;)
+	{
+		/*
+		 * Ensure, while the spinlock's not yet held, that there's a free
+		 * refcount entry.
+		 */
+		ReservePrivateRefCountEntry();
+
+		/*
+		 * Select a victim buffer.  The buffer is returned with its header
+		 * spinlock still held!
+		 */
+		buf = StrategyGetBuffer(strategy, &buf_state);
+
+		Assert(BUF_STATE_GET_REFCOUNT(buf_state) == 0);
+
+		/* Must copy buffer flags while we still hold the spinlock */
+		oldFlags = buf_state & BUF_FLAG_MASK;
+
+		/* Pin the buffer and then release the buffer spinlock */
+		PinBuffer_Locked(buf);
+
+		/*
+		 * If the buffer was dirty, try to write it out.  There is a race
+		 * condition here, in that someone might dirty it after we released it
+		 * above, or even while we are writing it out (since our share-lock
+		 * won't prevent hint-bit updates).  We will recheck the dirty bit
+		 * after re-locking the buffer header.
+		 */
+		if (oldFlags & BM_DIRTY)
+		{
+			/*
+			 * We need a share-lock on the buffer contents to write it out
+			 * (else we might write invalid data, eg because someone else is
+			 * compacting the page contents while we write).  We must use a
+			 * conditional lock acquisition here to avoid deadlock.  Even
+			 * though the buffer was not pinned (and therefore surely not
+			 * locked) when StrategyGetBuffer returned it, someone else could
+			 * have pinned and exclusive-locked it by the time we get here. If
+			 * we try to get the lock unconditionally, we'd block waiting for
+			 * them; if they later block waiting for us, deadlock ensues.
+			 * (This has been observed to happen when two backends are both
+			 * trying to split btree index pages, and the second one just
+			 * happens to be trying to split the page the first one got from
+			 * StrategyGetBuffer.)
+			 */
+			if (LWLockConditionalAcquire(BufferDescriptorGetContentLock(buf),
+										 LW_SHARED))
+			{
+				/*
+				 * If using a nondefault strategy, and writing the buffer
+				 * would require a WAL flush, let the strategy decide whether
+				 * to go ahead and write/reuse the buffer or to choose another
+				 * victim.  We need lock to inspect the page LSN, so this
+				 * can't be done inside StrategyGetBuffer.
+				 */
+				if (strategy != NULL)
+				{
+					XLogRecPtr	lsn;
+
+					/* Read the LSN while holding buffer header lock */
+					buf_state = LockBufHdr(buf);
+					lsn = BufferGetLSN(buf);
+					UnlockBufHdr(buf, buf_state);
+
+					if (XLogNeedsFlush(lsn) &&
+						StrategyRejectBuffer(strategy, buf))
+					{
+						/* Drop lock/pin and loop around for another buffer */
+						LWLockRelease(BufferDescriptorGetContentLock(buf));
+						UnpinBuffer(buf, true);
+						continue;
+					}
+				}
+
+				/* OK, do the I/O */
+				TRACE_POSTGRESQL_BUFFER_WRITE_DIRTY_START(forkNum, blockNum,
+														  smgr->smgr_rnode.node.spcNode,
+														  smgr->smgr_rnode.node.dbNode,
+														  smgr->smgr_rnode.node.relNode);
+
+				FlushBuffer(buf, NULL);
+				LWLockRelease(BufferDescriptorGetContentLock(buf));
+
+				ScheduleBufferTagForWriteback(&BackendWritebackContext,
+											  &buf->tag);
+
+				TRACE_POSTGRESQL_BUFFER_WRITE_DIRTY_DONE(forkNum, blockNum,
+														 smgr->smgr_rnode.node.spcNode,
+														 smgr->smgr_rnode.node.dbNode,
+														 smgr->smgr_rnode.node.relNode);
+			}
+			else
+			{
+				/*
+				 * Someone else has locked the buffer, so give it up and loop
+				 * back to get another one.
+				 */
+				UnpinBuffer(buf, true);
+				continue;
+			}
+		}
+
+		/*
+		 * To change the association of a valid buffer, we'll need to have
+		 * exclusive lock on both the old and new mapping partitions.
+		 */
+		if (oldFlags & BM_TAG_VALID)
+		{
+			/*
+			 * Need to compute the old tag's hashcode and partition lock ID.
+			 * XXX is it worth storing the hashcode in BufferDesc so we need
+			 * not recompute it here?  Probably not.
+			 */
+			oldTag = buf->tag;
+			oldHash = BufTableHashCode(&oldTag);
+			oldPartitionLock = BufMappingPartitionLock(oldHash);
+
+			if (*lastPartitionLock && 
+				(*lastPartitionLock != oldPartitionLock || 
+				 *lastPartitionLock != newPartitionLock))
+			{
+				LWLockRelease(*lastPartitionLock);
+				*lastPartitionLock = NULL;
+			}
+
+			/*
+			 * Must lock the lower-numbered partition first to avoid
+			 * deadlocks.
+			 */
+			if (oldPartitionLock < newPartitionLock)
+			{
+				Assert(*lastPartitionLock == NULL);
+				LWLockAcquire(oldPartitionLock, LW_EXCLUSIVE);
+				LWLockAcquire(newPartitionLock, LW_EXCLUSIVE);
+			}
+			else if (oldPartitionLock > newPartitionLock)
+			{
+				Assert(*lastPartitionLock == NULL);
+				LWLockAcquire(newPartitionLock, LW_EXCLUSIVE);
+				LWLockAcquire(oldPartitionLock, LW_EXCLUSIVE);
+			}
+			else
+			{
+				/* only one partition, only one lock */
+				if (*lastPartitionLock != newPartitionLock)
+					LWLockAcquire(newPartitionLock, LW_EXCLUSIVE);
+			}
+		}
+		else
+		{
+			/* if it wasn't valid, we need only the new partition */
+			if (*lastPartitionLock != newPartitionLock)
+			{
+				if (*lastPartitionLock)
+					LWLockRelease(*lastPartitionLock);
+				LWLockAcquire(newPartitionLock, LW_EXCLUSIVE);
+			}
+
+			/* remember we have no old-partition lock or tag */
+			oldPartitionLock = NULL;
+			/* keep the compiler quiet about uninitialized variables */
+			oldHash = 0;
+		}
+
+		*lastPartitionLock = newPartitionLock;
+
+		/*
+		 * Try to make a hashtable entry for the buffer under its new tag.
+		 * This could fail because while we were writing someone else
+		 * allocated another buffer for the same block we want to read in.
+		 * Note that we have not yet removed the hashtable entry for the old
+		 * tag.
+		 */
+		buf_id = BufTableInsert(&newTag, newHash, buf->buf_id);
+		Assert(buf_id < 0);
+
+		/*
+		 * Need to lock the buffer header too in order to change its tag.
+		 */
+		buf_state = LockBufHdr(buf);
+
+		/*
+		 * Somebody could have pinned or re-dirtied the buffer while we were
+		 * doing the I/O and making the new hashtable entry.  If so, we can't
+		 * recycle this buffer; we must undo everything we've done and start
+		 * over with a new victim buffer.
+		 */
+		oldFlags = buf_state & BUF_FLAG_MASK;
+		if (BUF_STATE_GET_REFCOUNT(buf_state) == 1 && !(oldFlags & BM_DIRTY))
+			break;
+
+		pg_unreachable();
+	}
+
+	/*
+	 * Okay, it's finally safe to rename the buffer.
+	 *
+	 * Clearing BM_VALID here is necessary, clearing the dirtybits is just
+	 * paranoia.  We also reset the usage_count since any recency of use of
+	 * the old content is no longer relevant.  (The usage_count starts out at
+	 * 1 so that the buffer can survive one clock-sweep pass.)
+	 *
+	 * Make sure BM_PERMANENT is set for buffers that must be written at every
+	 * checkpoint.  Unlogged buffers only need to be written at shutdown
+	 * checkpoints, except for their "init" forks, which need to be treated
+	 * just like permanent relations.
+	 */
+	buf->tag = newTag;
+	buf_state &= ~(BM_VALID | BM_DIRTY | BM_JUST_DIRTIED |
+				   BM_CHECKPOINT_NEEDED | BM_IO_ERROR | BM_PERMANENT |
+				   BUF_USAGECOUNT_MASK);
+	if (relpersistence == RELPERSISTENCE_PERMANENT || forkNum == INIT_FORKNUM)
+		buf_state |= BM_TAG_VALID | BM_PERMANENT | BUF_USAGECOUNT_ONE;
+	else
+		buf_state |= BM_TAG_VALID | BUF_USAGECOUNT_ONE;
+
+	UnlockBufHdr(buf, buf_state);
+
+	if (oldPartitionLock != NULL)
+	{
+		BufTableDelete(&oldTag, oldHash);
+		if (oldPartitionLock != newPartitionLock)
+			LWLockRelease(oldPartitionLock);
+	}
+
+	/*
+	 * Buffer contents are currently invalid.  Try to get the io_in_progress
+	 * lock.  If StartBufferIO returns false, then someone else managed to
+	 * read it before we did, so there's nothing left for BufferAlloc() to do.
+	 */
+	StartBufferIO(buf, true);
+
+	return buf;
+}
+
+
 /*
  * InvalidateBuffer -- mark a shared buffer invalid and return it to the
  * freelist.
diff --git a/src/include/storage/buf_internals.h b/src/include/storage/buf_internals.h
index 3377fa5676..277705f18c 100644
--- a/src/include/storage/buf_internals.h
+++ b/src/include/storage/buf_internals.h
@@ -124,7 +124,7 @@ typedef struct buftag
  * NB: NUM_BUFFER_PARTITIONS must be a power of 2!
  */
 #define BufTableHashPartition(hashcode) \
-	((hashcode) % NUM_BUFFER_PARTITIONS)
+	((hashcode >> 7) % NUM_BUFFER_PARTITIONS)
 #define BufMappingPartitionLock(hashcode) \
 	(&MainLWLockArray[BUFFER_MAPPING_LWLOCK_OFFSET + \
 		BufTableHashPartition(hashcode)].lock)
-- 
2.25.1


--------------065E078E0383D7C686F94091--





^ permalink  raw  reply  [nested|flat] 15+ messages in thread

* Re: Add PQsendSyncMessage() to libpq
@ 2023-11-12 13:37 Anton Kirilov <[email protected]>
  2023-11-13 08:19 ` Re: Add PQsendSyncMessage() to libpq Anthonin Bonnefoy <[email protected]>
  2023-12-29 11:49 ` Re: Add PQsendSyncMessage() to libpq Jelte Fennema-Nio <[email protected]>
  0 siblings, 2 replies; 15+ messages in thread

From: Anton Kirilov @ 2023-11-12 13:37 UTC (permalink / raw)
  To: Alvaro Herrera <[email protected]>; Jelte Fennema-Nio <[email protected]>; +Cc: Michael Paquier <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers

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 
needed a flag. Which makes sense, because they are the lowest level 
building blocks that make sense from a user perspective: They send a 
message type over the socket and don't do anything else.

Yes, I think that this is quite close to my thinking when I created the 
original version of the patch. Also, the protocol specification states 
that the Sync message lacks parameters.

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 of the patch.

Best wishes,
Anton Kirilov

Attachments:

  [text/x-patch] v5-0001-Add-PQsendPipelineSync-to-libpq.patch (9.8K, ../../[email protected]/2-v5-0001-Add-PQsendPipelineSync-to-libpq.patch)
  download | inline diff:
From b752269b2763f8d66bcfc79faf751e52226c344b Mon Sep 17 00:00:00 2001
From: Anton Kirilov <[email protected]>
Date: Wed, 22 Mar 2023 20:39:57 +0000
Subject: [PATCH v5] Add PQsendPipelineSync() to libpq

This new function is equivalent to PQpipelineSync(), except
that it does not flush anything to the server; the user must
subsequently call PQflush() instead. Its purpose is to reduce
the system call overhead of pipeline mode.
---
 doc/src/sgml/libpq.sgml                       | 45 ++++++++++++++++---
 src/interfaces/libpq/exports.txt              |  1 +
 src/interfaces/libpq/fe-exec.c                | 17 +++++--
 src/interfaces/libpq/libpq-fe.h               |  1 +
 .../modules/libpq_pipeline/libpq_pipeline.c   | 37 +++++++++++++++
 .../traces/multi_pipelines.trace              | 11 +++++
 6 files changed, 102 insertions(+), 10 deletions(-)

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 64b2910fee..61bee82a54 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -3547,8 +3547,9 @@ ExecStatusType PQresultStatus(const PGresult *res);
           <listitem>
            <para>
             The <structname>PGresult</structname> represents a
-            synchronization point in pipeline mode, requested by
-            <xref linkend="libpq-PQpipelineSync"/>.
+            synchronization point in pipeline mode, requested by either
+            <xref linkend="libpq-PQpipelineSync"/> or
+            <xref linkend="libpq-PQsendPipelineSync"/>.
             This status occurs only when pipeline mode has been selected.
            </para>
           </listitem>
@@ -5122,7 +5123,8 @@ int PQsendClosePortal(PGconn *conn, const char *portalName);
        <xref linkend="libpq-PQsendDescribePrepared"/>,
        <xref linkend="libpq-PQsendDescribePortal"/>,
        <xref linkend="libpq-PQsendClosePrepared"/>,
-       <xref linkend="libpq-PQsendClosePortal"/>, or
+       <xref linkend="libpq-PQsendClosePortal"/>,
+       <xref linkend="libpq-PQsendPipelineSync"/>, or
        <xref linkend="libpq-PQpipelineSync"/>
        call, and returns it.
        A null pointer is returned when the command is complete and there
@@ -5506,8 +5508,9 @@ int PQflush(PGconn *conn);
      client sends them.  The server will begin executing the commands in the
      pipeline immediately, not waiting for the end of the pipeline.
      Note that results are buffered on the server side; the server flushes
-     that buffer when a synchronization point is established with
-     <function>PQpipelineSync</function>, or when
+     that buffer when a synchronization point is established with either
+     <function>PQpipelineSync</function> or
+     <function>PQsendPipelineSync</function>, or when
      <function>PQsendFlushRequest</function> is called.
      If any statement encounters an error, the server aborts the current
      transaction and does not execute any subsequent command in the queue
@@ -5564,7 +5567,8 @@ int PQflush(PGconn *conn);
      <type>PGresult</type> types <literal>PGRES_PIPELINE_SYNC</literal>
      and <literal>PGRES_PIPELINE_ABORTED</literal>.
      <literal>PGRES_PIPELINE_SYNC</literal> is reported exactly once for each
-     <function>PQpipelineSync</function> at the corresponding point
+     <function>PQpipelineSync</function> or
+     <function>PQsendPipelineSync</function> at the corresponding point
      in the pipeline.
      <literal>PGRES_PIPELINE_ABORTED</literal> is emitted in place of a normal
      query result for the first error and all subsequent results
@@ -5602,7 +5606,8 @@ int PQflush(PGconn *conn);
      <function>PQresultStatus</function> will report a
      <literal>PGRES_PIPELINE_ABORTED</literal> result for each remaining queued
      operation in an aborted pipeline. The result for
-     <function>PQpipelineSync</function> is reported as
+     <function>PQpipelineSync</function> or
+     <function>PQsendPipelineSync</function> is reported as
      <literal>PGRES_PIPELINE_SYNC</literal> to signal the end of the aborted pipeline
      and resumption of normal result processing.
     </para>
@@ -5834,6 +5839,32 @@ int PQsendFlushRequest(PGconn *conn);
        </para>
       </listitem>
      </varlistentry>
+
+    <varlistentry id="libpq-PQsendPipelineSync">
+     <term><function>PQsendPipelineSync</function><indexterm><primary>PQsendPipelineSync</primary></indexterm></term>
+
+     <listitem>
+      <para>
+       Marks a synchronization point in a pipeline by sending a
+       <link linkend="protocol-flow-ext-query">sync message</link>
+       without flushing the send buffer. This serves as
+       the delimiter of an implicit transaction and an error recovery
+       point; see <xref linkend="libpq-pipeline-errors"/>.
+
+<synopsis>
+int PQsendPipelineSync(PGconn *conn);
+</synopsis>
+      </para>
+      <para>
+       Returns 1 for success. Returns 0 if the connection is not in
+       pipeline mode or sending a
+       <link linkend="protocol-flow-ext-query">sync message</link>
+       failed.
+       Note that the message is not itself flushed to the server automatically;
+       use <function>PQflush</function> if necessary.
+      </para>
+     </listitem>
+    </varlistentry>
    </variablelist>
   </sect2>
 
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index 850734ac96..f5aa641001 100644
--- a/src/interfaces/libpq/exports.txt
+++ b/src/interfaces/libpq/exports.txt
@@ -191,3 +191,4 @@ PQclosePrepared           188
 PQclosePortal             189
 PQsendClosePrepared       190
 PQsendClosePortal         191
+PQsendPipelineSync        192
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 04610ccf5e..c0dcb04282 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -3224,6 +3224,16 @@ pqPipelineProcessQueue(PGconn *conn)
  */
 int
 PQpipelineSync(PGconn *conn)
+{
+	return PQsendPipelineSync(conn) && pqFlush(conn) >= 0;
+}
+
+/*
+ * PQsendPipelineSync
+ *		Send a Sync message as part of a pipeline without flushing to server
+ */
+int
+PQsendPipelineSync(PGconn *conn)
 {
 	PGcmdQueueEntry *entry;
 
@@ -3267,10 +3277,11 @@ PQpipelineSync(PGconn *conn)
 		goto sendFailed;
 
 	/*
-	 * Give the data a push.  In nonblock mode, don't complain if we're unable
-	 * to send it all; PQgetResult() will do any additional flushing needed.
+	 * 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;
 
 	/* OK, it's launched! */
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index 97762d56f5..b0d89a592d 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -474,6 +474,7 @@ extern int	PQenterPipelineMode(PGconn *conn);
 extern int	PQexitPipelineMode(PGconn *conn);
 extern int	PQpipelineSync(PGconn *conn);
 extern int	PQsendFlushRequest(PGconn *conn);
+extern int	PQsendPipelineSync(PGconn *conn);
 
 /* LISTEN/NOTIFY support */
 extern PGnotify *PQnotifies(PGconn *conn);
diff --git a/src/test/modules/libpq_pipeline/libpq_pipeline.c b/src/test/modules/libpq_pipeline/libpq_pipeline.c
index 3c009ee153..23fb80986b 100644
--- a/src/test/modules/libpq_pipeline/libpq_pipeline.c
+++ b/src/test/modules/libpq_pipeline/libpq_pipeline.c
@@ -169,6 +169,14 @@ test_multi_pipelines(PGconn *conn)
 	if (PQpipelineSync(conn) != 1)
 		pg_fatal("Pipeline sync failed: %s", PQerrorMessage(conn));
 
+	if (PQsendQueryParams(conn, "SELECT $1", 1, dummy_param_oids,
+						  dummy_params, NULL, NULL, 0) != 1)
+		pg_fatal("dispatching first SELECT failed: %s", PQerrorMessage(conn));
+
+	/* Skip flushing once. */
+	if (PQsendPipelineSync(conn) != 1)
+		pg_fatal("Pipeline sync failed: %s", PQerrorMessage(conn));
+
 	if (PQsendQueryParams(conn, "SELECT $1", 1, dummy_param_oids,
 						  dummy_params, NULL, NULL, 0) != 1)
 		pg_fatal("dispatching second SELECT failed: %s", PQerrorMessage(conn));
@@ -206,6 +214,35 @@ test_multi_pipelines(PGconn *conn)
 
 	/* second pipeline */
 
+	res = PQgetResult(conn);
+	if (res == NULL)
+		pg_fatal("PQgetResult returned null when there's a pipeline item: %s",
+				 PQerrorMessage(conn));
+
+	if (PQresultStatus(res) != PGRES_TUPLES_OK)
+		pg_fatal("Unexpected result code %s from first pipeline item",
+				 PQresStatus(PQresultStatus(res)));
+	PQclear(res);
+	res = NULL;
+
+	if (PQgetResult(conn) != NULL)
+		pg_fatal("PQgetResult returned something extra after first result");
+
+	if (PQexitPipelineMode(conn) != 0)
+		pg_fatal("exiting pipeline mode after query but before sync succeeded incorrectly");
+
+	res = PQgetResult(conn);
+	if (res == NULL)
+		pg_fatal("PQgetResult returned null when sync result expected: %s",
+				 PQerrorMessage(conn));
+
+	if (PQresultStatus(res) != PGRES_PIPELINE_SYNC)
+		pg_fatal("Unexpected result code %s instead of sync result, error: %s",
+				 PQresStatus(PQresultStatus(res)), PQerrorMessage(conn));
+	PQclear(res);
+
+	/* third pipeline */
+
 	res = PQgetResult(conn);
 	if (res == NULL)
 		pg_fatal("PQgetResult returned null when there's a pipeline item: %s",
diff --git a/src/test/modules/libpq_pipeline/traces/multi_pipelines.trace b/src/test/modules/libpq_pipeline/traces/multi_pipelines.trace
index 4b9ab07ca4..1ee21f61dc 100644
--- a/src/test/modules/libpq_pipeline/traces/multi_pipelines.trace
+++ b/src/test/modules/libpq_pipeline/traces/multi_pipelines.trace
@@ -8,6 +8,17 @@ F	19	Bind	 "" "" 0 1 1 '1' 1 0
 F	6	Describe	 P ""
 F	9	Execute	 "" 0
 F	4	Sync
+F	21	Parse	 "" "SELECT $1" 1 NNNN
+F	19	Bind	 "" "" 0 1 1 '1' 1 0
+F	6	Describe	 P ""
+F	9	Execute	 "" 0
+F	4	Sync
+B	4	ParseComplete
+B	4	BindComplete
+B	33	RowDescription	 1 "?column?" NNNN 0 NNNN 4 -1 0
+B	11	DataRow	 1 1 '1'
+B	13	CommandComplete	 "SELECT 1"
+B	5	ReadyForQuery	 I
 B	4	ParseComplete
 B	4	BindComplete
 B	33	RowDescription	 1 "?column?" NNNN 0 NNNN 4 -1 0
-- 
2.34.1



^ permalink  raw  reply  [nested|flat] 15+ messages in thread

* Re: Add PQsendSyncMessage() to libpq
  2023-11-12 13:37 Re: Add PQsendSyncMessage() to libpq Anton Kirilov <[email protected]>
@ 2023-11-13 08:19 ` Anthonin Bonnefoy <[email protected]>
  2023-12-29 11:52   ` Re: Add PQsendSyncMessage() to libpq Jelte Fennema-Nio <[email protected]>
  1 sibling, 1 reply; 15+ messages in thread

From: Anthonin Bonnefoy @ 2023-11-13 08:19 UTC (permalink / raw)
  To: Anton Kirilov <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Jelte Fennema-Nio <[email protected]>; Michael Paquier <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers

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 pgbench (don't want to mess with the
thread's attachment so here's a GH link
https://github.com/bonnefoa/postgres/commit/047b5b05169e36361fe29fef9f430da045ef012d).
Here's some quick results:

echo "\set aid1 random(1, 100000 * :scale)
\set aid2 random(1, 100000 * :scale)
\startpipeline
select 1;
select * from pgbench_accounts where aid=:aid1;
select 2;
\syncpipeline
select 1;
select * from pgbench_accounts where aid=:aid2;
select 2;
\endpipeline" > /tmp/pipeline_without_flush.sql
pgbench -T30 -Mextended -f /tmp/pipeline_without_flush.sql -h127.0.0.1
latency average = 0.383 ms
initial connection time = 2.810 ms
tps = 2607.587877 (without initial connection time)

echo "\set aid1 random(1, 100000 * :scale)
\set aid2 random(1, 100000 * :scale)
\startpipeline
select 1;
select * from pgbench_accounts where aid=:aid1;
select 2;
\endpipeline
\startpipeline
select 1;
select * from pgbench_accounts where aid=:aid2;
select 2;
\endpipeline" > /tmp/pipeline_with_flush.sql
pgbench -T30 -Mextended -f /tmp/pipeline_with_flush.sql -h127.0.0.1
latency average = 0.437 ms
initial connection time = 2.602 ms
tps = 2290.527462 (without initial connection time)

I took some perfs and the main change is from the server spending less time in
ReadCommand which makes sense since the commands are sent in a single tcp
frame with the \syncpipeline version.

Regards,
Anthonin

On Sun, Nov 12, 2023 at 2:37 PM Anton Kirilov <[email protected]> wrote:
>
> 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
> needed a flag. Which makes sense, because they are the lowest level
> building blocks that make sense from a user perspective: They send a
> message type over the socket and don't do anything else.
>
> Yes, I think that this is quite close to my thinking when I created the
> original version of the patch. Also, the protocol specification states
> that the Sync message lacks parameters.
>
> 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 of the patch.
>
> Best wishes,
> Anton Kirilov






^ permalink  raw  reply  [nested|flat] 15+ messages in thread

* Re: Add PQsendSyncMessage() to libpq
  2023-11-12 13:37 Re: Add PQsendSyncMessage() to libpq Anton Kirilov <[email protected]>
  2023-11-13 08:19 ` Re: Add PQsendSyncMessage() to libpq Anthonin Bonnefoy <[email protected]>
@ 2023-12-29 11:52   ` Jelte Fennema-Nio <[email protected]>
  2023-12-31 00:37     ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  0 siblings, 1 reply; 15+ messages in thread

From: Jelte Fennema-Nio @ 2023-12-29 11:52 UTC (permalink / raw)
  To: Anthonin Bonnefoy <[email protected]>; +Cc: Anton Kirilov <[email protected]>; Alvaro Herrera <[email protected]>; Michael Paquier <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers

On Mon, 13 Nov 2023 at 09:20, Anthonin Bonnefoy
<[email protected]> 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 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.






^ permalink  raw  reply  [nested|flat] 15+ messages in thread

* Re: Add PQsendSyncMessage() to libpq
  2023-11-12 13:37 Re: Add PQsendSyncMessage() to libpq Anton Kirilov <[email protected]>
  2023-11-13 08:19 ` Re: Add PQsendSyncMessage() to libpq Anthonin Bonnefoy <[email protected]>
  2023-12-29 11:52   ` Re: Add PQsendSyncMessage() to libpq Jelte Fennema-Nio <[email protected]>
@ 2023-12-31 00:37     ` Michael Paquier <[email protected]>
  2024-01-10 06:40       ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  0 siblings, 1 reply; 15+ messages in thread

From: Michael Paquier @ 2023-12-31 00:37 UTC (permalink / raw)
  To: Jelte Fennema-Nio <[email protected]>; +Cc: Anthonin Bonnefoy <[email protected]>; Anton Kirilov <[email protected]>; Alvaro Herrera <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers

On Fri, Dec 29, 2023 at 12:52:30PM +0100, Jelte Fennema-Nio wrote:
> On Mon, 13 Nov 2023 at 09:20, Anthonin Bonnefoy
> <[email protected]> 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 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.)
--
Michael


Attachments:

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

^ permalink  raw  reply  [nested|flat] 15+ messages in thread

* Re: Add PQsendSyncMessage() to libpq
  2023-11-12 13:37 Re: Add PQsendSyncMessage() to libpq Anton Kirilov <[email protected]>
  2023-11-13 08:19 ` Re: Add PQsendSyncMessage() to libpq Anthonin Bonnefoy <[email protected]>
  2023-12-29 11:52   ` Re: Add PQsendSyncMessage() to libpq Jelte Fennema-Nio <[email protected]>
  2023-12-31 00:37     ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
@ 2024-01-10 06:40       ` Michael Paquier <[email protected]>
  2024-01-15 07:50         ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  0 siblings, 1 reply; 15+ messages in thread

From: Michael Paquier @ 2024-01-10 06:40 UTC (permalink / raw)
  To: Jelte Fennema-Nio <[email protected]>; +Cc: Anthonin Bonnefoy <[email protected]>; Anton Kirilov <[email protected]>; Alvaro Herrera <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers

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

^ permalink  raw  reply  [nested|flat] 15+ messages in thread

* Re: Add PQsendSyncMessage() to libpq
  2023-11-12 13:37 Re: Add PQsendSyncMessage() to libpq Anton Kirilov <[email protected]>
  2023-11-13 08:19 ` Re: Add PQsendSyncMessage() to libpq Anthonin Bonnefoy <[email protected]>
  2023-12-29 11:52   ` Re: Add PQsendSyncMessage() to libpq Jelte Fennema-Nio <[email protected]>
  2023-12-31 00:37     ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  2024-01-10 06:40       ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
@ 2024-01-15 07:50         ` Michael Paquier <[email protected]>
  2024-01-15 09:01           ` Re: Add PQsendSyncMessage() to libpq Jelte Fennema-Nio <[email protected]>
  2024-01-15 09:49           ` Re: Add PQsendSyncMessage() to libpq Alvaro Herrera <[email protected]>
  0 siblings, 2 replies; 15+ messages in thread

From: Michael Paquier @ 2024-01-15 07:50 UTC (permalink / raw)
  To: Jelte Fennema-Nio <[email protected]>; +Cc: Anthonin Bonnefoy <[email protected]>; Anton Kirilov <[email protected]>; Alvaro Herrera <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers

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

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.

I have also made a few edits to the tests.
--
Michael


Attachments:

  [text/x-diff] v6-0001-Add-PQsendPipelineSync-to-libpq.patch (11.7K, ../../[email protected]/2-v6-0001-Add-PQsendPipelineSync-to-libpq.patch)
  download | inline diff:
From b653759f8dfdfe83d0d8bc1d4a0ac9d4e272a061 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Mon, 15 Jan 2024 16:15:00 +0900
Subject: [PATCH v6] Add PQsendPipelineSync() to libpq

This new function is equivalent to PQpipelineSync(), except
that it does not flush anything to the server; the user must
subsequently call PQflush() instead. Its purpose is to reduce
the system call overhead of pipeline mode.
---
 src/interfaces/libpq/exports.txt              |  1 +
 src/interfaces/libpq/fe-exec.c                | 41 +++++++++++++++--
 src/interfaces/libpq/libpq-fe.h               |  1 +
 .../modules/libpq_pipeline/libpq_pipeline.c   | 43 ++++++++++++++++++
 .../traces/multi_pipelines.trace              | 11 +++++
 doc/src/sgml/libpq.sgml                       | 45 ++++++++++++++++---
 6 files changed, 131 insertions(+), 11 deletions(-)

diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index 28b861fd93..088592deb1 100644
--- a/src/interfaces/libpq/exports.txt
+++ b/src/interfaces/libpq/exports.txt
@@ -192,3 +192,4 @@ PQclosePortal             189
 PQsendClosePrepared       190
 PQsendClosePortal         191
 PQchangePassword          192
+PQsendPipelineSync        193
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 106d14e6ee..9e7e670921 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -81,6 +81,7 @@ static int	PQsendTypedCommand(PGconn *conn, char command, char type,
 							   const char *target);
 static int	check_field_number(const PGresult *res, int field_num);
 static void pqPipelineProcessQueue(PGconn *conn);
+static int	pqPipelineSyncInternal(PGconn *conn, bool immediate_flush);
 static int	pqPipelineFlush(PGconn *conn);
 
 
@@ -3224,6 +3225,25 @@ pqPipelineProcessQueue(PGconn *conn)
 /*
  * PQpipelineSync
  *		Send a Sync message as part of a pipeline, and flush to server
+ */
+int
+PQpipelineSync(PGconn *conn)
+{
+	return pqPipelineSyncInternal(conn, true);
+}
+
+/*
+ * PQsendPipelineSync
+ *		Send a Sync message as part of a pipeline, without flushing to server
+ */
+int
+PQsendPipelineSync(PGconn *conn)
+{
+	return pqPipelineSyncInternal(conn, false);
+}
+
+/*
+ * 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 no need to
@@ -3240,9 +3260,12 @@ pqPipelineProcessQueue(PGconn *conn)
  * The connection will remain in pipeline mode and unavailable for new
  * synchronous command execution functions until all results from the pipeline
  * are processed by the client.
+ *
+ * immediate_flush controls if the flush happens immediately after sending the
+ * Sync message or not.
  */
-int
-PQpipelineSync(PGconn *conn)
+static int
+pqPipelineSyncInternal(PGconn *conn, bool immediate_flush)
 {
 	PGcmdQueueEntry *entry;
 
@@ -3288,9 +3311,19 @@ PQpipelineSync(PGconn *conn)
 	/*
 	 * Give the data a push.  In nonblock mode, don't complain if we're unable
 	 * to send it all; PQgetResult() will do any additional flushing needed.
+	 * If immediate_flush is disabled, the data is pushed if we are past the
+	 * size threshold.
 	 */
-	if (PQflush(conn) < 0)
-		goto sendFailed;
+	if (immediate_flush)
+	{
+		if (pqFlush(conn) < 0)
+			goto sendFailed;
+	}
+	else
+	{
+		if (pqPipelineFlush(conn) < 0)
+			goto sendFailed;
+	}
 
 	/* OK, it's launched! */
 	pqAppendCmdQueueEntry(conn, entry);
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index f0ec660cb6..defc415fa3 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -474,6 +474,7 @@ extern int	PQenterPipelineMode(PGconn *conn);
 extern int	PQexitPipelineMode(PGconn *conn);
 extern int	PQpipelineSync(PGconn *conn);
 extern int	PQsendFlushRequest(PGconn *conn);
+extern int	PQsendPipelineSync(PGconn *conn);
 
 /* LISTEN/NOTIFY support */
 extern PGnotify *PQnotifies(PGconn *conn);
diff --git a/src/test/modules/libpq_pipeline/libpq_pipeline.c b/src/test/modules/libpq_pipeline/libpq_pipeline.c
index 71cd04c5f2..c68e20d0b5 100644
--- a/src/test/modules/libpq_pipeline/libpq_pipeline.c
+++ b/src/test/modules/libpq_pipeline/libpq_pipeline.c
@@ -162,6 +162,7 @@ test_multi_pipelines(PGconn *conn)
 	if (PQenterPipelineMode(conn) != 1)
 		pg_fatal("failed to enter pipeline mode: %s", PQerrorMessage(conn));
 
+	/* first pipeline */
 	if (PQsendQueryParams(conn, "SELECT $1", 1, dummy_param_oids,
 						  dummy_params, NULL, NULL, 0) != 1)
 		pg_fatal("dispatching first SELECT failed: %s", PQerrorMessage(conn));
@@ -169,6 +170,16 @@ test_multi_pipelines(PGconn *conn)
 	if (PQpipelineSync(conn) != 1)
 		pg_fatal("Pipeline sync failed: %s", PQerrorMessage(conn));
 
+	/* second pipeline */
+	if (PQsendQueryParams(conn, "SELECT $1", 1, dummy_param_oids,
+						  dummy_params, NULL, NULL, 0) != 1)
+		pg_fatal("dispatching first SELECT failed: %s", PQerrorMessage(conn));
+
+	/* Skip flushing once. */
+	if (PQsendPipelineSync(conn) != 1)
+		pg_fatal("Pipeline sync failed: %s", PQerrorMessage(conn));
+
+	/* third pipeline */
 	if (PQsendQueryParams(conn, "SELECT $1", 1, dummy_param_oids,
 						  dummy_params, NULL, NULL, 0) != 1)
 		pg_fatal("dispatching second SELECT failed: %s", PQerrorMessage(conn));
@@ -177,6 +188,9 @@ test_multi_pipelines(PGconn *conn)
 		pg_fatal("pipeline sync failed: %s", PQerrorMessage(conn));
 
 	/* OK, start processing the results */
+
+	/* first pipeline */
+
 	res = PQgetResult(conn);
 	if (res == NULL)
 		pg_fatal("PQgetResult returned null when there's a pipeline item: %s",
@@ -206,6 +220,35 @@ test_multi_pipelines(PGconn *conn)
 
 	/* second pipeline */
 
+	res = PQgetResult(conn);
+	if (res == NULL)
+		pg_fatal("PQgetResult returned null when there's a pipeline item: %s",
+				 PQerrorMessage(conn));
+
+	if (PQresultStatus(res) != PGRES_TUPLES_OK)
+		pg_fatal("Unexpected result code %s from first pipeline item",
+				 PQresStatus(PQresultStatus(res)));
+	PQclear(res);
+	res = NULL;
+
+	if (PQgetResult(conn) != NULL)
+		pg_fatal("PQgetResult returned something extra after first result");
+
+	if (PQexitPipelineMode(conn) != 0)
+		pg_fatal("exiting pipeline mode after query but before sync succeeded incorrectly");
+
+	res = PQgetResult(conn);
+	if (res == NULL)
+		pg_fatal("PQgetResult returned null when sync result expected: %s",
+				 PQerrorMessage(conn));
+
+	if (PQresultStatus(res) != PGRES_PIPELINE_SYNC)
+		pg_fatal("Unexpected result code %s instead of sync result, error: %s",
+				 PQresStatus(PQresultStatus(res)), PQerrorMessage(conn));
+	PQclear(res);
+
+	/* third pipeline */
+
 	res = PQgetResult(conn);
 	if (res == NULL)
 		pg_fatal("PQgetResult returned null when there's a pipeline item: %s",
diff --git a/src/test/modules/libpq_pipeline/traces/multi_pipelines.trace b/src/test/modules/libpq_pipeline/traces/multi_pipelines.trace
index 4b9ab07ca4..1ee21f61dc 100644
--- a/src/test/modules/libpq_pipeline/traces/multi_pipelines.trace
+++ b/src/test/modules/libpq_pipeline/traces/multi_pipelines.trace
@@ -8,6 +8,17 @@ F	19	Bind	 "" "" 0 1 1 '1' 1 0
 F	6	Describe	 P ""
 F	9	Execute	 "" 0
 F	4	Sync
+F	21	Parse	 "" "SELECT $1" 1 NNNN
+F	19	Bind	 "" "" 0 1 1 '1' 1 0
+F	6	Describe	 P ""
+F	9	Execute	 "" 0
+F	4	Sync
+B	4	ParseComplete
+B	4	BindComplete
+B	33	RowDescription	 1 "?column?" NNNN 0 NNNN 4 -1 0
+B	11	DataRow	 1 1 '1'
+B	13	CommandComplete	 "SELECT 1"
+B	5	ReadyForQuery	 I
 B	4	ParseComplete
 B	4	BindComplete
 B	33	RowDescription	 1 "?column?" NNNN 0 NNNN 4 -1 0
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 21195e0e72..6d1d477276 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -3547,8 +3547,9 @@ ExecStatusType PQresultStatus(const PGresult *res);
           <listitem>
            <para>
             The <structname>PGresult</structname> represents a
-            synchronization point in pipeline mode, requested by
-            <xref linkend="libpq-PQpipelineSync"/>.
+            synchronization point in pipeline mode, requested by either
+            <xref linkend="libpq-PQpipelineSync"/> or
+            <xref linkend="libpq-PQsendPipelineSync"/>.
             This status occurs only when pipeline mode has been selected.
            </para>
           </listitem>
@@ -5122,7 +5123,8 @@ int PQsendClosePortal(PGconn *conn, const char *portalName);
        <xref linkend="libpq-PQsendDescribePrepared"/>,
        <xref linkend="libpq-PQsendDescribePortal"/>,
        <xref linkend="libpq-PQsendClosePrepared"/>,
-       <xref linkend="libpq-PQsendClosePortal"/>, or
+       <xref linkend="libpq-PQsendClosePortal"/>,
+       <xref linkend="libpq-PQsendPipelineSync"/>, or
        <xref linkend="libpq-PQpipelineSync"/>
        call, and returns it.
        A null pointer is returned when the command is complete and there
@@ -5507,8 +5509,9 @@ int PQflush(PGconn *conn);
      client sends them.  The server will begin executing the commands in the
      pipeline immediately, not waiting for the end of the pipeline.
      Note that results are buffered on the server side; the server flushes
-     that buffer when a synchronization point is established with
-     <function>PQpipelineSync</function>, or when
+     that buffer when a synchronization point is established with either
+     <function>PQpipelineSync</function> or
+     <function>PQsendPipelineSync</function>, or when
      <function>PQsendFlushRequest</function> is called.
      If any statement encounters an error, the server aborts the current
      transaction and does not execute any subsequent command in the queue
@@ -5565,7 +5568,8 @@ int PQflush(PGconn *conn);
      <type>PGresult</type> types <literal>PGRES_PIPELINE_SYNC</literal>
      and <literal>PGRES_PIPELINE_ABORTED</literal>.
      <literal>PGRES_PIPELINE_SYNC</literal> is reported exactly once for each
-     <function>PQpipelineSync</function> at the corresponding point
+     <function>PQpipelineSync</function> or
+     <function>PQsendPipelineSync</function> at the corresponding point
      in the pipeline.
      <literal>PGRES_PIPELINE_ABORTED</literal> is emitted in place of a normal
      query result for the first error and all subsequent results
@@ -5603,7 +5607,8 @@ int PQflush(PGconn *conn);
      <function>PQresultStatus</function> will report a
      <literal>PGRES_PIPELINE_ABORTED</literal> result for each remaining queued
      operation in an aborted pipeline. The result for
-     <function>PQpipelineSync</function> is reported as
+     <function>PQpipelineSync</function> or
+     <function>PQsendPipelineSync</function> is reported as
      <literal>PGRES_PIPELINE_SYNC</literal> to signal the end of the aborted pipeline
      and resumption of normal result processing.
     </para>
@@ -5835,6 +5840,32 @@ int PQsendFlushRequest(PGconn *conn);
        </para>
       </listitem>
      </varlistentry>
+
+    <varlistentry id="libpq-PQsendPipelineSync">
+     <term><function>PQsendPipelineSync</function><indexterm><primary>PQsendPipelineSync</primary></indexterm></term>
+
+     <listitem>
+      <para>
+       Marks a synchronization point in a pipeline by sending a
+       <link linkend="protocol-flow-ext-query">sync message</link>
+       without flushing the send buffer. This serves as
+       the delimiter of an implicit transaction and an error recovery
+       point; see <xref linkend="libpq-pipeline-errors"/>.
+
+<synopsis>
+int PQsendPipelineSync(PGconn *conn);
+</synopsis>
+      </para>
+      <para>
+       Returns 1 for success. Returns 0 if the connection is not in
+       pipeline mode or sending a
+       <link linkend="protocol-flow-ext-query">sync message</link>
+       failed.
+       Note that the message is not itself flushed to the server automatically;
+       use <function>PQflush</function> if necessary.
+      </para>
+     </listitem>
+    </varlistentry>
    </variablelist>
   </sect2>
 
-- 
2.43.0



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

^ permalink  raw  reply  [nested|flat] 15+ messages in thread

* Re: Add PQsendSyncMessage() to libpq
  2023-11-12 13:37 Re: Add PQsendSyncMessage() to libpq Anton Kirilov <[email protected]>
  2023-11-13 08:19 ` Re: Add PQsendSyncMessage() to libpq Anthonin Bonnefoy <[email protected]>
  2023-12-29 11:52   ` Re: Add PQsendSyncMessage() to libpq Jelte Fennema-Nio <[email protected]>
  2023-12-31 00:37     ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  2024-01-10 06:40       ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  2024-01-15 07:50         ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
@ 2024-01-15 09:01           ` Jelte Fennema-Nio <[email protected]>
  2024-01-15 23:28             ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  1 sibling, 1 reply; 15+ messages in thread

From: Jelte Fennema-Nio @ 2024-01-15 09:01 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: Anthonin Bonnefoy <[email protected]>; Anton Kirilov <[email protected]>; Alvaro Herrera <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers

On Mon, 15 Jan 2024 at 08:50, Michael Paquier <[email protected]> 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 on the test.

+     /* second pipeline */
+     if (PQsendQueryParams(conn, "SELECT $1", 1, dummy_param_oids,
+                                     dummy_params, NULL, NULL, 0) != 1)
+           pg_fatal("dispatching first SELECT failed: %s",
PQerrorMessage(conn));

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


+     res = PQgetResult(conn);
+     if (res == NULL)
+           pg_fatal("PQgetResult returned null when there's a
pipeline item: %s",
+                        PQerrorMessage(conn));
+
+     if (PQresultStatus(res) != PGRES_TUPLES_OK)
+           pg_fatal("Unexpected result code %s from first pipeline item",
+                        PQresStatus(PQresultStatus(res)));
+     PQclear(res);
+     res = NULL;
+
+     if (PQgetResult(conn) != NULL)
+           pg_fatal("PQgetResult returned something extra after first result");

same issue: s/first/second/g (and s/second/third/g for the existing
part of the test).






^ permalink  raw  reply  [nested|flat] 15+ messages in thread

* Re: Add PQsendSyncMessage() to libpq
  2023-11-12 13:37 Re: Add PQsendSyncMessage() to libpq Anton Kirilov <[email protected]>
  2023-11-13 08:19 ` Re: Add PQsendSyncMessage() to libpq Anthonin Bonnefoy <[email protected]>
  2023-12-29 11:52   ` Re: Add PQsendSyncMessage() to libpq Jelte Fennema-Nio <[email protected]>
  2023-12-31 00:37     ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  2024-01-10 06:40       ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  2024-01-15 07:50         ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  2024-01-15 09:01           ` Re: Add PQsendSyncMessage() to libpq Jelte Fennema-Nio <[email protected]>
@ 2024-01-15 23:28             ` Michael Paquier <[email protected]>
  0 siblings, 0 replies; 15+ messages in thread

From: Michael Paquier @ 2024-01-15 23:28 UTC (permalink / raw)
  To: Jelte Fennema-Nio <[email protected]>; +Cc: Anthonin Bonnefoy <[email protected]>; Anton Kirilov <[email protected]>; Alvaro Herrera <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers

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
> part of the test).

Ugh, yes.  The note in the test was wrong.  Thanks for
double-checking.
--
Michael


Attachments:

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

^ permalink  raw  reply  [nested|flat] 15+ messages in thread

* Re: Add PQsendSyncMessage() to libpq
  2023-11-12 13:37 Re: Add PQsendSyncMessage() to libpq Anton Kirilov <[email protected]>
  2023-11-13 08:19 ` Re: Add PQsendSyncMessage() to libpq Anthonin Bonnefoy <[email protected]>
  2023-12-29 11:52   ` Re: Add PQsendSyncMessage() to libpq Jelte Fennema-Nio <[email protected]>
  2023-12-31 00:37     ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  2024-01-10 06:40       ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  2024-01-15 07:50         ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
@ 2024-01-15 09:49           ` Alvaro Herrera <[email protected]>
  2024-01-16 03:32             ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  1 sibling, 1 reply; 15+ messages in thread

From: Alvaro Herrera @ 2024-01-15 09:49 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: Jelte Fennema-Nio <[email protected]>; Anthonin Bonnefoy <[email protected]>; Anton Kirilov <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers

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 no need to

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.

In the docs, after this patch we have

- PQpipelineSync
- PQsendFlushRequest
- PQsendPipelineSync

Wouldn't it make more sense to add the new function in the middle of the
two existing ones instead?


Looking again at the largish comment that's now atop
pqPipelineSyncInternal(), I think most of it should be removed -- these
things should be explained in the SGML docs, and I think they are, in
the "Using Pipeline Mode" section.  We can just have the lines this
patch is adding.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"I can't go to a restaurant and order food because I keep looking at the
fonts on the menu.  Five minutes later I realize that it's also talking
about food" (Donald Knuth)






^ permalink  raw  reply  [nested|flat] 15+ messages in thread

* Re: Add PQsendSyncMessage() to libpq
  2023-11-12 13:37 Re: Add PQsendSyncMessage() to libpq Anton Kirilov <[email protected]>
  2023-11-13 08:19 ` Re: Add PQsendSyncMessage() to libpq Anthonin Bonnefoy <[email protected]>
  2023-12-29 11:52   ` Re: Add PQsendSyncMessage() to libpq Jelte Fennema-Nio <[email protected]>
  2023-12-31 00:37     ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  2024-01-10 06:40       ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  2024-01-15 07:50         ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  2024-01-15 09:49           ` Re: Add PQsendSyncMessage() to libpq Alvaro Herrera <[email protected]>
@ 2024-01-16 03:32             ` Michael Paquier <[email protected]>
  2024-01-16 13:55               ` Re: Add PQsendSyncMessage() to libpq Alvaro Herrera <[email protected]>
  0 siblings, 1 reply; 15+ messages in thread

From: Michael Paquier @ 2024-01-16 03:32 UTC (permalink / raw)
  To: Alvaro Herrera <[email protected]>; +Cc: Jelte Fennema-Nio <[email protected]>; Anthonin Bonnefoy <[email protected]>; Anton Kirilov <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers

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.

> In the docs, after this patch we have
> 
> - PQpipelineSync
> - PQsendFlushRequest
> - PQsendPipelineSync
> 
> Wouldn't it make more sense to add the new function in the middle of the
> two existing ones instead?

Ordering PQsendPipelineSync just after PQpipelineSync is OK by me.
I've applied the patch with all these modifications to move on with
the subject.

> Looking again at the largish comment that's now atop
> pqPipelineSyncInternal(), I think most of it should be removed -- these
> things should be explained in the SGML docs, and I think they are, in
> the "Using Pipeline Mode" section.  We can just have the lines this
> patch is adding.

Hmm.  The first two sentences about being able to submit more commands
to the pipeline are documented in the subsection "Issuing Queries".
The third sentence is implied in the second paragraph of this
subsection.  The 4th paragraph of the comment where sync commands
cannot be issued until all the results from the pipeline have been
consumed is mentioned in the first paragraph in "Using Pipeline Mode".
So you are right that this could be entirely removed.

How about the attached to remove all that, then?
--
Michael


Attachments:

  [text/x-diff] libpq-exec-comments.patch (1.3K, ../../[email protected]/2-libpq-exec-comments.patch)
  download | inline diff:
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 52d41658c1..152b100624 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -3245,23 +3245,6 @@ PQsendPipelineSync(PGconn *conn)
 /*
  * Workhorse function 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 no need to
- * end pipeline mode and start it again.
- *
- * If a command in a pipeline fails, every subsequent command up to and
- * including the result to the Sync message sent by pqPipelineSyncInternal
- * gets set to PGRES_PIPELINE_ABORTED state. If the whole pipeline is
- * processed without error, a PGresult with PGRES_PIPELINE_SYNC is produced.
- *
- * Queries can already have been sent before pqPipelineSyncInternal is called,
- * but pqPipelineSyncInternal needs to be called before retrieving command
- * results.
- *
- * The connection will remain in pipeline mode and unavailable for new
- * synchronous command execution functions until all results from the pipeline
- * are processed by the client.
- *
  * immediate_flush controls if the flush happens immediately after sending the
  * Sync message or not.
  */


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

^ permalink  raw  reply  [nested|flat] 15+ messages in thread

* Re: Add PQsendSyncMessage() to libpq
  2023-11-12 13:37 Re: Add PQsendSyncMessage() to libpq Anton Kirilov <[email protected]>
  2023-11-13 08:19 ` Re: Add PQsendSyncMessage() to libpq Anthonin Bonnefoy <[email protected]>
  2023-12-29 11:52   ` Re: Add PQsendSyncMessage() to libpq Jelte Fennema-Nio <[email protected]>
  2023-12-31 00:37     ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  2024-01-10 06:40       ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  2024-01-15 07:50         ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  2024-01-15 09:49           ` Re: Add PQsendSyncMessage() to libpq Alvaro Herrera <[email protected]>
  2024-01-16 03:32             ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
@ 2024-01-16 13:55               ` Alvaro Herrera <[email protected]>
  2024-01-17 07:30                 ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  0 siblings, 1 reply; 15+ messages in thread

From: Alvaro Herrera @ 2024-01-16 13:55 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: Jelte Fennema-Nio <[email protected]>; Anthonin Bonnefoy <[email protected]>; Anton Kirilov <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers

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 most of it should be removed -- these
> > things should be explained in the SGML docs, and I think they are, in
> > the "Using Pipeline Mode" section.  We can just have the lines this
> > patch is adding.
> 
> Hmm.  The first two sentences about being able to submit more commands
> to the pipeline are documented in the subsection "Issuing Queries".
> The third sentence is implied in the second paragraph of this
> subsection.  The 4th paragraph of the comment where sync commands
> cannot be issued until all the results from the pipeline have been
> consumed is mentioned in the first paragraph in "Using Pipeline Mode".
> So you are right that this could be entirely removed.

(I'm pretty sure that the history of this comment is that Craig Ringer
wrote it for his prototype patch, and then I took the various parts and
struggled to add them as SGML docs as it made logical sense.  So if
there's anything in the comment that's important and not covered by the
docs, that would be a docs bug.)  I agree with your findings.

> How about the attached to remove all that, then?

Looks good, thank you.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Tiene valor aquel que admite que es un cobarde" (Fernandel)






^ permalink  raw  reply  [nested|flat] 15+ messages in thread

* Re: Add PQsendSyncMessage() to libpq
  2023-11-12 13:37 Re: Add PQsendSyncMessage() to libpq Anton Kirilov <[email protected]>
  2023-11-13 08:19 ` Re: Add PQsendSyncMessage() to libpq Anthonin Bonnefoy <[email protected]>
  2023-12-29 11:52   ` Re: Add PQsendSyncMessage() to libpq Jelte Fennema-Nio <[email protected]>
  2023-12-31 00:37     ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  2024-01-10 06:40       ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  2024-01-15 07:50         ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  2024-01-15 09:49           ` Re: Add PQsendSyncMessage() to libpq Alvaro Herrera <[email protected]>
  2024-01-16 03:32             ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  2024-01-16 13:55               ` Re: Add PQsendSyncMessage() to libpq Alvaro Herrera <[email protected]>
@ 2024-01-17 07:30                 ` Michael Paquier <[email protected]>
  2024-01-18 23:11                   ` Re: Add PQsendSyncMessage() to libpq Anton Kirilov <[email protected]>
  0 siblings, 1 reply; 15+ messages in thread

From: Michael Paquier @ 2024-01-17 07:30 UTC (permalink / raw)
  To: Alvaro Herrera <[email protected]>; +Cc: Jelte Fennema-Nio <[email protected]>; Anthonin Bonnefoy <[email protected]>; Anton Kirilov <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers

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


Attachments:

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

^ permalink  raw  reply  [nested|flat] 15+ messages in thread

* Re: Add PQsendSyncMessage() to libpq
  2023-11-12 13:37 Re: Add PQsendSyncMessage() to libpq Anton Kirilov <[email protected]>
  2023-11-13 08:19 ` Re: Add PQsendSyncMessage() to libpq Anthonin Bonnefoy <[email protected]>
  2023-12-29 11:52   ` Re: Add PQsendSyncMessage() to libpq Jelte Fennema-Nio <[email protected]>
  2023-12-31 00:37     ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  2024-01-10 06:40       ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  2024-01-15 07:50         ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  2024-01-15 09:49           ` Re: Add PQsendSyncMessage() to libpq Alvaro Herrera <[email protected]>
  2024-01-16 03:32             ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
  2024-01-16 13:55               ` Re: Add PQsendSyncMessage() to libpq Alvaro Herrera <[email protected]>
  2024-01-17 07:30                 ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
@ 2024-01-18 23:11                   ` Anton Kirilov <[email protected]>
  0 siblings, 0 replies; 15+ messages in thread

From: Anton Kirilov @ 2024-01-18 23:11 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Anthonin Bonnefoy <[email protected]>; Jelte Fennema-Nio <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers

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 pipelining support to libpq - perhaps it ought to be updated?

Best wishes,
Anton Kirilov





^ permalink  raw  reply  [nested|flat] 15+ messages in thread

* Re: Add PQsendSyncMessage() to libpq
  2023-11-12 13:37 Re: Add PQsendSyncMessage() to libpq Anton Kirilov <[email protected]>
@ 2023-12-29 11:49 ` Jelte Fennema-Nio <[email protected]>
  1 sibling, 0 replies; 15+ messages in thread

From: Jelte Fennema-Nio @ 2023-12-29 11:49 UTC (permalink / raw)
  To: Anton Kirilov <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Michael Paquier <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers

On Sun, 12 Nov 2023 at 14:37, Anton Kirilov <[email protected]> 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 of the patch.

The new patch looks great to me. And indeed consensus seems to have
been reached on the approach and that this patch is useful. So I'm
taking the liberty of marking this patch as Ready for Committer.






^ permalink  raw  reply  [nested|flat] 15+ messages in thread


end of thread, other threads:[~2024-01-18 23:11 UTC | newest]

Thread overview: 15+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-12-31 12:20 [PATCH v1 2/2] WIP: buffer alloc specialized for relation extension Luc Vlaming <[email protected]>
2023-11-12 13:37 Re: Add PQsendSyncMessage() to libpq Anton Kirilov <[email protected]>
2023-11-13 08:19 ` Re: Add PQsendSyncMessage() to libpq Anthonin Bonnefoy <[email protected]>
2023-12-29 11:52   ` Re: Add PQsendSyncMessage() to libpq Jelte Fennema-Nio <[email protected]>
2023-12-31 00:37     ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
2024-01-10 06:40       ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
2024-01-15 07:50         ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
2024-01-15 09:01           ` Re: Add PQsendSyncMessage() to libpq Jelte Fennema-Nio <[email protected]>
2024-01-15 23:28             ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
2024-01-15 09:49           ` Re: Add PQsendSyncMessage() to libpq Alvaro Herrera <[email protected]>
2024-01-16 03:32             ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
2024-01-16 13:55               ` Re: Add PQsendSyncMessage() to libpq Alvaro Herrera <[email protected]>
2024-01-17 07:30                 ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
2024-01-18 23:11                   ` Re: Add PQsendSyncMessage() to libpq Anton Kirilov <[email protected]>
2023-12-29 11:49 ` Re: Add PQsendSyncMessage() to libpq Jelte Fennema-Nio <[email protected]>

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