On Mon, Sep 9, 2024 at 01:15:32PM +1000, Peter Smith wrote:
> On Mon, Sep 9, 2024 at 12:20 PM David G. Johnston
> <[email protected]> wrote:
> >
> >
> >
> > On Sun, Sep 8, 2024, 18:55 Peter Smith <[email protected]> wrote:
> >>
> >> Saying "The time..." is fine, but the suggestions given seem backwards to
> >> me:
> >> - The time this slot was inactivated
> >> - The time when the slot became inactive.
> >> - The time when the slot was deactivated.
> >>
> >> e.g. It is not like light switch. So, there is no moment when the
> >> active slot "became inactive" or "was deactivated".
> >
> >
> > While this is plausible the existing wording and the name of the field
> > definitely fail to convey this.
> >
> >>
> >> Rather, the 'inactive_since' timestamp field is simply:
> >> - The time the slot was last active.
> >> - The last time the slot was active.
> >
> >
> > I see your point but that wording is also quite confusing when an active
> > slot returns null for this field.
> >
> > At this point I'm confused enough to need whatever wording is taken to be
> > supported by someone explaining the code that interacts with this field.
> >
>
> Me too. I created this thread primarily to get the description changed
> to clarify this field represents a moment in time, rather than a
> duration. So I will be happy with any wording that addresses that.
I dug into the code and came up with the attached patch. "active" means
there is a process streaming the slot, and the "inactive_since" time
means the last time synchronous slot streaming was stopped. Doc patch
attached.
I am not sure what other things are needed, but this is certainly
unclear. This comment from src/backend/replication/logical/slotsync.c
helped me understand this:
* We need to update inactive_since only when we are promoting standby to
* correctly interpret the inactive_since if the standby gets promoted
* without a restart. We don't want the slots to appear inactive for a
* long time after promotion if they haven't been synchronized recently.
* Whoever acquires the slot, i.e., makes the slot active, will reset it.
--
Bruce Momjian <[email protected]> https://momjian.us
EDB https://enterprisedb.com
When a patient asks the doctor, "Am I going to die?", he means
"Am I going to die soon?"
diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index 61d28e701f2..934104e1bc9 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -2435,7 +2435,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 actively being used
+ True if this slot is currently currently being streamed
</para></entry>
</row>
@@ -2444,9 +2444,8 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
<structfield>active_pid</structfield> <type>int4</type>
</para>
<para>
- The process ID of the session using this slot if the slot
- is currently actively being used. <literal>NULL</literal> if
- inactive.
+ The process ID of the session streaming data for this slot.
+ <literal>NULL</literal> if inactive.
</para></entry>
</row>
@@ -2566,15 +2565,13 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
<structfield>inactive_since</structfield> <type>timestamptz</type>
</para>
<para>
- The time since the slot has become inactive.
- <literal>NULL</literal> if the slot is currently being used.
- 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 last
- synchronization (see
- <xref linkend="logicaldecoding-replication-slots-synchronization"/>)
- time.
+ The time slot synchronization (see <xref
+ linkend="logicaldecoding-replication-slots-synchronization"/>)
+ was most recently stopped. <literal>NULL</literal> if the slot
+ has always been synchronized. This is useful for slots on the
+ standby that are intended to be synced from a primary server (whose
+ <structfield>synced</structfield> field is <literal>true</literal>)
+ so they know when the slot stopped being synchronized.
</para></entry>
</row>
diff --git a/src/backend/replication/logical/slotsync.c b/src/backend/replication/logical/slotsync.c
index f9649eec1a5..cbd8c00aa3f 100644
--- a/src/backend/replication/logical/slotsync.c
+++ b/src/backend/replication/logical/slotsync.c
@@ -1517,7 +1517,7 @@ update_synced_slots_inactive_since(void)
* correctly interpret the inactive_since if the standby gets promoted
* without a restart. We don't want the slots to appear inactive for a
* long time after promotion if they haven't been synchronized recently.
- * Whoever acquires the slot i.e.makes the slot active will reset it.
+ * Whoever acquires the slot, i.e., makes the slot active, will reset it.
*/
if (!StandbyMode)
return;
diff --git a/src/include/replication/slot.h b/src/include/replication/slot.h
index 45582cf9d89..e3e0816bcd3 100644
--- a/src/include/replication/slot.h
+++ b/src/include/replication/slot.h
@@ -205,7 +205,7 @@ typedef struct ReplicationSlot
*/
XLogRecPtr last_saved_confirmed_flush;
- /* The time since the slot has become inactive */
+ /* The time slot sychronized was stopped. */
TimestampTz inactive_since;
} ReplicationSlot;
