Re: IDLE in transaction introspection

From: Scott Mead <scottm(at)openscg(dot)com>
To: Robert Treat <rob(at)xzilla(dot)net>
Cc: Greg Smith <greg(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: IDLE in transaction introspection
Date: 2011-11-19 01:55:40
Message-ID: CAKq0gvLtL=2fZbDqzTdTZgvU-49dt10tWZYR3w7vHP8LE+qV0A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Nov 17, 2011 at 11:58 AM, Scott Mead <scottm(at)openscg(dot)com> wrote:

> 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.
>>>
>>
V3 Attached:

* Updates Documentation
-- Adds a separate table to cover each column of pg_stat_activity

* Adds 'state_change' (timestamp) column
-- Tracks when the 'state' column is updated independently
of when the 'query' column is updated

* renames procpid => pid

* Bug Fixes
-- If a backend had never before issued a query, another
session looking at pg_stat_activity would print <command string not
enabled>
in the query column.
-- query_start was being updated on a 'state' change, now only updated
on query
change

* Fixed 'rules' regression failure
-- Added new columns for pg_stat_activity

--
Scott Mead
OpenSCG, http://www.openscg.com

>
> 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
>>>
>>
>>
>

Attachment Content-Type Size
pg_stat_activity_state_query.v3.patch application/octet-stream 56.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2011-11-19 02:12:52 Re: EXPLAIN (plan off, rewrite off) for benchmarking
Previous Message Robert Haas 2011-11-19 01:03:01 Re: testing ProcArrayLock patches