DOCS - inactive_since field readability

Started by Peter Smith11 months ago6 messages

smithpb2250@gmail.com

11 months ago

2 attachment(s)

Hi hackers.

This thread proposes to improve the readability of the
'inactive_since' field description (see pg_replication_slots system
view).

No changes to any facts, just the wording.

The current description looks like this:
------
The time when the slot became inactive. NULL if the slot is currently
being streamed. If the slot becomes invalid, this value will never be
updated. Note that for slots on the standby that are being synced from
a primary server (whose synced field is true), the inactive_since
indicates the time when slot synchronization (see Section 47.2.3) was
most recently stopped. NULL if the slot has always been synchronized.
On standby, this is useful for slots that are being synced from a
primary server (whose synced field is true) so they know when the slot
stopped being synchronized.
------

Notice there are two very different descriptions depending on the context:
- one for when the replication slot is on the primary server
- and another description for when the slot is on the standby and
being synced from the primary.

e.g. how to interpret NULL values is entirely different:
* NULL if the slot is currently being streamed.
* NULL if the slot has always been synchronized.

IMO the separation of these distinct contexts is not as apparent as it
could be ==> Patch 0001.

~~~

Patch 0001.

This patch simply adds a blank line between these description parts.

======

Patch 0002 (optional)

The 2nd part description seems overly wordy due to unnecessary repetitions.
* "Note that for slots on the standby that are being synced from a
primary server (whose synced field is true) ..."
* "On standby, this is useful for slots that are being synced from a
primary server (whose synced field is true) ..."

I've had a go at rewording this paragraph in the 0002 patch.

======

Thoughts?

======
Kind Regards,
Peter Smith.
Fujitsu Australia

Attachments:

v1-0001-DOCS-inactive_since.-Add-blank-line.patchapplication/octet-stream; name=v1-0001-DOCS-inactive_since.-Add-blank-line.patchDownload

From a50562a5628be514a6dac65d5f7473a1488b6b56 Mon Sep 17 00:00:00 2001
From: Peter Smith <peter.b.smith@fujitsu.com>
Date: Fri, 7 Feb 2025 16:57:12 +1100
Subject: [PATCH v1] DOCS - inactive_since. Add blank line.

---
 doc/src/sgml/system-views.sgml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index 8e2b0a7..7db509d 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -2567,6 +2567,8 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
       <para>
         The time when the slot became inactive. <literal>NULL</literal> if the
         slot is currently being streamed.
+      </para>
+      <para>
         Note that for slots on the standby that are being synced from a
         primary server (whose <structfield>synced</structfield> field is
         <literal>true</literal>), the <structfield>inactive_since</structfield>
-- 
1.8.3.1

v1-0002-DOCS-inactive_since.-2nd-para-wording.patchapplication/octet-stream; name=v1-0002-DOCS-inactive_since.-2nd-para-wording.patchDownload

From de9934c3738f07b7c87c04d7343a730735c843b6 Mon Sep 17 00:00:00 2001
From: Peter Smith <peter.b.smith@fujitsu.com>
Date: Fri, 7 Feb 2025 17:26:28 +1100
Subject: [PATCH v1] DOCS - inactive_since. 2nd para wording

---
 doc/src/sgml/system-views.sgml | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index 7db509d..00625ce 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -2569,16 +2569,14 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
         slot is currently being streamed.
       </para>
       <para>
-        Note that for slots on the standby that are being synced from a
-        primary server (whose <structfield>synced</structfield> field is
-        <literal>true</literal>), the <structfield>inactive_since</structfield>
-        indicates the time when slot synchronization (see <xref
-        linkend="logicaldecoding-replication-slots-synchronization"/>)
-        was most recently stopped.  <literal>NULL</literal> if the slot
-        has always been synchronized. On standby, this is useful for slots
-        that are being synced from a primary server (whose
-        <structfield>synced</structfield> field is <literal>true</literal>)
-        so they know when the slot stopped being synchronized.
+        For standby slots that are being synced from a primary server
+        (<structfield>synced</structfield> field is <literal>true</literal>),
+        <structfield>inactive_since</structfield> records the last time slot
+        synchronization (see <xref
+        linkend="logicaldecoding-replication-slots-synchronization"/>) stopped.
+        <structfield>inactive_since</structfield> remains <literal>NULL</literal>
+        if the slot has always been synchronized. This helps standby slots
+        track when synchronization was interrupted.
       </para></entry>
      </row>
 
-- 
1.8.3.1

Amit Kapila

amit.kapila16@gmail.com

11 months ago

