MIME-Version: 1.0
References: <E1roiEh-005Z7e-88@gemulon.postgresql.org>
 <CA+Tgmob_Ta-t2ty8QrKHBGnNLrf4ZYcwhGHGFsuUoFrAEDw4sA@mail.gmail.com>
 <CAA4eK1Ld9bSm1gEYD2_B78tkjhSpfG+ON-iJ7E3NiH5RLYtPOQ@mail.gmail.com>
 <CA+TgmoaCA-aaBiszkMKOTn6C3BKkY4hNAdNOLUhfMXdQxyrh9g@mail.gmail.com>
 <CAA4eK1+KAZoLBaPqnr8e8icF30fBMX1QwOBjYfLCZKXF_WRGnw@mail.gmail.com>
 <ZgGVZiZnvqRTDodo@ip-10-97-1-34.eu-west-3.compute.internal>
In-Reply-To: <ZgGVZiZnvqRTDodo@ip-10-97-1-34.eu-west-3.compute.internal>
From: Robert Haas <robertmhaas@gmail.com>
Date: Mon, 25 Mar 2024 11:49:00 -0400
Message-ID: 
 <CA+TgmoaY6RvsFM39B0QXr2pTpSm8p2NovRLkYizFJLLyRYWN0A@mail.gmail.com>
Subject: Re: pgsql: Track last_inactive_time in pg_replication_slots.
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>,
 shveta malik <shveta.malik@gmail.com>,
	Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>,
	Amit Kapila <akapila@postgresql.org>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CA%2BTgmoaY6RvsFM39B0QXr2pTpSm8p2NovRLkYizFJLLyRYWN0A%40mail.gmail.com>
Precedence: bulk

On Mon, Mar 25, 2024 at 11:16=E2=80=AFAM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:
> > IIUC, Bertrand's point was that users can interpret last_active_time
> > as a value that gets updated each time they decode a change which is
> > not what we are doing. So, this can confuse users. Your expectation of
> > answer (NULL) when the slot is active is correct and that is what will
> > happen.
>
> Yeah, and so would be the confusion: why is last_active_time NULL while o=
ne is
> using the slot?

I agree that users could get confused here, but the solution to that
shouldn't be to give the field a name that is the opposite of what it
actually does. I expect a field called last_inactive_time to tell me
the last time that the slot was inactive. Here, it tells me the last
time that a currently-inactive slot previously *WAS* active. How can
you justify calling that the last *INACTIVE* time?

AFAICS, the user who has the confusion that you mention here is simply
wrong. If they are looking at a field called "last active time" and
the slot is active, then the correct answer is "right now" or
"undefined" and that is what they will see. Sure, they might not
understand that. But flipping the name of the field on its head cannot
be the right way to help them.

With the current naming, I expect to have the exact opposite confusion
as your hypothetical confused user. I'm going to be looking at a slot
that's currently inactive, and it's going to tell me that the
last_inactive_time was at some time in the past. And I'm going to say
"what the heck is going on here, the slot is inactive *right now*!"

Half of me wonders whether we should avoid this whole problem by
renaming it to something like last_state_change or
last_state_change_time, or maybe just state_change like we do in
pg_stat_activity, and making it mean the last time the slot flipped
between active and inactive in either direction. I'm not sure if this
is better, but unless I'm misunderstanding something, the current
situation is terrible.

--=20
Robert Haas
EDB: http://www.enterprisedb.com