Re: IDLE in transaction introspection

On Wed, Nov 16, 2011 at 4:09 PM, Scott Mead <scottm(at)openscg(dot)com> wrote:

>
>
> On Tue, Nov 15, 2011 at 1:18 PM, Robert Treat <rob(at)xzilla(dot)net> wrote:
>
>> On Tue, Nov 15, 2011 at 12:00 PM, Greg Smith <greg(at)2ndquadrant(dot)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(at)postgresql(dot)org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgsql-hackers
>>
>
>