From: | Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Exposing the stats snapshot timestamp to SQL |
Date: | 2015-02-20 02:08:54 |
Message-ID: | 54E69736.6060509@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 20.2.2015 02:58, Tom Lane wrote:
> Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> writes:
>> I see the patch only works with the top-level snapshot timestamp,
>> stored in globalStats, but since 9.3 (when the stats were split
>> into per-db files) we track per-database timestamps too.
>
>> Shouldn't we make those timestamps accessible too? It's not
>> necessary for detecting unresponsive statistics collector (if it's
>> stuck it's not writing anything, so the global timestamp will be
>> old too), but it seems more appropriate for querying database-level
>> stats to query database-level timestamp too.
>
> I'm inclined to say not; I think that's exposing an implementation
> detail that we might regret exposing, later. (IOW, it's not hard to
> think of rearrangements that might mean there wasn't a per-database
> stamp anymore.)
Fair point, but isn't the global timestamp an implementation detail too?
Although we're less likely to remove the global timestamp, no doubt
about that ...
>> But maybe that's not necessary, because to query database stats you
>> have to be connected to that particular database and that should
>> write fresh stats, so the timestamps should not be very different.
>
> Yeah. The only use-case that's been suggested is detecting an
> unresponsive stats collector, and the main timestamp should be plenty
> for that.
Well, the patch also does this:
*** 28,34 **** SELECT pg_sleep_for('2 seconds');
CREATE TEMP TABLE prevstats AS
SELECT t.seq_scan, t.seq_tup_read, t.idx_scan, t.idx_tup_fetch,
(b.heap_blks_read + b.heap_blks_hit) AS heap_blks,
! (b.idx_blks_read + b.idx_blks_hit) AS idx_blks
FROM pg_catalog.pg_stat_user_tables AS t,
pg_catalog.pg_statio_user_tables AS b
WHERE t.relname='tenk2' AND b.relname='tenk2';
--- 28,35 ----
CREATE TEMP TABLE prevstats AS
SELECT t.seq_scan, t.seq_tup_read, t.idx_scan, t.idx_tup_fetch,
(b.heap_blks_read + b.heap_blks_hit) AS heap_blks,
! (b.idx_blks_read + b.idx_blks_hit) AS idx_blks,
! pg_stat_snapshot_timestamp() as snap_ts
FROM pg_catalog.pg_stat_user_tables AS t,
pg_catalog.pg_statio_user_tables AS b
WHERE t.relname='tenk2' AND b.relname='tenk2';
***************
i.e. it adds the timestamp into queries selecting from database-level
catalogs. But OTOH we're usually using now() here, so the global
snapshot is an improvement here, and if the collector is stuck it's not
going to update of the timestamps.
So I'm OK with this patch.
--
Tomas Vondra http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | David Steele | 2015-02-20 02:35:02 | Re: pg_upgrade and rsync |
Previous Message | Tom Lane | 2015-02-20 01:58:11 | Re: Exposing the stats snapshot timestamp to SQL |