Re: IDLE in transaction introspection - Mailing list pgsql-hackers

On Wed, Nov 16, 2011 at 4:09 PM, Scott Mead <scottm@openscg.com> wrote:

On Tue, Nov 15, 2011 at 1:18 PM, Robert Treat <rob@xzilla.net> wrote:

On Tue, Nov 15, 2011 at 12:00 PM, Greg Smith <greg@2ndquadrant.com> wrote:
> On 11/15/2011 09:44 AM, Scott Mead wrote:
>>
>> Fell off the map last week, here's v2 that:
>>  * RUNNING => active
>>  * all states from CAPS to lower case
>>
>
> This looks like what I was hoping someone would add here now.  Patch looks
> good, only issue I noticed was a spaces instead of a tab goof where you set
> beentry_>st_state at line 2419 in src/backend/postmaster/pgstat.c
>
> Missing pieces:
>
> -There is one regression test that uses pg_stat_activity that is broken now.
> -The documentation should list the new field and all of the states it might
> include.  That's a serious doc update from the minimal info available right
> now.

I'm working on the doc update now, and just realized something interesting:

 My patch doesn't take the 'query_start' column into account.  Basically, the query_start column actually corresponds to the most recent update of the 'state' field.  It'd be easy enough to tie it to the 'query' field so that we are hooked in.  The problem is, if I'm monitoring this, now I don't know how long I've been 'idle in transaction' for, I would only know that the last query started at 'query_start'.  

AFAICS:

*) Adjust the query_start column to point only to the actual 'query' start time

*) Allow the query_start column to be updated as part of the 'state' change

*) Add a 'state_change' column (timestamp)

Looking for opinions here...

 

--

 Scott Mead

  OpenSCG, http://www.openscg.com

>
> I know this issue has been beat up already some, but let me summarize and
> extend that thinking a moment.  I see two equally valid schools of thought
> on how exactly to deal with introducing this change:
>
> -Add the new state field just as you've done it, but keep updating the query
> text anyway.  Do not rename current_query.  Declare the overloading of
> current_query as both a state and the query text to be deprecated in 9.3.
>  This would keep existing tools working fine against 9.2 and give a clean
> transition period.
>
> -Forget about backward compatibility and just put all the breaking stuff
> we've been meaning to do in here.  If we're going to rename current_query to
> query--what Scott's patch does here--that will force all code using
> pg_stat_activity to be rewritten.  This seems like the perfect time to also
> change "procpid" to "pid", finally blow away that wart.
>

+1

+1 for me as well.

 Anybody else?

 

> I'll happily update all of the tools and samples I deal with to support this
> change.  Most of the ones I can think of will be simplified; they're already
> parsing query_text and extracting the implicit state.  Just operating on an
> explicit one instead will be simpler and more robust.
>

lmk if you need help, we'll be doing this with some of our tools /
projects anyway, it probably wouldn't hurt to coordinate.

Robert Treat
conjecture: xzilla.net
consulting: omniti.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers