From: | Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | James Coleman <jtc331(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Add last_commit_lsn to pg_stat_database |
Date: | 2024-02-19 09:26:43 |
Message-ID: | 09070aaf-c3d4-4fb2-8d60-02003b8053ba@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2/19/24 07:57, Michael Paquier wrote:
> On Sun, Feb 18, 2024 at 02:28:06AM +0100, Tomas Vondra wrote:
>> Thanks for the updated patch. I don't have a clear opinion on the
>> feature and whether this is the way to implement it, but I have two
>> simple questions.
>
> Some users I know of would be really happy to be able to get this
> information for each database in a single view, so that feels natural
> to plug the information into pg_stat_database.
>
OK
>> 1) Do we really need to modify RecordTransactionCommitPrepared and
>> XactLogCommitRecord to return the LSN of the commit record? Can't we
>> just use XactLastRecEnd? It's good enough for advancing
>> replorigin_session_origin_lsn, why wouldn't it be good enough for this?
>
> Or XactLastCommitEnd?
But that's not set in RecordTransactionCommitPrepared (or twophase.c in
general), and the patch seems to need that.
> The changes in AtEOXact_PgStat() are not really
> attractive for what's a static variable in all the backends.
>
I'm sorry, I'm not sure what "changes not being attractive" means :-(
>> 2) Earlier in this thread it was claimed the function returning the
>> last_commit_lsn is STABLE, but I have to admit it's not clear to me why
>> that's the case :-( All the pg_stat_get_db_* functions are marked as
>> stable, so I guess it's thanks to the pgstat system ...
>
> The consistency of the shared stats data depends on
> stats_fetch_consistency. The default of "cache" makes sure that the
> values don't change across a scan, until the end of a transaction.
>
> I have not paid much attention about that until now, but note that it
> would not be the case of "none" where the data is retrieved each time
> it is requested. So it seems to me that these functions should be
> actually volatile, not stable, because they could deliver different
> values across the same scan as an effect of the design behind
> pgstat_fetch_entry() and a non-default stats_fetch_consistency. I may
> be missing something, of course.
Right. If I do this:
create or replace function get_db_lsn(oid) returns pg_lsn as $$
declare
v_lsn pg_lsn;
begin
select last_commit_lsn into v_lsn from pg_stat_database
where datid = $1;
return v_lsn;
end; $$ language plpgsql;
select min(l), max(l) from (select i, get_db_lsn(16384) AS l from
generate_series(1,100000) s(i)) foo;
and run this concurrently with a pgbench on the same database (with OID
16384), I get:
- stats_fetch_consistency=cache: always the same min/max value
- stats_fetch_consistency=none: min != max
Which would suggest you're right and this should be VOLATILE and not
STABLE. But then again - the existing pg_stat_get_db_* functions are all
marked as STABLE, so (a) is that correct and (b) why should this
function be marked different?
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From | Date | Subject | |
---|---|---|---|
Next Message | Daniel Gustafsson | 2024-02-19 09:27:58 | numeric_big in make check? |
Previous Message | Andrey M. Borodin | 2024-02-19 09:14:05 | Re: Transaction timeout |