From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, "Drouvot, Bertrand" <bertranddrouvot(dot)pg(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
Cc: | Amit Kapila <akapila(at)postgresql(dot)org>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: pgsql: Track last_inactive_time in pg_replication_slots. |
Date: | 2024-03-25 14:02:11 |
Message-ID: | CAA4eK1Ld9bSm1gEYD2_B78tkjhSpfG+ON-iJ7E3NiH5RLYtPOQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers pgsql-hackers |
On Mon, Mar 25, 2024 at 6:57 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Mon, Mar 25, 2024 at 7:11 AM Amit Kapila <akapila(at)postgresql(dot)org> wrote:
> > Track last_inactive_time in pg_replication_slots.
> >
> > This commit adds a new property called last_inactive_time for slots. It is
> > set to 0 whenever a slot is made active/acquired and set to the current
> > timestamp whenever the slot is inactive/released or restored from the disk.
> > Note that we don't set the last_inactive_time for the slots currently being
> > synced from the primary to the standby because such slots are typically
> > inactive as decoding is not allowed on those.
>
> So the field name is last_inactive_time, but if I'm reading this
> description right, it's actually the last time the slot was active,
> except for the weird exception for slots being synced. I'm wondering
> if this field needs to be renamed.
>
We considered the other two names as last_inactive_at and
last_active_time. For the first (last_inactive_at), there was an
argument that most other fields that display time ends with _time. For
the second (last_active_time), there was an argument that it could be
misleading as one could think that it should be updated each time WAL
record decoding is happening [1]. The other possibility is to name it
last_used_time but I think it won't be much different from
last_active_time.
> And I'm suspicious that having an exception for slots being synced is
> a bad idea. That makes too much of a judgement about how the user will
> use this field. It's usually better to just expose the data, and if
> the user needs helps to make sense of that data, then give them that
> help separately.
The reason we didn't set this for sync slots is that they won't be
usable (one can't use them to decode WAL) unless standby is promoted
[2]. But I see your point as well. So, I have copied the others
involved in this discussion to see what they think.
>
> In this case, that would mean removing the exception,
> but making it easy to tell the difference between slots are inactive
> because they're being synced and slots that are inactive for some
> other reason.
>
I think this can be differentiated with the help of 'synced' column in
pg_replication_slots.
[1] - https://www.postgresql.org/message-id/Zf1yx9QMbhgJ/Lfy%40ip-10-97-1-34.eu-west-3.compute.internal
[2] - https://www.postgresql.org/message-id/CAJpy0uBGv85dFiWMnNLm6NuEs3eTVicsJCyRvMGbR8H%2BfOVBnA%40mail.gmail.com
--
With Regards,
Amit Kapila.
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2024-03-25 14:20:51 | Re: pgsql: Track last_inactive_time in pg_replication_slots. |
Previous Message | Daniel Gustafsson | 2024-03-25 13:52:59 | pgsql: Fix typo in comment |
From | Date | Subject | |
---|---|---|---|
Next Message | Dean Rasheed | 2024-03-25 14:04:30 | Re: Adding OLD/NEW support to RETURNING |
Previous Message | Pankaj Raghav | 2024-03-25 13:53:56 | Re: Large block sizes support in Linux |