In reply to: Peter Smith (#1)

Re: DOCS - inactive_since field readability

On Fri, Feb 7, 2025 at 12:05 PM Peter Smith <smithpb2250@gmail.com> wrote:

I've had a go at rewording this paragraph in the 0002 patch.

The change in 0001 looks odd after seeing it in HTML format. We should
either add one empty line between two paragraphs otherwise it doesn't
appear good. Did you see multi-paragraphs in any other column
definitions?

For the 0002 patch, I find the following changes as improvements,
*
      <para>
-        Note that for slots on the standby
to
+        For standby slots

*
On standby, this is useful for slots
-        that are being synced from a primary server (whose
-        <structfield>synced</structfield> field is <literal>true</literal>)
-        so they know when the slot stopped being synchronized.
to
This helps standby slots
+        track when synchronization was interrupted.

Other than that, I find the current wording okay.

--
With Regards,
Amit Kapila.

Peter Smith

smithpb2250@gmail.com

11 months ago

In reply to: Amit Kapila (#2)

2 attachment(s)

Re: DOCS - inactive_since field readability

On Tue, Feb 11, 2025 at 10:10 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

...

The change in 0001 looks odd after seeing it in HTML format. We should
either add one empty line between two paragraphs otherwise it doesn't
appear good. Did you see multi-paragraphs in any other column
definitions?
For the 0002 patch, I find the following changes as improvements,
*
<para>
-        Note that for slots on the standby
to
+        For standby slots
*
On standby, this is useful for slots
-        that are being synced from a primary server (whose
-        <structfield>synced</structfield> field is <literal>true</literal>)
-        so they know when the slot stopped being synchronized.
to
This helps standby slots
+        track when synchronization was interrupted.
Other than that, I find the current wording okay.

Hi Amit, thanks for the feedback!

//////

Patch 0001

The pg_replication_slots system view is unusual in that there can be
entirely different descriptions of the same field depending on the
context, such as:
a- for logical slots
b- for physical slots
c- for primary servers versus standby servers

IIUC your 0001 feedback says that a blank line might be ok, but just
doing it for 'active_since' and nothing else makes it look odd. So, I
have modified the 0001 patch to add blank lines separating those
different contexts (e.g. a,b,c) for all fields. I think it improves
the readability.

In passing.
- changed null to <literal>NULL</literal>
- changed true to <literal>true</literal>
- changed false to <literal>false</literal>

//////

Patch 0002

Modified the text as suggested.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

Attachments:

v2-0001-DOCS-pg_replication_slots.-Add-blank-lines.patchapplication/octet-stream; name=v2-0001-DOCS-pg_replication_slots.-Add-blank-lines.patchDownload

From efd2043d8f55edca4f889eab6b76fc9ee6887bf9 Mon Sep 17 00:00:00 2001
From: Peter Smith <peter.b.smith@fujitsu.com>
Date: Fri, 14 Feb 2025 08:47:55 +1100
Subject: [PATCH v2] DOCS - pg_replication_slots. Add blank lines.

---
 doc/src/sgml/system-views.sgml | 64 +++++++++++++++++++++++++++++-------------
 1 file changed, 44 insertions(+), 20 deletions(-)

diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index be81c2b..f4dfc34 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -2384,7 +2384,10 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
        <structfield>plugin</structfield> <type>name</type>
       </para>
       <para>
-       The base name of the shared object containing the output plugin this logical slot is using, or null for physical slots.
+       The base name of the shared object containing the output plugin this logical slot is using.
+      </para>
+      <para>
+       Always <literal>NULL</literal> for physical slots.
       </para></entry>
      </row>
 
@@ -2404,7 +2407,10 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
       </para>
       <para>
        The OID of the database this slot is associated with, or
-       null. Only logical slots have an associated database.
+       <literal>NULL</literal>. Only logical slots have an associated database.
+      </para>
+      <para>
+       Always <literal>NULL</literal> for physical slots.
       </para></entry>
      </row>
 
@@ -2415,7 +2421,10 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
       </para>
       <para>
        The name of the database this slot is associated with, or
-       null. Only logical slots have an associated database.
+       <literal>NULL</literal>. Only logical slots have an associated database.
+      </para>
+      <para>
+       Always <literal>NULL</literal> for physical slots.
       </para></entry>
      </row>
 
@@ -2424,7 +2433,7 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
        <structfield>temporary</structfield> <type>bool</type>
       </para>
       <para>
-       True if this is a temporary replication slot. Temporary slots are
+       <literal>true</literal> if this is a temporary replication slot. Temporary slots are
        not saved to disk and are automatically dropped on error or when
        the session has finished.
       </para></entry>
@@ -2435,7 +2444,7 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
        <structfield>active</structfield> <type>bool</type>
       </para>
       <para>
-       True if this slot is currently being streamed
+       <literal>true</literal> if this slot is currently being streamed
       </para></entry>
      </row>
 
@@ -2493,7 +2502,10 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
        The address (<literal>LSN</literal>) up to which the logical
        slot's consumer has confirmed receiving data. Data corresponding to the
        transactions committed before this <literal>LSN</literal> is not
-       available anymore. <literal>NULL</literal> for physical slots.
+       available anymore.
+      </para>
+      <para>
+       Always <literal>NULL</literal> for physical slots.
       </para></entry>
      </row>
 
@@ -2555,10 +2567,11 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
        <structfield>two_phase</structfield> <type>bool</type>
       </para>
       <para>
-       True if the slot is enabled for decoding prepared transactions.  Always
-       false for physical slots.
-      </para></entry>
-     </row>
+       <literal>true</literal> if the slot is enabled for decoding prepared transactions.
+      </para>
+      <para>
+       Always <literal>false</literal> for physical slots.
+      </para></entry>     </row>
 
      <row>
       <entry role="catalog_table_entry"><para role="column_definition">
@@ -2568,6 +2581,8 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
         The time when the slot became inactive. <literal>NULL</literal> if the
         slot is currently being streamed. If the slot becomes invalid,
         this value will never be updated.
+      </para>
+      <para>
         Note that for slots on the standby that are being synced from a
         primary server (whose <structfield>synced</structfield> field is
         <literal>true</literal>), the <structfield>inactive_since</structfield>
@@ -2586,10 +2601,13 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
        <structfield>conflicting</structfield> <type>bool</type>
       </para>
       <para>
-       True if this logical slot conflicted with recovery (and so is now
-       invalidated). When this column is true, check
+       <literal>true</literal> if this logical slot conflicted with recovery (and so is now
+       invalidated). When this column is <literal>true</literal>, check
        <structfield>invalidation_reason</structfield> column for the conflict
-       reason. Always <literal>NULL</literal> for physical slots.
+       reason.
+      </para>
+      <para>
+       Always <literal>NULL</literal> for physical slots.
       </para></entry>
      </row>
 
@@ -2630,9 +2648,12 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
        <structfield>failover</structfield> <type>bool</type>
       </para>
       <para>
-       True if this is a logical slot enabled to be synced to the standbys
+       <literal>true</literal> if this is a logical slot enabled to be synced to the standbys
        so that logical replication can be resumed from the new primary
-       after failover. Always false for physical slots.
+       after failover.
+      </para>
+      <para>
+       Always <literal>false</literal> for physical slots.
       </para></entry>
      </row>
 
@@ -2641,12 +2662,15 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
        <structfield>synced</structfield> <type>bool</type>
       </para>
       <para>
-       True if this is a logical slot that was synced from a primary server.
-       On a hot standby, the slots with the synced column marked as true can
-       neither be used for logical decoding nor dropped manually. The value
+       <literal>true</literal> if this is a logical slot that was synced from a primary server.
+       On a hot standby, the slots with the synced column marked as <literal>true</literal> can
+       neither be used for logical decoding nor dropped manually.
+      </para>
+      <para>
+       The value
        of this column has no meaning on the primary server; the column value on
-       the primary is default false for all slots but may (if leftover from a
-       promoted standby) also be true.
+       the primary is default <literal>false</literal> for all slots but may (if leftover from a
+       promoted standby) also be <literal>true</literal>.
       </para></entry>
      </row>
 
-- 
1.8.3.1

v2-0002-DOCS-pg_replication_slots.-Improve-the-active_sin.patchapplication/octet-stream; name=v2-0002-DOCS-pg_replication_slots.-Improve-the-active_sin.patchDownload

From 80332fe141319d8a3bde4c1fc1214a676cdda9ef Mon Sep 17 00:00:00 2001
From: Peter Smith <peter.b.smith@fujitsu.com>
Date: Fri, 14 Feb 2025 09:00:36 +1100
Subject: [PATCH v2] DOCS - pg_replication_slots. Improve the active_since
 description.

---
 doc/src/sgml/system-views.sgml | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index f4dfc34..85267cf 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -2583,16 +2583,14 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
         this value will never be updated.
       </para>
       <para>
-        Note that for slots on the standby that are being synced from a
+        For standby slots that are being synced from a
         primary server (whose <structfield>synced</structfield> field is
         <literal>true</literal>), the <structfield>inactive_since</structfield>
         indicates the time when slot synchronization (see <xref
         linkend="logicaldecoding-replication-slots-synchronization"/>)
         was most recently stopped.  <literal>NULL</literal> if the slot
-        has always been synchronized. On standby, this is useful for slots
-        that are being synced from a primary server (whose
-        <structfield>synced</structfield> field is <literal>true</literal>)
-        so they know when the slot stopped being synchronized.
+        has always been synchronized. This helps standby slots track when
+        synchronization was interrupted.
       </para></entry>
      </row>
 
-- 
1.8.3.1

Amit Kapila

amit.kapila16@gmail.com

11 months ago

In reply to: Peter Smith (#3)

Re: DOCS - inactive_since field readability

On Fri, Feb 14, 2025 at 4:04 AM Peter Smith <smithpb2250@gmail.com> wrote:

On Tue, Feb 11, 2025 at 10:10 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

...

The change in 0001 looks odd after seeing it in HTML format. We should
either add one empty line between two paragraphs otherwise it doesn't
appear good. Did you see multi-paragraphs in any other column
definitions?

Patch 0001

The pg_replication_slots system view is unusual in that there can be
entirely different descriptions of the same field depending on the
context, such as:
a- for logical slots
b- for physical slots
c- for primary servers versus standby servers

IIUC your 0001 feedback says that a blank line might be ok, but just
doing it for 'active_since' and nothing else makes it look odd.

No, I meant to say that the description didn't looked any better to me
even after your 0001 patch. The second paragraph started immediately
in the next line which doesn't make it look any better. If we really
want to make it look better then one more additional line is required.
However, I don't want to go in that direction unless we have some
history of writing the docs similarly. I suggest let's go with your
0002 patch as that makes the description concise and clear.

--
With Regards,
Amit Kapila.

Peter Smith

smithpb2250@gmail.com

11 months ago

In reply to: Amit Kapila (#4)

1 attachment(s)

Re: DOCS - inactive_since field readability

On Fri, Feb 14, 2025 at 4:06 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

...

No, I meant to say that the description didn't looked any better to me
even after your 0001 patch. The second paragraph started immediately
in the next line which doesn't make it look any better. If we really
want to make it look better then one more additional line is required.
However, I don't want to go in that direction unless we have some
history of writing the docs similarly. I suggest let's go with your
0002 patch as that makes the description concise and clear.

OK. My blank lines patch has been abandoned. Here is just the
'inactive_since' description patch (now called 0001).

======
Kind Regards,
Peter Smith.
Fujitsu Australia

Attachments:

v3-0001-DOCS-improve-pg_replication_slots.inactive_since-.patchapplication/octet-stream; name=v3-0001-DOCS-improve-pg_replication_slots.inactive_since-.patchDownload

From a631647af6bb8d57e0858fda468e0b9f37bd3929 Mon Sep 17 00:00:00 2001
From: Peter Smith <peter.b.smith@fujitsu.com>
Date: Mon, 17 Feb 2025 07:30:26 +1100
Subject: [PATCH v3] DOCS -improve pg_replication_slots.inactive_since
 description

---
 doc/src/sgml/system-views.sgml | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index be81c2b..ad2903d 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -2568,16 +2568,14 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
         The time when the slot became inactive. <literal>NULL</literal> if the
         slot is currently being streamed. If the slot becomes invalid,
         this value will never be updated.
-        Note that for slots on the standby that are being synced from a
+        For standby slots that are being synced from a
         primary server (whose <structfield>synced</structfield> field is
         <literal>true</literal>), the <structfield>inactive_since</structfield>
         indicates the time when slot synchronization (see <xref
         linkend="logicaldecoding-replication-slots-synchronization"/>)
         was most recently stopped.  <literal>NULL</literal> if the slot
-        has always been synchronized. On standby, this is useful for slots
-        that are being synced from a primary server (whose
-        <structfield>synced</structfield> field is <literal>true</literal>)
-        so they know when the slot stopped being synchronized.
+        has always been synchronized. This helps standby slots track when
+        synchronization was interrupted.
       </para></entry>
      </row>
 
-- 
1.8.3.1

Amit Kapila

amit.kapila16@gmail.com

11 months ago

In reply to: Peter Smith (#5)

Re: DOCS - inactive_since field readability

On Mon, Feb 17, 2025 at 2:06 AM Peter Smith <smithpb2250@gmail.com> wrote:

On Fri, Feb 14, 2025 at 4:06 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

...

No, I meant to say that the description didn't looked any better to me
even after your 0001 patch. The second paragraph started immediately
in the next line which doesn't make it look any better. If we really
want to make it look better then one more additional line is required.
However, I don't want to go in that direction unless we have some
history of writing the docs similarly. I suggest let's go with your
0002 patch as that makes the description concise and clear.

OK. My blank lines patch has been abandoned. Here is just the
'inactive_since' description patch (now called 0001).

Looks good to me. I'll push this tomorrow unless there are any further
comments/suggestions.

--
With Regards,
Amit Kapila.