Date: Mon, 25 Mar 2024 16:12:29 +0000
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Robert Haas <robertmhaas@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>
Subject: Re: pgsql: Track last_inactive_time in pg_replication_slots.
Message-ID: <ZgGibamdXfvyqaZ5@ip-10-97-1-34.eu-west-3.compute.internal>
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>
 <CA+TgmoaY6RvsFM39B0QXr2pTpSm8p2NovRLkYizFJLLyRYWN0A@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CA+TgmoaY6RvsFM39B0QXr2pTpSm8p2NovRLkYizFJLLyRYWN0A@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/ZgGibamdXfvyqaZ5%40ip-10-97-1-34.eu-west-3.compute.internal>
Precedence: bulk

Hi,

On Mon, Mar 25, 2024 at 11:49:00AM -0400, Robert Haas wrote:
> On Mon, Mar 25, 2024 at 11:16 AM 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 one 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.
> 

Now that I read your arguments I think that last_<active|inactive>_time could be
both missleading because at the end they rely on users "expectation".

Would "released_time" sounds better? (at the end this is exactly what it does 
represent unless for the case where it is restored from disk for which the meaning
would still makes sense to me though). It seems to me that released_time does not
lead to any expectation then removing any confusion.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com