Re: [PROPOSAL] timestamp informations to pg_stat_statements - Mailing list pgsql-hackers

From Jason Kim
Subject Re: [PROPOSAL] timestamp informations to pg_stat_statements
Date
Msg-id 1468853471792-5912461.post@n5.nabble.com
Whole thread Raw
In response to Re: [PROPOSAL] timestamp informations to pg_stat_statements  (Peter Geoghegan <pg@heroku.com>)
List pgsql-hackers
Hi guys,
Thank you for feedbacks.

> I think that this is the job of a tool that aggregates things from 
> pg_stat_statements. It's unfortunate that there isn't a good 
> third-party tool that does that, but there is nothing that prevents 
> it. 

Right. We can do this if we aggregate it frequently enough. However,
third-parties can do it better if we have more informations.
I think these are fundamental informations to strong third-parties could be.

> The concern I've got about this proposal is that the results get very 
> questionable as soon as we start dropping statement entries for lack 
> of space.  last_executed would be okay, perhaps, but first_executed 
> not so much. 

If we set pg_stat_statements.max to large enough, there could be long
lived entries and short lived ones simultaneously. In this case, per call 
statistics could be accurate but per seconds stats can not.
The idea of I named it as created and last_updated (not
first_executed and last_executed) was this. It represents timestamp ofstats are created and updated, so we can
calculateper second stats with
 
simple math.

> Also, for what it's worth, I should point out to Jun that 
> GetCurrentTimestamp() should definitely not be called when a spinlock 
> is held like that. 

I moved it outside of spinlock.

@@ -1204,6 +1209,11 @@ pgss_store(const char *query, uint32 queryId,    */   volatile pgssEntry *e = (volatile
pgssEntry*) entry;
 

+   /*
+    * Read a timestamp before grab the spinlock
+    */
+   TimestampTz last_updated = GetCurrentTimestamp();
+   SpinLockAcquire(&e->mutex);
   /* "Unstick" entry if it was previously sticky */
@@ -1251,6 +1261,7 @@ pgss_store(const char *query, uint32 queryId,   e->counters.blk_read_time +=
INSTR_TIME_GET_MILLISEC(bufusage->blk_read_time);   e->counters.blk_write_time +=
INSTR_TIME_GET_MILLISEC(bufusage->blk_write_time);   e->counters.usage += USAGE_EXEC(total_time);
+   e->counters.last_updated = last_updated;
   SpinLockRelease(&e->mutex); }

pg_stat_statements_with_timestamp_v2.patch
<http://postgresql.nabble.com/file/n5912461/pg_stat_statements_with_timestamp_v2.patch>  

Regards,
Jason Kim.



--
View this message in context:
http://postgresql.nabble.com/PROPOSAL-timestamp-informations-to-pg-stat-statements-tp5912306p5912461.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.



pgsql-hackers by date:

Previous
From: Jan Wieck
Date:
Subject: Re: DO with a large amount of statements get stuck with high memory consumption
Next
From: Robert Haas
Date:
Subject: Re: rethinking dense_alloc (HashJoin) as a memory context