Re: Mention idle_replication_slot_timeout in pg_replication_slots docs

2025-06-26 Thread Fujii Masao



On 2025/06/26 15:43, Hayato Kuroda (Fujitsu) wrote:

Dear Fujii-san,


The pg_replication_slots documentation mentions only max_slot_wal_keep_size
as a condition under which the wal_status column can show unreserved or lost.
However, since commit ac0e33136ab, idle_replication_slot_timeout can also
cause this behavior when it is set. This has not been documented yet.
https://www.postgresql.org/docs/devel/view-pg-replication-slots.html


Oh, I feel the doc should be also updated.


Thanks for the review!



So, how about updating the documentation to also mention
idle_replication_slot_timeout as a factor that can cause wal_status to
become unreserved or lost? Patch attached.


One comment:

```
  
   lost means that some required WAL files have
   been removed and this slot is no longer usable.
  
```

IIUC, there is a case that status is "lost" but the required WALs have not been
dropped yet if the slot was invalidated due to the timeout. How about removing 
the
first part:

```
lost means that this slot is no longer usable.
```


Agreed. Attached is the updated version of the patch.

Regards,

--
Fujii Masao
NTT DATA Japan Corporation
From a730908764b2255fd7ab36441417bddc643d4a5e Mon Sep 17 00:00:00 2001
From: Fujii Masao 
Date: Thu, 26 Jun 2025 16:49:59 +0900
Subject: [PATCH v2] doc: Mention idle_replication_slot_timeout in
 pg_replication_slots docs.

The pg_replication_slots documentation previously mentioned only
max_slot_wal_keep_size as a condition under which the wal_status column
can show unreserved or lost. However, since commit ac0e33136ab,
idle_replication_slot_timeout can also cause this behavior when it is set.
This was not documented.

This commit updates the documentation to also mention
idle_replication_slot_timeout as a factor that can cause wal_status
to become unreserved or lost.

Author: Fujii Masao 
Reviewed-by: Hayato Kuroda 
Reviewed-by: Nisha Moond 
Discussion: 
https://postgr.es/m/78b34e84-2195-4f28-a151-5d204a382...@oss.nttdata.com
---
 doc/src/sgml/system-views.sgml | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index 986ae1f543d..308e5dabf3b 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -2825,15 +2825,15 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts 
ppx
 
 
  
-  lost means that some required WAL files have
-  been removed and this slot is no longer usable.
+  lost means that this slot is no longer usable.
  
 

The last two states are seen only when
 is
-   non-negative. If restart_lsn is NULL, this
-   field is null.
+   non-negative and/or 
+   is greater than zero. If restart_lsn is NULL,
+   this field is null.
   
  
 
-- 
2.49.0



Re: Mention idle_replication_slot_timeout in pg_replication_slots docs

2025-06-26 Thread Fujii Masao




On 2025/06/26 15:46, Nisha Moond wrote:

On Wed, Jun 25, 2025 at 9:56 PM Fujii Masao  wrote:


Hi,

The pg_replication_slots documentation mentions only max_slot_wal_keep_size
as a condition under which the wal_status column can show unreserved or lost.
However, since commit ac0e33136ab, idle_replication_slot_timeout can also
cause this behavior when it is set. This has not been documented yet.
https://www.postgresql.org/docs/devel/view-pg-replication-slots.html



+1 to the doc update.


Thanks for the review!



So, how about updating the documentation to also mention
idle_replication_slot_timeout as a factor that can cause wal_status to
become unreserved or lost? Patch attached.



Since idle_replication_slot_timeout can only cause wal_status to
become 'lost' and not 'unreserved', perhaps we can reword the sentence
slightly for clarity, suggestion -
"The last two states are seen when max_slot_wal_keep_size is
non-negative and, the 'lost' state may also appear when
idle_replication_slot_timeout is greater than zero."


I was thinking that when idle_replication_slot_timeout triggers,
the following functions are called, and that wal_status can become
"unreserved" before ReplicationSlotRelease() runs. It's very short
period, though. Am I wrong?

ReplicationSlotMarkDirty();
ReplicationSlotSave();
ReplicationSlotRelease();

Regards,

--
Fujii Masao
NTT DATA Japan Corporation





RE: Mention idle_replication_slot_timeout in pg_replication_slots docs

2025-06-26 Thread Hayato Kuroda (Fujitsu)
Dear Fujii-san,

> Agreed. Attached is the updated version of the patch.

I confirmed that my point was fixed. LGTM.

Best regards,
Hayato Kuroda
FUJITSU LIMITED



Re: Mention idle_replication_slot_timeout in pg_replication_slots docs

2025-06-26 Thread Nisha Moond
On Thu, Jun 26, 2025 at 1:33 PM Fujii Masao  wrote:
>
>
>
> On 2025/06/26 15:46, Nisha Moond wrote:
> > On Wed, Jun 25, 2025 at 9:56 PM Fujii Masao  
> > wrote:
> >>
> >> Hi,
> >>
> >> The pg_replication_slots documentation mentions only max_slot_wal_keep_size
> >> as a condition under which the wal_status column can show unreserved or 
> >> lost.
> >> However, since commit ac0e33136ab, idle_replication_slot_timeout can also
> >> cause this behavior when it is set. This has not been documented yet.
> >> https://www.postgresql.org/docs/devel/view-pg-replication-slots.html
> >>
> >
> > +1 to the doc update.
>
> Thanks for the review!
>
>
> >> So, how about updating the documentation to also mention
> >> idle_replication_slot_timeout as a factor that can cause wal_status to
> >> become unreserved or lost? Patch attached.
> >>
> >
> > Since idle_replication_slot_timeout can only cause wal_status to
> > become 'lost' and not 'unreserved', perhaps we can reword the sentence
> > slightly for clarity, suggestion -
> > "The last two states are seen when max_slot_wal_keep_size is
> > non-negative and, the 'lost' state may also appear when
> > idle_replication_slot_timeout is greater than zero."
>
> I was thinking that when idle_replication_slot_timeout triggers,
> the following functions are called, and that wal_status can become
> "unreserved" before ReplicationSlotRelease() runs. It's very short
> period, though. Am I wrong?
>
> ReplicationSlotMarkDirty();
> ReplicationSlotSave();
> ReplicationSlotRelease();
>

Thank you for pointing it out.
You are correct that while the checkpointer is in the process of
invalidating a slot, it sets its PID as the slot’s active_pid. During
this short window, if a user queries pg_replication_slot, the
underlying function pg_get_replication_slots will compute the
wal_status as 'unreserved' for the invalidated slot because the slot
has a valid active_pid.

That said, it's reasonable to mention in the doc that 'unreserved' may
appear when idle_replication_slot_timeout is greater than zero, as
this can indeed happen. So, let's retain the current description.

However, this behavior isn’t specific to
idle_replication_slot_timeout. For example, when a slot is being
invalidated due to a different cause "wal_level_insufficient",
'unreserved' may also briefly appear in wal_status.

The current patch LGTM.

--
Thanks,
Nisha