Improve doc on parallel stream changes for Stream Abort message

Started by Anthonin Bonnefoy7 months ago4 messages
#1Anthonin Bonnefoy
anthonin.bonnefoy@datadoghq.com
1 attachment(s)

Hi,

While adding parsing of logical replication to Wireshark[1]https://gitlab.com/wireshark/wireshark/-/merge_requests/20268, I've
found the Stream Abort description possibly misleading. Stream Abort
will have the Abort LSN and TS when parallel streaming is enabled.
However, the documentation only mentions "This field is available
since protocol version 4" which could be interpreted (at least, I've
interpreted it this way) as "this field will always present with
protocol version 4".

The attached patch adds a "(only present for parallel stream)" mention
to the Abort LSN and TS documentation, akin to what's done for
parallel transactions.

Regards,
Anthonin

[1]: https://gitlab.com/wireshark/wireshark/-/merge_requests/20268

Attachments:

v01-0001-Doc-improve-documentation-on-stream-abort.patchapplication/octet-stream; name=v01-0001-Doc-improve-documentation-on-stream-abort.patchDownload
From c8bcddf22c02422c5a678f70f2120cf56d0022bc Mon Sep 17 00:00:00 2001
From: Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com>
Date: Mon, 23 Jun 2025 09:16:13 +0200
Subject: Doc: improve documentation on stream abort

Protocol v4 introduces parallel streaming. With this change, Stream
Abort messages will have additional abort info when parallel streaming
is enabled. However, the doc only mentions "This field is available
since protocol version 4", which can be confused as they will be
present as long as it's using protocol version 4.

This patch adds precision that abort LSN and TS are only present if
parallel stream is used.
---
 doc/src/sgml/protocol.sgml | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 137ffc8d0b7..b4b916496a7 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -7292,23 +7292,23 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;"
        <term>Int64 (XLogRecPtr)</term>
        <listitem>
         <para>
-         The LSN of the abort. This field is available since protocol version
-         4.
-        </para>
-       </listitem>
-      </varlistentry>
-
-      <varlistentry>
-       <term>Int64 (TimestampTz)</term>
-       <listitem>
-        <para>
-         Abort timestamp of the transaction. The value is in number
-         of microseconds since PostgreSQL epoch (2000-01-01). This field is
+         The LSN of the abort (only present for parallel stream). This field is
          available since protocol version 4.
         </para>
        </listitem>
       </varlistentry>
 
+      <varlistentry>
+       <term>Int64 (TimestampTz)</term>
+       <listitem>
+        <para>
+         Abort timestamp of the transaction (only present for parallel stream).
+         The value is in number of microseconds since PostgreSQL epoch (2000-01-01).
+         This field is available since protocol version 4.
+        </para>
+       </listitem>
+      </varlistentry>
+
      </variablelist>
     </listitem>
    </varlistentry>
-- 
2.49.0

#2Amit Kapila
amit.kapila16@gmail.com
In reply to: Anthonin Bonnefoy (#1)
Re: Improve doc on parallel stream changes for Stream Abort message

On Mon, Jun 23, 2025 at 2:10 PM Anthonin Bonnefoy
<anthonin.bonnefoy@datadoghq.com> wrote:

While adding parsing of logical replication to Wireshark[1], I've
found the Stream Abort description possibly misleading. Stream Abort
will have the Abort LSN and TS when parallel streaming is enabled.
However, the documentation only mentions "This field is available
since protocol version 4" which could be interpreted (at least, I've
interpreted it this way) as "this field will always present with
protocol version 4".

The attached patch adds a "(only present for parallel stream)" mention
to the Abort LSN and TS documentation, akin to what's done for
parallel transactions.

How about a slightly modified version like: (a) The LSN of the abort
operation, present only when the change stream can be applied in
parallel. This field is available since protocol version 4. (b) Abort
timestamp of the transaction, present only when the change stream can
be applied in parallel. The value is in number of microseconds since
PostgreSQL epoch (2000-01-01). This field is available since protocol
version 4.

--
With Regards,
Amit Kapila.

#3Anthonin Bonnefoy
anthonin.bonnefoy@datadoghq.com
In reply to: Amit Kapila (#2)
Re: Improve doc on parallel stream changes for Stream Abort message

On Tue, Jun 24, 2025 at 7:26 AM Amit Kapila <amit.kapila16@gmail.com> wrote:

How about a slightly modified version like: (a) The LSN of the abort
operation, present only when the change stream can be applied in
parallel. This field is available since protocol version 4. (b) Abort
timestamp of the transaction, present only when the change stream can
be applied in parallel. The value is in number of microseconds since
PostgreSQL epoch (2000-01-01). This field is available since protocol
version 4.

What about ", present only when streaming is set to parallel"? I think
clarifying the relation between streaming=parallel and the presence of
those fields is the important part.

#4Amit Kapila
amit.kapila16@gmail.com
In reply to: Anthonin Bonnefoy (#3)
Re: Improve doc on parallel stream changes for Stream Abort message

On Tue, Jun 24, 2025 at 11:58 AM Anthonin Bonnefoy
<anthonin.bonnefoy@datadoghq.com> wrote:

On Tue, Jun 24, 2025 at 7:26 AM Amit Kapila <amit.kapila16@gmail.com> wrote:

How about a slightly modified version like: (a) The LSN of the abort
operation, present only when the change stream can be applied in
parallel. This field is available since protocol version 4. (b) Abort
timestamp of the transaction, present only when the change stream can
be applied in parallel. The value is in number of microseconds since
PostgreSQL epoch (2000-01-01). This field is available since protocol
version 4.

What about ", present only when streaming is set to parallel"? I think
clarifying the relation between streaming=parallel and the presence of
those fields is the important part.

Works for me. I'll wait till tomorrow morning to see if there are any
further comments, and then push it.

--
With Regards,
Amit Kapila.