Re: [HACKERS] IDLE in transaction introspection

2012-01-20 Thread Magnus Hagander
On Thu, Jan 19, 2012 at 15:42, Fujii Masao masao.fu...@gmail.com wrote:
 On Thu, Jan 19, 2012 at 10:31 PM, Magnus Hagander mag...@hagander.net wrote:
 Applied with fairly extensive modifications. I moved things around,
 switched to using enum instead of int+#define and a few things like
 that. Also changed most of the markup in the docs - I may well have
 broken some previously good language that, so if I did and someone
 spots it, please mention it :-)

 The attached patch seems to need to be committed.

Thanks, applied.


 BTW, the following change in the patch is not required, but ISTM that
 it's better to change that way.

 -SELECT pg_stat_get_backend_pid(s.backendid) AS procpid,
 -       pg_stat_get_backend_activity(s.backendid) AS current_query
 +SELECT pg_stat_get_backend_pid(s.backendid) AS pid,
 +       pg_stat_get_backend_activity(s.backendid) AS query

Agreed.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

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


Re: [HACKERS] IDLE in transaction introspection

2012-01-19 Thread Magnus Hagander
On Wed, Jan 18, 2012 at 16:35, Scott Mead sco...@openscg.com wrote:

 On Wed, Jan 18, 2012 at 8:27 AM, Magnus Hagander mag...@hagander.net
 wrote:

 On Tue, Jan 17, 2012 at 01:43, Scott Mead sco...@openscg.com wrote:
 
 
  On Mon, Jan 16, 2012 at 6:10 PM, Scott Mead sco...@openscg.com wrote:
 
 
  On Sun, Jan 15, 2012 at 4:28 AM, Greg Smith g...@2ndquadrant.com
  wrote:
 
  On 01/12/2012 11:57 AM, Scott Mead wrote:
 
  Pretty delayed, but please find the attached patch that addresses all
  the issues discussed.
 
 
  The docs on this v4 look like they suffered a patch order problem
  here.
   In the v3, you added a whole table describing the pg_stat_activity
  documentation in more detail than before.  v4 actually tries to remove
  those
  new docs, a change which won't even apply as they don't exist
  upstream.
 
  My guess is you committed v3 to somewhere, applied the code changes
  for
  v4, but not the documentation ones.  It's easy to do that and end up
  with a
  patch that removes a bunch of docs the previous patch added.  I have
  to be
  careful to always do something like git diff origin/master to avoid
  this
  class of problem, until I got into that habit I did this sort of thing
  regularly.
 
  gak
 
 
  I did a 'backwards' diff last time.  This time around, I diff-ed off of
  a
  fresh pull of 'master' (and I did the diff in the right direction.
 
  Also includes whitespace cleanup and the pg_stat_replication (procpid
  ==
  pid) regression fix.
 

 I'm reviewing this again, and have changed a few things around. I came
 up with a question, too :-)

 Right now, if you turn off track activities, we put command string
 not enabled in the query text. Shouldn't this also be a state, such
 as disabled? It seems more consistent to me...


 Sure, I was going the route of 'client_addr', i.e. leaving it null when not
 in use just to keep screen clutter down, but I'm not married to it.

Applied with fairly extensive modifications. I moved things around,
switched to using enum instead of int+#define and a few things like
that. Also changed most of the markup in the docs - I may well have
broken some previously good language that, so if I did and someone
spots it, please mention it :-)

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

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


Re: [HACKERS] IDLE in transaction introspection

2012-01-19 Thread Fujii Masao
On Thu, Jan 19, 2012 at 10:31 PM, Magnus Hagander mag...@hagander.net wrote:
 Applied with fairly extensive modifications. I moved things around,
 switched to using enum instead of int+#define and a few things like
 that. Also changed most of the markup in the docs - I may well have
 broken some previously good language that, so if I did and someone
 spots it, please mention it :-)

The attached patch seems to need to be committed.

BTW, the following change in the patch is not required, but ISTM that
it's better to change that way.

-SELECT pg_stat_get_backend_pid(s.backendid) AS procpid,
-   pg_stat_get_backend_activity(s.backendid) AS current_query
+SELECT pg_stat_get_backend_pid(s.backendid) AS pid,
+   pg_stat_get_backend_activity(s.backendid) AS query

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


noname
Description: Binary data

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


Re: [HACKERS] IDLE in transaction introspection

2012-01-18 Thread Magnus Hagander
On Tue, Jan 17, 2012 at 01:43, Scott Mead sco...@openscg.com wrote:


 On Mon, Jan 16, 2012 at 6:10 PM, Scott Mead sco...@openscg.com wrote:


 On Sun, Jan 15, 2012 at 4:28 AM, Greg Smith g...@2ndquadrant.com wrote:

 On 01/12/2012 11:57 AM, Scott Mead wrote:

 Pretty delayed, but please find the attached patch that addresses all
 the issues discussed.


 The docs on this v4 look like they suffered a patch order problem here.
  In the v3, you added a whole table describing the pg_stat_activity
 documentation in more detail than before.  v4 actually tries to remove those
 new docs, a change which won't even apply as they don't exist upstream.

 My guess is you committed v3 to somewhere, applied the code changes for
 v4, but not the documentation ones.  It's easy to do that and end up with a
 patch that removes a bunch of docs the previous patch added.  I have to be
 careful to always do something like git diff origin/master to avoid this
 class of problem, until I got into that habit I did this sort of thing
 regularly.

 gak


 I did a 'backwards' diff last time.  This time around, I diff-ed off of a
 fresh pull of 'master' (and I did the diff in the right direction.

 Also includes whitespace cleanup and the pg_stat_replication (procpid ==
 pid) regression fix.


I'm reviewing this again, and have changed a few things around. I came
up with a question, too :-)

Right now, if you turn off track activities, we put command string
not enabled in the query text. Shouldn't this also be a state, such
as disabled? It seems more consistent to me...

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

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


Re: [HACKERS] IDLE in transaction introspection

2012-01-18 Thread Scott Mead
On Wed, Jan 18, 2012 at 8:27 AM, Magnus Hagander mag...@hagander.netwrote:

 On Tue, Jan 17, 2012 at 01:43, Scott Mead sco...@openscg.com wrote:
 
 
  On Mon, Jan 16, 2012 at 6:10 PM, Scott Mead sco...@openscg.com wrote:
 
 
  On Sun, Jan 15, 2012 at 4:28 AM, Greg Smith g...@2ndquadrant.com
 wrote:
 
  On 01/12/2012 11:57 AM, Scott Mead wrote:
 
  Pretty delayed, but please find the attached patch that addresses all
  the issues discussed.
 
 
  The docs on this v4 look like they suffered a patch order problem here.
   In the v3, you added a whole table describing the pg_stat_activity
  documentation in more detail than before.  v4 actually tries to remove
 those
  new docs, a change which won't even apply as they don't exist upstream.
 
  My guess is you committed v3 to somewhere, applied the code changes for
  v4, but not the documentation ones.  It's easy to do that and end up
 with a
  patch that removes a bunch of docs the previous patch added.  I have
 to be
  careful to always do something like git diff origin/master to avoid
 this
  class of problem, until I got into that habit I did this sort of thing
  regularly.
 
  gak
 
 
  I did a 'backwards' diff last time.  This time around, I diff-ed off of a
  fresh pull of 'master' (and I did the diff in the right direction.
 
  Also includes whitespace cleanup and the pg_stat_replication (procpid ==
  pid) regression fix.
 

 I'm reviewing this again, and have changed a few things around. I came
 up with a question, too :-)

 Right now, if you turn off track activities, we put command string
 not enabled in the query text. Shouldn't this also be a state, such
 as disabled? It seems more consistent to me...


Sure, I was going the route of 'client_addr', i.e. leaving it null when not
in use just to keep screen clutter down, but I'm not married to it.

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


 --
  Magnus Hagander
  Me: http://www.hagander.net/
  Work: http://www.redpill-linpro.com/



Re: [HACKERS] IDLE in transaction introspection

2012-01-16 Thread Scott Mead
On Sun, Jan 15, 2012 at 4:28 AM, Greg Smith g...@2ndquadrant.com wrote:

 On 01/12/2012 11:57 AM, Scott Mead wrote:

 Pretty delayed, but please find the attached patch that addresses all the
 issues discussed.


 The docs on this v4 look like they suffered a patch order problem here.
  In the v3, you added a whole table describing the pg_stat_activity
 documentation in more detail than before.  v4 actually tries to remove
 those new docs, a change which won't even apply as they don't exist
 upstream.

 My guess is you committed v3 to somewhere, applied the code changes for
 v4, but not the documentation ones.  It's easy to do that and end up with a
 patch that removes a bunch of docs the previous patch added.  I have to be
 careful to always do something like git diff origin/master to avoid this
 class of problem, until I got into that habit I did this sort of thing
 regularly.

 gak

Okay, I'll fix that and the rules.out regression that I missed

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




 --
 Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
 PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com




Re: [HACKERS] IDLE in transaction introspection

2012-01-15 Thread Greg Smith

On 01/12/2012 11:57 AM, Scott Mead wrote:
Pretty delayed, but please find the attached patch that addresses all 
the issues discussed.


The docs on this v4 look like they suffered a patch order problem here.  
In the v3, you added a whole table describing the pg_stat_activity 
documentation in more detail than before.  v4 actually tries to remove 
those new docs, a change which won't even apply as they don't exist 
upstream.


My guess is you committed v3 to somewhere, applied the code changes for 
v4, but not the documentation ones.  It's easy to do that and end up 
with a patch that removes a bunch of docs the previous patch added.  I 
have to be careful to always do something like git diff origin/master 
to avoid this class of problem, until I got into that habit I did this 
sort of thing regularly.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com


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


Re: [HACKERS] IDLE in transaction introspection

2011-12-15 Thread Greg Smith
This patch seems closing in on being done, but it's surely going to take 
at least one more round of review to make sure all the naming and 
documentation is up right.  I can work on that again whenever Scott gets 
another version necessary, and Magnus is already poking around with an 
eye toward possibly committing the result.  I think we can make this one 
returned with feedback for this CF, but encourage Scott to resubmit as 
early as feasible before the next one.  With some good luck, we might 
even close this out before that one even starts.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us


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


Re: [HACKERS] IDLE in transaction introspection

2011-12-15 Thread Magnus Hagander
On Thu, Dec 15, 2011 at 13:18, Greg Smith g...@2ndquadrant.com wrote:
 This patch seems closing in on being done, but it's surely going to take at
 least one more round of review to make sure all the naming and documentation
 is up right.  I can work on that again whenever Scott gets another version
 necessary, and Magnus is already poking around with an eye toward possibly
 committing the result.  I think we can make this one returned with
 feedback for this CF, but encourage Scott to resubmit as early as feasible
 before the next one.  With some good luck, we might even close this out
 before that one even starts.

My hope was to get a quick update from Scott and then apply it. But
feel free to set it to returned, but Scott - don't delay until the
next CF, just post it when you're ready and I'll pick it up in between
the two CFs when I find a moment.


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

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


Re: [HACKERS] IDLE in transaction introspection

2011-12-15 Thread Scott Mead
On Thu, Dec 15, 2011 at 12:10 PM, Magnus Hagander mag...@hagander.netwrote:

 On Thu, Dec 15, 2011 at 13:18, Greg Smith g...@2ndquadrant.com wrote:
  This patch seems closing in on being done, but it's surely going to take
 at
  least one more round of review to make sure all the naming and
 documentation
  is up right.  I can work on that again whenever Scott gets another
 version
  necessary, and Magnus is already poking around with an eye toward
 possibly
  committing the result.  I think we can make this one returned with
  feedback for this CF, but encourage Scott to resubmit as early as
 feasible
  before the next one.  With some good luck, we might even close this out
  before that one even starts.

 My hope was to get a quick update from Scott and then apply it. But
 feel free to set it to returned, but Scott - don't delay until the
 next CF, just post it when you're ready and I'll pick it up in between
 the two CFs when I find a moment.


Thanks guys, I should have something by this weekend.  Sorry for the delay.

--Scott



 --
  Magnus Hagander
  Me: http://www.hagander.net/
  Work: http://www.redpill-linpro.com/



Re: [HACKERS] IDLE in transaction introspection

2011-12-12 Thread Magnus Hagander
On Wed, Dec 7, 2011 at 17:45, Scott Mead sco...@openscg.com wrote:

 On Tue, Dec 6, 2011 at 6:38 AM, Magnus Hagander mag...@hagander.net wrote:

 On Sat, Nov 19, 2011 at 02:55, Scott Mead sco...@openscg.com wrote:
 
  On Thu, Nov 17, 2011 at 11:58 AM, Scott Mead sco...@openscg.com wrote:
 
  On Wed, Nov 16, 2011 at 4:09 PM, Scott Mead sco...@openscg.com wrote:
 
  On Tue, Nov 15, 2011 at 1:18 PM, Robert Treat r...@xzilla.net wrote:
 
  On Tue, Nov 15, 2011 at 12:00 PM, Greg Smith g...@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.
 
 
  V3 Attached:
 
    * Updates Documentation
      -- Adds a separate table to cover each column of pg_stat_activity

 I like that a lot - we should've done taht years ago :-)

 For consistency, either both datname and usename should refer to their
 respective oid, or none of them.


 application_name  should probably have a link to the libpq
 documentation for said parameter.

 field is not set should probably be field is NULL


 Thanks for pointing these out.


 Boolean, if the query is waiting because of a block / lock reads
 really strange to me. Boolean indicating if would be a good start,
 but I'm not sure what you're trying to say with block / lock at all?


 Yeah, me neither.  What about:
    Boolean indicating if a query is in a wait state due to a conflict with
 another backend.

Maybe Boolean indicating if a backend is currently waiting on a lock?


 The use of single quotes in the descriptions, things like This is the
 'state' of seems wrong. If we should use anything, it should be
 double quotes, but why do we need double quotes around that? And the
 reference to think time is just strange, IMO.

 Yeah, that's a 'Scottism' (see, I used it again :-).  I'll clean that up

:-)


 In some other cases, the single quotes should probably be replaced
 with literal.

 NB: should probably be note.


 I am actually looking for some helping in describing the FASTPATH
 function call state.  Any takers?

Not sure how detailed you have to be - since it's a fairly uncommon
thing, you can probalby get away with something as simple as
executing a fast-path function or something like that?

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

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


Re: [HACKERS] IDLE in transaction introspection

2011-12-07 Thread Scott Mead
On Tue, Dec 6, 2011 at 6:38 AM, Magnus Hagander mag...@hagander.net wrote:

 On Sat, Nov 19, 2011 at 02:55, Scott Mead sco...@openscg.com wrote:
 
  On Thu, Nov 17, 2011 at 11:58 AM, Scott Mead sco...@openscg.com wrote:
 
  On Wed, Nov 16, 2011 at 4:09 PM, Scott Mead sco...@openscg.com wrote:
 
 
 
  On Tue, Nov 15, 2011 at 1:18 PM, Robert Treat r...@xzilla.net wrote:
 
  On Tue, Nov 15, 2011 at 12:00 PM, Greg Smith g...@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.
 
 
  V3 Attached:
 
* Updates Documentation
  -- Adds a separate table to cover each column of pg_stat_activity

 I like that a lot - we should've done taht years ago :-)

 For consistency, either both datname and usename should refer to their
 respective oid, or none of them.


 application_name  should probably have a link to the libpq
 documentation for said parameter.

 field is not set should probably be field is NULL


Thanks for pointing these out.


 Boolean, if the query is waiting because of a block / lock reads
 really strange to me. Boolean indicating if would be a good start,
 but I'm not sure what you're trying to say with block / lock at all?


Yeah, me neither.  What about:
   Boolean indicating if a query is in a wait state due to a conflict with
another backend.



 The way the list of states is built up looks really strange. I think
 it should be built out of variablelist like other such places
 instead - but I admit to not having checked what that actually looks
 like in the output.


Agreed.  I'll look at variablelist


 The use of single quotes in the descriptions, things like This is the
 'state' of seems wrong. If we should use anything, it should be
 double quotes, but why do we need double quotes around that? And the
 reference to think time is just strange, IMO.

 Yeah, that's a 'Scottism' (see, I used it again :-).  I'll clean that up


 In some other cases, the single quotes should probably be replaced
 with literal.

 NB: should probably be note.


I am actually looking for some helping in describing the FASTPATH
function call state.  Any takers?




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

 Very useful.


* renames procpid = pid

 Shouldn't we do this consistently if we do it? It's change in
 pg_stat_get_activity() but not pg_stat_get_wal_senders()? I think we
 either change in both functions, or none of them


Agreed



* 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

 Also, I think state=-1 needs a symbolic value. STATE_UNDEFINED or
 something like that.


Agreed.



 There's also a lot of random trailing whitespace in the patch - git
 diff (which you seem to be using already, that's why I mention it)
 will happily alert you about that - they should be removed. Or the
 committer can clean that up of course, but it makes for less work ;)

 Thanks for pointing that out.



 The code is starting to look really good,  but I think the docs
 definitely need another round of work.


Yeah, I figured they would.  Thanks for going through them!


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

--
  Magnus Hagander
  Me: http://www.hagander.net/
  Work: http://www.redpill-linpro.com/



Re: [HACKERS] IDLE in transaction introspection

2011-12-06 Thread Magnus Hagander
On Sat, Nov 19, 2011 at 02:55, Scott Mead sco...@openscg.com wrote:

 On Thu, Nov 17, 2011 at 11:58 AM, Scott Mead sco...@openscg.com wrote:

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



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

 On Tue, Nov 15, 2011 at 12:00 PM, Greg Smith g...@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.


 V3 Attached:

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

I like that a lot - we should've done taht years ago :-)

For consistency, either both datname and usename should refer to their
respective oid, or none of them.

application_name  should probably have a link to the libpq
documentation for said parameter.

field is not set should probably be field is NULL

Boolean, if the query is waiting because of a block / lock reads
really strange to me. Boolean indicating if would be a good start,
but I'm not sure what you're trying to say with block / lock at all?

The way the list of states is built up looks really strange. I think
it should be built out of variablelist like other such places
instead - but I admit to not having checked what that actually looks
like in the output.

The use of single quotes in the descriptions, things like This is the
'state' of seems wrong. If we should use anything, it should be
double quotes, but why do we need double quotes around that? And the
reference to think time is just strange, IMO.

In some other cases, the single quotes should probably be replaced
with literal.

NB: should probably be note.



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

Very useful.


   * renames procpid = pid

Shouldn't we do this consistently if we do it? It's change in
pg_stat_get_activity() but not pg_stat_get_wal_senders()? I think we
either change in both functions, or none of them


   * 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

Also, I think state=-1 needs a symbolic value. STATE_UNDEFINED or
something like that.

There's also a lot of random trailing whitespace in the patch - git
diff (which you seem to be using already, that's why I mention it)
will happily alert you about that - they should be removed. Or the
committer can clean that up of course, but it makes for less work ;)


The code is starting to look really good,  but I think the docs
definitely need another round of work.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-17 Thread Scott Mead
On Wed, Nov 16, 2011 at 4:09 PM, Scott Mead sco...@openscg.com wrote:



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

 On Tue, Nov 15, 2011 at 12:00 PM, Greg Smith g...@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





Re: [HACKERS] IDLE in transaction introspection

2011-11-16 Thread Scott Mead
On Tue, Nov 15, 2011 at 1:18 PM, Robert Treat r...@xzilla.net wrote:

 On Tue, Nov 15, 2011 at 12:00 PM, Greg Smith g...@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 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



Re: [HACKERS] IDLE in transaction introspection

2011-11-15 Thread Scott Mead
On Thu, Nov 10, 2011 at 2:49 PM, Tom Lane t...@sss.pgh.pa.us wrote:

 Bruce Momjian br...@momjian.us writes:
  Well, we could use an optional details string for that.  If not, we
  are still using the magic-string approach, which I thought we didn't
  like.

 No, we're not using magic strings, we're using an enum --- maybe not an
 officially declared enum type, but it's a column with a predetermined
 set of possible values.  It would be a magic string if it were still in
 the query field and thus confusable with user-written queries.


Fell off the map last week, here's v2 that:
 * RUNNING = active
 * all states from CAPS to lower case

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


regards, tom lane



pg_stat_activity_state_query.v2.patch
Description: Binary data

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-15 Thread Greg Smith

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


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.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us


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


Re: [HACKERS] IDLE in transaction introspection

2011-11-15 Thread Robert Treat
On Tue, Nov 15, 2011 at 12:00 PM, Greg Smith g...@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 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

 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


Re: [HACKERS] IDLE in transaction introspection

2011-11-10 Thread Scott Mead
On Nov 5, 2011 9:02 AM, Greg Smith g...@2ndquadrant.com wrote:

 On 11/04/2011 05:01 PM, Tom Lane wrote:

 Scott Meadsco...@openscg.com  writes:


I leave the waiting flag in place for posterity.  With this in mind,
is
 the consensus:
RUNNING
 or
ACTIVE


 Personally, I'd go for lower case.



 I was thinking it would be nice if this state looked like the WAL sender
state values in pg_stat_replication, which are all lower case.  For
comparison those states are:

 startup
 backup
 catchup
 streaming

+1, it'll be easier to query against.

 --
 Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
 PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us



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


Re: [HACKERS] IDLE in transaction introspection

2011-11-10 Thread Bruce Momjian
Scott Mead wrote:
 On Wed, Nov 2, 2011 at 4:12 AM, Albe Laurenz laurenz.a...@wien.gv.atwrote:
 
  Andrew Dunstan wrote:
   On 11/01/2011 09:52 AM, Tom Lane wrote:
   I'm for just redefining the query field as current or last
   query.
  
   +1
  
   I could go either way on whether to rename it.
  
   Rename it please. current_query will just be wrong. I'd be inclined
   just to call it query or query_string and leave it to the docs to
   define the exact semantics.
 
  +1 for renaming, +1 for a state column.
  I think it is overkill to keep a query history beyond that -- if you
  want that,
  you can resort to the log files.
 
 
 ISTM that we're all for:
 
creating a new column: state
renaming current_query = query
 
State will display RUNNING, IDLE, IDLE in transaction, etc...
query will display the last query that was executed.
 
 I've written this up in the attached patch, looking for feedback. (NB:
 Originally I was using 9.1.1 release, I just did a git clone today to
 generate this).

It might be cleaner to use booleans:

active: t/f
in transaction: t/f

or maybe instead of 'active':

idle:   t/f
in transaction: t/f

That avoids the magic string values for the state column.  Those are
much easier to query against too:

WHERE NOT idle;

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-10 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 It might be cleaner to use booleans:
   active: t/f
   in transaction: t/f

I don't think so, because that makes some very strict assumptions that
there are exactly four interesting states (an assumption that isn't
even true today, to judge by the activity strings we're using now).

regards, tom lane

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-10 Thread Bruce Momjian
Tom Lane wrote:
 Bruce Momjian br...@momjian.us writes:
  It might be cleaner to use booleans:
  active: t/f
  in transaction: t/f
 
 I don't think so, because that makes some very strict assumptions that
 there are exactly four interesting states (an assumption that isn't
 even true today, to judge by the activity strings we're using now).

Well, we could use an optional details string for that.  If not, we
are still using the magic-string approach, which I thought we didn't
like.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-10 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 Well, we could use an optional details string for that.  If not, we
 are still using the magic-string approach, which I thought we didn't
 like.

No, we're not using magic strings, we're using an enum --- maybe not an
officially declared enum type, but it's a column with a predetermined
set of possible values.  It would be a magic string if it were still in
the query field and thus confusable with user-written queries.

regards, tom lane

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-05 Thread Greg Smith

On 11/04/2011 05:01 PM, Tom Lane wrote:

Scott Meadsco...@openscg.com  writes:
   

I leave the waiting flag in place for posterity.  With this in mind, is
the consensus:
RUNNING
 or
ACTIVE
 

Personally, I'd go for lower case.
   


I was thinking it would be nice if this state looked like the WAL sender 
state values in pg_stat_replication, which are all lower case.  For 
comparison those states are:


startup
backup
catchup
streaming

--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us


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


Re: [HACKERS] IDLE in transaction introspection

2011-11-04 Thread Magnus Hagander
On Fri, Nov 4, 2011 at 03:27, Fujii Masao masao.fu...@gmail.com wrote:
 On Thu, Nov 3, 2011 at 3:18 AM, Scott Mead sco...@openscg.com wrote:
 ISTM that we're all for:
    creating a new column: state
    renaming current_query = query
    State will display RUNNING, IDLE, IDLE in transaction, etc...
    query will display the last query that was executed.

 The greater/less-than-sign is still required in the State display?

+1 for removing that. I think the only reason for them to be there in
the first place was to decrease the chance of matching up against a
real query - but that's not going to happen if they are in a separate
field...

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-04 Thread Marti Raudsepp
On Wed, Nov 2, 2011 at 20:18, Scott Mead sco...@openscg.com wrote:
    State will display RUNNING, IDLE, IDLE in transaction, etc...

While we're already breaking everything, we could remove the waiting
column and use a state with value 'waiting' instead.

Also, returning these as text seems a little lame. Should there be an
enum type for that?

Or we could return single-character mnemonics like some pg_catalog
tables do, and substitute them in the view.

Regards,
Marti

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-04 Thread Tom Lane
Marti Raudsepp ma...@juffo.org writes:
 While we're already breaking everything, we could remove the waiting
 column and use a state with value 'waiting' instead.

-1 ... I think it's useful to see the underlying state as well as the
waiting flag.  Also, this would represent breakage of part of the API
that doesn't need to be broken.

 Also, returning these as text seems a little lame. Should there be an
 enum type for that?

Perhaps, but we don't really use enum types in any other system views,
so inventing one here would be out of character.

regards, tom lane

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-04 Thread Magnus Hagander
On Fri, Nov 4, 2011 at 14:42, Tom Lane t...@sss.pgh.pa.us wrote:
 Marti Raudsepp ma...@juffo.org writes:
 While we're already breaking everything, we could remove the waiting
 column and use a state with value 'waiting' instead.

 -1 ... I think it's useful to see the underlying state as well as the
 waiting flag.  Also, this would represent breakage of part of the API
 that doesn't need to be broken.

I guess with the changes that showed different thing like fastpath,
that makes sense. But if we just mapped the states that are there
today straight off, is there any case where waiting can be true, when
we're either idle or idle in transaction? I think not..


 Also, returning these as text seems a little lame. Should there be an
 enum type for that?

 Perhaps, but we don't really use enum types in any other system views,
 so inventing one here would be out of character.

Yeha, that seems inconsistent. Using a single character might work -
but it's not particularly user-friendly to do that in the view itself.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-04 Thread Robert Haas
On Fri, Nov 4, 2011 at 9:48 AM, Magnus Hagander mag...@hagander.net wrote:
 On Fri, Nov 4, 2011 at 14:42, Tom Lane t...@sss.pgh.pa.us wrote:
 Marti Raudsepp ma...@juffo.org writes:
 While we're already breaking everything, we could remove the waiting
 column and use a state with value 'waiting' instead.

 -1 ... I think it's useful to see the underlying state as well as the
 waiting flag.  Also, this would represent breakage of part of the API
 that doesn't need to be broken.

 I guess with the changes that showed different thing like fastpath,
 that makes sense. But if we just mapped the states that are there
 today straight off, is there any case where waiting can be true, when
 we're either idle or idle in transaction? I think not..

Maybe if we get a sinval reset while we're just sitting there?  Not sure.

In any case, I agree with Tom.  I think that most people will be happy
to have a query column and a state column rather than a
current_query column that amalgamates the two, but I see no reason
to tinker with the waiting column if the changes we want to make don't
otherwise require it.

 Also, returning these as text seems a little lame. Should there be an
 enum type for that?

 Perhaps, but we don't really use enum types in any other system views,
 so inventing one here would be out of character.

 Yeha, that seems inconsistent. Using a single character might work -
 but it's not particularly user-friendly to do that in the view itself.

+1 for keeping it as text, but loosing the angle brackets.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-04 Thread Scott Mead
On Fri, Nov 4, 2011 at 9:48 AM, Magnus Hagander mag...@hagander.net wrote:

 On Fri, Nov 4, 2011 at 14:42, Tom Lane t...@sss.pgh.pa.us wrote:
  Marti Raudsepp ma...@juffo.org writes:
  While we're already breaking everything, we could remove the waiting
  column and use a state with value 'waiting' instead.
 
  -1 ... I think it's useful to see the underlying state as well as the
  waiting flag.  Also, this would represent breakage of part of the API
  that doesn't need to be broken.

 I guess with the changes that showed different thing like fastpath,
 that makes sense. But if we just mapped the states that are there
 today straight off, is there any case where waiting can be true, when
 we're either idle or idle in transaction? I think not..


   Leave the waiting column and display 'WAITING' if st_watiting = 1 seems
to be the clearest solution.  I can see people getting confused by waiting
= 't' and state='RUNNING'.




  Also, returning these as text seems a little lame. Should there be an
  enum type for that?
 
  Perhaps, but we don't really use enum types in any other system views,
  so inventing one here would be out of character.

 Yeha, that seems inconsistent. Using a single character might work -
 but it's not particularly user-friendly to do that in the view itself.


 I'll nuke the '', which is definitely an improvement, anything more
complex seems like it'll require fairly wordy documentation.

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



 --
  Magnus Hagander
  Me: http://www.hagander.net/
  Work: http://www.redpill-linpro.com/

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



Re: [HACKERS] IDLE in transaction introspection

2011-11-04 Thread Tom Lane
Magnus Hagander mag...@hagander.net writes:
 I guess with the changes that showed different thing like fastpath,
 that makes sense. But if we just mapped the states that are there
 today straight off, is there any case where waiting can be true, when
 we're either idle or idle in transaction? I think not..

I'm not totally sure about that.  I know that it's possible to see idle
waiting in the ps display, but that comes from getting blocked in parse
analysis before the command type has been determined.  The
pg_stat_activity string is set a bit earlier, so *maybe* it's impossible
there.  Still, why wire such an assumption into the structure of the
view?  Robert's point about sinval catchup is another good one, though
I don't remember what that does to the pg_stat_activity display.

BTW, a quick grep shows that there are four not two special values of
the activity string today:

src/backend/tcop/postgres.c: 3792: 
pgstat_report_activity(IDLE in transaction (aborted));
src/backend/tcop/postgres.c: 3797: 
pgstat_report_activity(IDLE in transaction);
src/backend/tcop/postgres.c: 3805: 
pgstat_report_activity(IDLE);
src/backend/tcop/postgres.c: 3925: 
pgstat_report_activity(FASTPATH function call);

It's also worth considering whether the autovacuum: that's prepended
by autovac_report_activity() ought to be folded into the state instead
of continuing to put something that's not valid SQL into the query
string.

regards, tom lane

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-04 Thread Robert Haas
On Fri, Nov 4, 2011 at 10:34 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert's point about sinval catchup is another good one, though
 I don't remember what that does to the pg_stat_activity display.

My thought was that sinval catchup might require acquiring a relation
lock (e.g. on pg_class), and that might block waiting for a lock.

Not sure it's possible, but even if it can't happen today, it doesn't
seem impossible that we might want to let it happen in the future.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-04 Thread Marti Raudsepp
On Fri, Nov 4, 2011 at 15:42, Tom Lane t...@sss.pgh.pa.us wrote:
 Marti Raudsepp ma...@juffo.org writes:
 While we're already breaking everything, we could remove the waiting
 column and use a state with value 'waiting' instead.

 -1 ... I think it's useful to see the underlying state as well as the
 waiting flag.  Also, this would represent breakage of part of the API
 that doesn't need to be broken.

Well the waiting column can stay. My concern is that listing lock-wait
backends as 'running' will be misleading for users. pg_stat_activity
is a pretty common starting point for debugging problems and if
there's a new column that says a query is 'running', then I'm afraid
the current waiting 't' and 'f' values will be too subtle for users to
notice. (I find that it's too subtle already now, if you don't know
what you're looking for).

Regards,
Marti

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-04 Thread Robert Haas
On Fri, Nov 4, 2011 at 12:46 PM, Marti Raudsepp ma...@juffo.org wrote:
 On Fri, Nov 4, 2011 at 15:42, Tom Lane t...@sss.pgh.pa.us wrote:
 Marti Raudsepp ma...@juffo.org writes:
 While we're already breaking everything, we could remove the waiting
 column and use a state with value 'waiting' instead.

 -1 ... I think it's useful to see the underlying state as well as the
 waiting flag.  Also, this would represent breakage of part of the API
 that doesn't need to be broken.

 Well the waiting column can stay. My concern is that listing lock-wait
 backends as 'running' will be misleading for users. pg_stat_activity
 is a pretty common starting point for debugging problems and if
 there's a new column that says a query is 'running', then I'm afraid
 the current waiting 't' and 'f' values will be too subtle for users to
 notice. (I find that it's too subtle already now, if you don't know
 what you're looking for).

Maybe there's a better term than running, like in progress or
something of that sort.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-04 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 Maybe there's a better term than running, like in progress or
 something of that sort.

active?

regards, tom lane

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-04 Thread Robert Haas
On Fri, Nov 4, 2011 at 2:28 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 Maybe there's a better term than running, like in progress or
 something of that sort.

 active?

+1.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-04 Thread Scott Mead
On Fri, Nov 4, 2011 at 2:31 PM, Robert Haas robertmh...@gmail.com wrote:

 On Fri, Nov 4, 2011 at 2:28 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  Robert Haas robertmh...@gmail.com writes:
  Maybe there's a better term than running, like in progress or
  something of that sort.
 
  active?

 +1.

Letting this one 'poll' a bit more before I post the patch, but here's
what I have:

   If waiting == true, then state = WAITING
   else
 state = appropriate state

   I leave the waiting flag in place for posterity.  With this in mind, is
the consensus:

   RUNNING

or

   ACTIVE

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


--
 Robert Haas
 EnterpriseDB: http://www.enterprisedb.com
 The Enterprise PostgreSQL Company



Re: [HACKERS] IDLE in transaction introspection

2011-11-04 Thread Tom Lane
Scott Mead sco...@openscg.com writes:
I leave the waiting flag in place for posterity.  With this in mind, is
 the consensus:
RUNNING
 or
ACTIVE

Personally, I'd go for lower case.

regards, tom lane

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-04 Thread Robert Haas
On Fri, Nov 4, 2011 at 2:46 PM, Scott Mead sco...@openscg.com wrote:
    If waiting == true, then state = WAITING
    else
      state = appropriate state

No, I think the state and waiting columns should be completely independent.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-04 Thread Robert Treat
On Fri, Nov 4, 2011 at 3:28 PM, Robert Haas robertmh...@gmail.com wrote:
 On Fri, Nov 4, 2011 at 2:46 PM, Scott Mead sco...@openscg.com wrote:
    If waiting == true, then state = WAITING
    else
      state = appropriate state

 No, I think the state and waiting columns should be completely independent.


If they aren't I don't think we need both columns. +1 for leaving them
independent though.

Robert Treat
play: xzilla.net
work: 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


Re: [HACKERS] IDLE in transaction introspection

2011-11-04 Thread Robert Treat
On Fri, Nov 4, 2011 at 10:34 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Magnus Hagander mag...@hagander.net writes:
 I guess with the changes that showed different thing like fastpath,
 that makes sense. But if we just mapped the states that are there
 today straight off, is there any case where waiting can be true, when
 we're either idle or idle in transaction? I think not..

 I'm not totally sure about that.  I know that it's possible to see idle
 waiting in the ps display, but that comes from getting blocked in parse
 analysis before the command type has been determined.  The
 pg_stat_activity string is set a bit earlier, so *maybe* it's impossible
 there.  Still, why wire such an assumption into the structure of the
 view?  Robert's point about sinval catchup is another good one, though
 I don't remember what that does to the pg_stat_activity display.

 BTW, a quick grep shows that there are four not two special values of
 the activity string today:

 src/backend/tcop/postgres.c: 3792:                 
 pgstat_report_activity(IDLE in transaction (aborted));
 src/backend/tcop/postgres.c: 3797:                 
 pgstat_report_activity(IDLE in transaction);
 src/backend/tcop/postgres.c: 3805:                 
 pgstat_report_activity(IDLE);
 src/backend/tcop/postgres.c: 3925:                 
 pgstat_report_activity(FASTPATH function call);

 It's also worth considering whether the autovacuum: that's prepended
 by autovac_report_activity() ought to be folded into the state instead
 of continuing to put something that's not valid SQL into the query
 string.


Well, it would be interesting to see rows for autovacuum or
replication processes showing up in pg_stat_activity with a
corresponding state (autovacuum, walreciever?) and the query field
showing what they are working on, at the risk that we'd need to build
more complex parsing into the various monitoring scripts, but I guess
it's no worse than before (and I guess probably easier on some level).

Robert Treat
play: xzilla.net
work: 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


Re: [HACKERS] IDLE in transaction introspection

2011-11-03 Thread Fujii Masao
On Thu, Nov 3, 2011 at 3:18 AM, Scott Mead sco...@openscg.com wrote:
 ISTM that we're all for:
    creating a new column: state
    renaming current_query = query
    State will display RUNNING, IDLE, IDLE in transaction, etc...
    query will display the last query that was executed.

The greater/less-than-sign is still required in the State display?

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-02 Thread Albe Laurenz
Andrew Dunstan wrote:
 On 11/01/2011 09:52 AM, Tom Lane wrote:
 I'm for just redefining the query field as current or last
 query.

 +1
 
 I could go either way on whether to rename it.
 
 Rename it please. current_query will just be wrong. I'd be inclined
 just to call it query or query_string and leave it to the docs to
 define the exact semantics.

+1 for renaming, +1 for a state column.
I think it is overkill to keep a query history beyond that -- if you
want that,
you can resort to the log files.

Yours,
Laurenz Albe

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-02 Thread Scott Mead
On Wed, Nov 2, 2011 at 4:12 AM, Albe Laurenz laurenz.a...@wien.gv.atwrote:

 Andrew Dunstan wrote:
  On 11/01/2011 09:52 AM, Tom Lane wrote:
  I'm for just redefining the query field as current or last
  query.
 
  +1
 
  I could go either way on whether to rename it.
 
  Rename it please. current_query will just be wrong. I'd be inclined
  just to call it query or query_string and leave it to the docs to
  define the exact semantics.

 +1 for renaming, +1 for a state column.
 I think it is overkill to keep a query history beyond that -- if you
 want that,
 you can resort to the log files.


ISTM that we're all for:

   creating a new column: state
   renaming current_query = query

   State will display RUNNING, IDLE, IDLE in transaction, etc...
   query will display the last query that was executed.

I've written this up in the attached patch, looking for feedback. (NB:
Originally I was using 9.1.1 release, I just did a git clone today to
generate this).

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




 Yours,
 Laurenz Albe



pg_stat_activity_state_query.patch
Description: Binary data

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-01 Thread Magnus Hagander
On Tue, Nov 1, 2011 at 00:18, Scott Mead sco...@openscg.com wrote:


 On Mon, Oct 31, 2011 at 6:13 PM, Robert Haas robertmh...@gmail.com wrote:

 On Mon, Oct 31, 2011 at 5:45 PM, Magnus Hagander mag...@hagander.net
 wrote:
  Actually, for the future, it might be useful to have a state column,
  that holds the idle/in transaction/running status, instead of the
  tools having to parse the query text to get that information...

 +1 for doing it this way.  Splitting current_query into query and
 state would be more elegant and easier to use all around.

 I'm all for splitting it out actually.  My concern was that I would break
 the 'ba-gillion' monitoring tools that already have support for
 pg_stat_activity if I dropped a column.  What if we had:
    'state' :             idle | in transaction | running ( per Robert )

If we're going with breaking compatibility, waiting should be a
state as well, I think. Actually, it should be even if we're not
breaking compatibilty, but just adding a state field.


    'current_query' :  the most recent query (either last / currently
 running)
    That may be a bit tougher to get across to people though (especially in
 the case where state='IDLE').
  I'll rework this when I don't have trick-or-treaters coming to the front
 door :)

I think the question is how Ok it is to break compatibility. We could
always leave current_query in there and create a new field for
last_query *and* introduce a state... And then advertise a change in
the future. But that might be too much...

If we are doing it, it might be useful to just call it query, so
that it is dead obvious to people that the definition changed..

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-01 Thread Robert Haas
On Tue, Nov 1, 2011 at 7:38 AM, Magnus Hagander mag...@hagander.net wrote:
 If we are doing it, it might be useful to just call it query, so
 that it is dead obvious to people that the definition changed..

Yeah.  Otherwise, people who are parsing the hard-coded strings idle
and idle in transaction are likely to get confused.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-01 Thread Simon Riggs
On Mon, Oct 31, 2011 at 10:13 PM, Robert Haas robertmh...@gmail.com wrote:
 On Mon, Oct 31, 2011 at 5:45 PM, Magnus Hagander mag...@hagander.net wrote:
 Actually, for the future, it might be useful to have a state column,
 that holds the idle/in transaction/running status, instead of the
 tools having to parse the query text to get that information...

 +1 for doing it this way.  Splitting current_query into query and
 state would be more elegant and easier to use all around.

Why not leave it exactly as it is, and add a previous_query column?

That gives you exactly what you need without breaking anything.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-01 Thread Magnus Hagander
On Tue, Nov 1, 2011 at 13:19, Simon Riggs si...@2ndquadrant.com wrote:
 On Mon, Oct 31, 2011 at 10:13 PM, Robert Haas robertmh...@gmail.com wrote:
 On Mon, Oct 31, 2011 at 5:45 PM, Magnus Hagander mag...@hagander.net wrote:
 Actually, for the future, it might be useful to have a state column,
 that holds the idle/in transaction/running status, instead of the
 tools having to parse the query text to get that information...

 +1 for doing it this way.  Splitting current_query into query and
 state would be more elegant and easier to use all around.

 Why not leave it exactly as it is, and add a previous_query column?

 That gives you exactly what you need without breaking anything.

That would be the backwards compatible way I suggested.

That said, I think there's still value in exposing a state column,
and to encourage people not to rely on the text in the query column.
Then you can add it to your list of things to remove in 10.0 :-)

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-01 Thread Simon Riggs
On Tue, Nov 1, 2011 at 12:41 PM, Magnus Hagander mag...@hagander.net wrote:
 On Tue, Nov 1, 2011 at 13:19, Simon Riggs si...@2ndquadrant.com wrote:
 On Mon, Oct 31, 2011 at 10:13 PM, Robert Haas robertmh...@gmail.com wrote:
 On Mon, Oct 31, 2011 at 5:45 PM, Magnus Hagander mag...@hagander.net 
 wrote:
 Actually, for the future, it might be useful to have a state column,
 that holds the idle/in transaction/running status, instead of the
 tools having to parse the query text to get that information...

 +1 for doing it this way.  Splitting current_query into query and
 state would be more elegant and easier to use all around.

 Why not leave it exactly as it is, and add a previous_query column?

 That gives you exactly what you need without breaking anything.

 That would be the backwards compatible way I suggested.

Sorry Magnus, I hadn't read the whole thread.

+1 to your suggestion.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-01 Thread Robert Haas
On Tue, Nov 1, 2011 at 8:41 AM, Magnus Hagander mag...@hagander.net wrote:
 On Tue, Nov 1, 2011 at 13:19, Simon Riggs si...@2ndquadrant.com wrote:
 On Mon, Oct 31, 2011 at 10:13 PM, Robert Haas robertmh...@gmail.com wrote:
 On Mon, Oct 31, 2011 at 5:45 PM, Magnus Hagander mag...@hagander.net 
 wrote:
 Actually, for the future, it might be useful to have a state column,
 that holds the idle/in transaction/running status, instead of the
 tools having to parse the query text to get that information...

 +1 for doing it this way.  Splitting current_query into query and
 state would be more elegant and easier to use all around.

 Why not leave it exactly as it is, and add a previous_query column?

 That gives you exactly what you need without breaking anything.

 That would be the backwards compatible way I suggested.

 That said, I think there's still value in exposing a state column,
 and to encourage people not to rely on the text in the query column.
 Then you can add it to your list of things to remove in 10.0 :-)

Personally, I think we're getting a bit overexcited about backward
compatibility here.  We make backward-incompatible changes in every
release.  Things that change the behavior of user queries (like
reserving keywords, or other changes in syntax, or having the same
syntax now mean something different) cause a fair amount of user pain,
and we're justifiably cautious about making them.  But changes that
have to do with server administration, as far as I can see, result in
much less pain.  Splitting the current_query field into query and
state is going to require only the most minimal adjustment to user
code, and anyone for whom it's really a problem can easily create
their own view that mimics the old behavior.  On the flip side,
keeping both fields around is going to make the pg_stat_activity,
which is already extremely wide, even more difficult to read in a
window of any reasonable width, especially because the field we're
proposing to duplicate is the widest one by far.  I also doubt very
much that it creates a meaningful migration path; most people will
just keep doing it the old way until it breaks, and even new users may
not realize that one column is nominally deprecated.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-01 Thread Tom Lane
Simon Riggs si...@2ndquadrant.com writes:
 Why not leave it exactly as it is, and add a previous_query column?

 That gives you exactly what you need without breaking anything.

That would cost twice as much shared memory for query strings, and twice
as much time to update the strings, for what seems pretty marginal
value.  I'm for just redefining the query field as current or last
query.  I could go either way on whether to rename it.

If anyone's really hot about backward compatibility, it would not be
very hard to create a view that replicates the old behavior working
from a state column and a current-or-last-query column.

regards, tom lane

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-01 Thread Simon Riggs
On Tue, Nov 1, 2011 at 1:52 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Simon Riggs si...@2ndquadrant.com writes:
 Why not leave it exactly as it is, and add a previous_query column?

 That gives you exactly what you need without breaking anything.

 That would cost twice as much shared memory for query strings, and twice
 as much time to update the strings, for what seems pretty marginal
 value.  I'm for just redefining the query field as current or last
 query.  I could go either way on whether to rename it.

That's a good reason.

 If anyone's really hot about backward compatibility, it would not be
 very hard to create a view that replicates the old behavior working
 from a state column and a current-or-last-query column.

I'm in favour of change, when that has a purpose, just like you.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-01 Thread Robert Haas
On Tue, Nov 1, 2011 at 9:52 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Simon Riggs si...@2ndquadrant.com writes:
 Why not leave it exactly as it is, and add a previous_query column?

 That gives you exactly what you need without breaking anything.

 That would cost twice as much shared memory for query strings, and twice
 as much time to update the strings, for what seems pretty marginal
 value.  I'm for just redefining the query field as current or last
 query.

Not really.  You could just store it once in shared memory, and put
the complexity in the view definition.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-01 Thread Marti Raudsepp
On Mon, Oct 31, 2011 at 23:37, Scott Mead sco...@openscg.com wrote:
    So I wrote the attached patch, it just turns IDLE in transaction into:
  IDLE in transaction\n: Previous: last query executed.  After seeing
 how quickly our dev's fixed the issue once they saw prepared statement XYZ,

Solving this problem is a good idea, but I don't particularly like the
proposed solution. I think the proposed state/query split is going to
make pg_stat_activity more confusing, especially if even idle
connections get a query string. And if we display the last query
there, why not the one before that? etc. (Adding a state column
might not be a bad idea though)

I'd very much like to see a more generic solution: a runtime query log
facility that can be queried in any way you want. pg_stat_statements
comes close, but is limited too due to its (arbitrary, I find)
deduplication -- you can't query for 10 last statements from process
N since it has no notion of processes, just users and databases.

So far my developers are simply grepping pg_log, but you can't use the
full power of SQL there. I know there's csvlog, but the pains aren't
big enough to make attempt to use that.

Regards,
Marti

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-01 Thread Jeroen Vermeulen

On 2011-11-01 21:13, Andrew Dunstan wrote:


Rename it please. current_query will just be wrong. I'd be inclined
just to call it query or query_string and leave it to the docs to
define the exact semantics.


I think query for a query that isn't ongoing would be just as wrong. 
How about last_query?



Jeroen

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-01 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Tue, Nov 1, 2011 at 9:52 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 That would cost twice as much shared memory for query strings, and twice
 as much time to update the strings, for what seems pretty marginal
 value.  I'm for just redefining the query field as current or last
 query.

 Not really.  You could just store it once in shared memory, and put
 the complexity in the view definition.

I understood the proposal to be store the previous query in addition
to the current-query-if-any.  If that's not what was meant, then my
objection was incorrect.  However, like you, I'm pretty dubious of
having two mostly-redundant fields in the view definition, just because
of window width issues.

regards, tom lane

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-01 Thread Cédric Villemain
2011/11/1 Marti Raudsepp ma...@juffo.org:
 On Mon, Oct 31, 2011 at 23:37, Scott Mead sco...@openscg.com wrote:
    So I wrote the attached patch, it just turns IDLE in transaction into:
  IDLE in transaction\n: Previous: last query executed.  After seeing
 how quickly our dev's fixed the issue once they saw prepared statement XYZ,

 Solving this problem is a good idea, but I don't particularly like the
 proposed solution. I think the proposed state/query split is going to
 make pg_stat_activity more confusing, especially if even idle
 connections get a query string. And if we display the last query
 there, why not the one before that? etc. (Adding a state column
 might not be a bad idea though)

 I'd very much like to see a more generic solution: a runtime query log
 facility that can be queried in any way you want. pg_stat_statements
 comes close, but is limited too due to its (arbitrary, I find)
 deduplication -- you can't query for 10 last statements from process
 N since it has no notion of processes, just users and databases.

I like the idea to have a pg_stat_activity with an history, it can be
in another view with a GUC to define how many queries to keep per pid.


 So far my developers are simply grepping pg_log, but you can't use the
 full power of SQL there. I know there's csvlog, but the pains aren't
 big enough to make attempt to use that.

 Regards,
 Marti

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




-- 
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-01 Thread Magnus Hagander
On Tue, Nov 1, 2011 at 18:11, Scott Mead sco...@openscg.com wrote:

 On Tue, Nov 1, 2011 at 10:40 AM, Tom Lane t...@sss.pgh.pa.us wrote:

 Robert Haas robertmh...@gmail.com writes:
  On Tue, Nov 1, 2011 at 9:52 AM, Tom Lane t...@sss.pgh.pa.us wrote:
  That would cost twice as much shared memory for query strings, and
  twice
  as much time to update the strings, for what seems pretty marginal
  value.  I'm for just redefining the query field as current or last
  query.

  Not really.  You could just store it once in shared memory, and put
  the complexity in the view definition.

 I understood the proposal to be store the previous query in addition
 to the current-query-if-any.  If that's not what was meant, then my
 objection was incorrect.  However, like you, I'm pretty dubious of
 having two mostly-redundant fields in the view definition, just because
 of window width issues.

 The biggest reason I dislike the multi-field approach is because it limits
 us to only the [single] previous_query in the system with all the overhead
 we talked about  (memory, window width and messing with system catalogs in
 general).  That's actually why I implemented it the way I did, just by
 appending the last query on the end of the string when it's IDLE in
 transaction.

Well, by appending it in that field, you require the end
user/monitoring app to parse out the string anyway, so you're not
exactly making life easier on the consumer of the information..


 Marti wrote:

 I'd very much like to see a more generic solution: a runtime query log
 facility that can be queried in any way you want. pg_stat_statements
 comes close, but is limited too due to its (arbitrary, I find)
 deduplication -- you can't query for 10 last statements from process
 N since it has no notion of processes, just users and databases.

 This is what I'd really like to see (just haven't had time as it is a much
 bigger project).  The next question my devs ask is what were the last five
 queries that ran... can you show me an overview of an entire transaction
 etc...
   That being said, having the previous_query available feels like it fixes
 about 80% of the *problem*; transaction profiling, or looking back 10 / 15 /
 20 queries is [immensely] useful, but I find that the bigger need is the
 ability to short-circuit dba / dev back-n-forth by just saying Your app
 refused to commit/rollback after running query XYZ.

This would be great, but as you say, it's a different project.

Perhaps something could be made along the line of each backend keeping
it's *own* set of old queries, and then making it available to a
specific function (pg_get_last_queries_for_backend(nnn)) - since
this is not the type of information you'd ask for often, it would be
ok if getting this data took longer.. And you seldom want give me the
last 5 queries for each backend at once.


 Robert Wrote:
 Yeah.  Otherwise, people who are parsing the hard-coded strings idle
 and idle in transaction are likely to get confused.

 I would be interested ( and frankly very surprised ) to find out if many
 monitoring tools are actually parsing that field.  Most that I see just dump
 whatever is in current_query to the user.  I would imaging that, so long as
 the server obeyed pgstat_track_activity_size most tools would behave nicely.

Really? I'd assume every single monitoring tool that graphs how many
active connections you have (vs idle vs idle in transaction) would do
just this.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-01 Thread Scott Mead
On Tue, Nov 1, 2011 at 1:20 PM, Magnus Hagander mag...@hagander.net wrote:

 On Tue, Nov 1, 2011 at 18:11, Scott Mead sco...@openscg.com wrote:
 
  On Tue, Nov 1, 2011 at 10:40 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 
  Robert Haas robertmh...@gmail.com writes:
   On Tue, Nov 1, 2011 at 9:52 AM, Tom Lane t...@sss.pgh.pa.us wrote:
   That would cost twice as much shared memory for query strings, and
   twice
   as much time to update the strings, for what seems pretty marginal
   value.  I'm for just redefining the query field as current or last
   query.
 
   Not really.  You could just store it once in shared memory, and put
   the complexity in the view definition.
 
  I understood the proposal to be store the previous query in addition
  to the current-query-if-any.  If that's not what was meant, then my
  objection was incorrect.  However, like you, I'm pretty dubious of
  having two mostly-redundant fields in the view definition, just because
  of window width issues.
 
  The biggest reason I dislike the multi-field approach is because it
 limits
  us to only the [single] previous_query in the system with all the
 overhead
  we talked about  (memory, window width and messing with system catalogs
 in
  general).  That's actually why I implemented it the way I did, just by
  appending the last query on the end of the string when it's IDLE in
  transaction.

 Well, by appending it in that field, you require the end
 user/monitoring app to parse out the string anyway, so you're not
 exactly making life easier on the consumer of the information..


  Marti wrote:
 
  I'd very much like to see a more generic solution: a runtime query log
  facility that can be queried in any way you want. pg_stat_statements
  comes close, but is limited too due to its (arbitrary, I find)
  deduplication -- you can't query for 10 last statements from process
  N since it has no notion of processes, just users and databases.
 
  This is what I'd really like to see (just haven't had time as it is a
 much
  bigger project).  The next question my devs ask is what were the last
 five
  queries that ran... can you show me an overview of an entire
 transaction
  etc...
That being said, having the previous_query available feels like it
 fixes
  about 80% of the *problem*; transaction profiling, or looking back 10 /
 15 /
  20 queries is [immensely] useful, but I find that the bigger need is the
  ability to short-circuit dba / dev back-n-forth by just saying Your app
  refused to commit/rollback after running query XYZ.

 This would be great, but as you say, it's a different project.

 Perhaps something could be made along the line of each backend keeping
 it's *own* set of old queries, and then making it available to a
 specific function (pg_get_last_queries_for_backend(nnn))


Yeah, I was kind of thinking this too, I just feel dirty adding a (*n
* track_activity_query_size) *overhead to shared memory for tracking that
if we're already concerned about adding just a 'previous_query' string.
 It's easy enough to either hard-code or set a limit on 'n', but, if I were
to do that, is it something that would be accepted? (my ability to code
not-withstanding :-)


 - since
 this is not the type of information you'd ask for often, it would be
 ok if getting this data took longer.. And you seldom want give me the
 last 5 queries for each backend at once.


thinking-aloud
 I'm more concerned with the overhead of managing that array every single
time that a backend updates its status than I am on retrieval.  I guess if
the array were small enough and the the logic clean, it could end up having
about the same overhead as what I'm doing now (obviously, I'm still
gobbling more memory).
/thinking-aloud

Doing that *would* allow us to get away from concerns about breaking
monitoring tools and the like.

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






  Robert Wrote:
  Yeah.  Otherwise, people who are parsing the hard-coded strings idle
  and idle in transaction are likely to get confused.
 
  I would be interested ( and frankly very surprised ) to find out if many
  monitoring tools are actually parsing that field.  Most that I see just
 dump
  whatever is in current_query to the user.  I would imaging that, so long
 as
  the server obeyed pgstat_track_activity_size most tools would behave
 nicely.

 Really? I'd assume every single monitoring tool that graphs how many
 active connections you have (vs idle vs idle in transaction) would do
 just this.

 --
  Magnus Hagander
  Me: http://www.hagander.net/
  Work: http://www.redpill-linpro.com/



Re: [HACKERS] IDLE in transaction introspection

2011-11-01 Thread Magnus Hagander
On Tue, Nov 1, 2011 at 18:40, Scott Mead sco...@openscg.com wrote:


 On Tue, Nov 1, 2011 at 1:20 PM, Magnus Hagander mag...@hagander.net wrote:

 On Tue, Nov 1, 2011 at 18:11, Scott Mead sco...@openscg.com wrote:
 
  On Tue, Nov 1, 2011 at 10:40 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 
  Robert Haas robertmh...@gmail.com writes:
   On Tue, Nov 1, 2011 at 9:52 AM, Tom Lane t...@sss.pgh.pa.us wrote:
   That would cost twice as much shared memory for query strings, and
   twice
   as much time to update the strings, for what seems pretty marginal
   value.  I'm for just redefining the query field as current or last
   query.
 
   Not really.  You could just store it once in shared memory, and put
   the complexity in the view definition.
 
  I understood the proposal to be store the previous query in addition
  to the current-query-if-any.  If that's not what was meant, then my
  objection was incorrect.  However, like you, I'm pretty dubious of
  having two mostly-redundant fields in the view definition, just because
  of window width issues.
 
  The biggest reason I dislike the multi-field approach is because it
  limits
  us to only the [single] previous_query in the system with all the
  overhead
  we talked about  (memory, window width and messing with system catalogs
  in
  general).  That's actually why I implemented it the way I did, just by
  appending the last query on the end of the string when it's IDLE in
  transaction.

 Well, by appending it in that field, you require the end
 user/monitoring app to parse out the string anyway, so you're not
 exactly making life easier on the consumer of the information..


  Marti wrote:
 
  I'd very much like to see a more generic solution: a runtime query log
  facility that can be queried in any way you want. pg_stat_statements
  comes close, but is limited too due to its (arbitrary, I find)
  deduplication -- you can't query for 10 last statements from process
  N since it has no notion of processes, just users and databases.
 
  This is what I'd really like to see (just haven't had time as it is a
  much
  bigger project).  The next question my devs ask is what were the last
  five
  queries that ran... can you show me an overview of an entire
  transaction
  etc...
    That being said, having the previous_query available feels like it
  fixes
  about 80% of the *problem*; transaction profiling, or looking back 10 /
  15 /
  20 queries is [immensely] useful, but I find that the bigger need is the
  ability to short-circuit dba / dev back-n-forth by just saying Your app
  refused to commit/rollback after running query XYZ.

 This would be great, but as you say, it's a different project.

 Perhaps something could be made along the line of each backend keeping
 it's *own* set of old queries, and then making it available to a
 specific function (pg_get_last_queries_for_backend(nnn))

 Yeah, I was kind of thinking this too, I just feel dirty adding a (n
 * track_activity_query_size) overhead to shared memory for tracking that if
 we're already concerned about adding just a 'previous_query' string.  It's
 easy enough to either hard-code or set a limit on 'n', but, if I were to do
 that, is it something that would be accepted? (my ability to code
 not-withstanding :-)

No, I meant storing it in backend local memory, and then transfer it
upon request. That would remove locking requirements etc.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-01 Thread Robert Treat
On Tue, Nov 1, 2011 at 1:20 PM, Magnus Hagander mag...@hagander.net wrote:
 On Tue, Nov 1, 2011 at 18:11, Scott Mead sco...@openscg.com wrote:

 On Tue, Nov 1, 2011 at 10:40 AM, Tom Lane t...@sss.pgh.pa.us wrote:

 Robert Haas robertmh...@gmail.com writes:
  On Tue, Nov 1, 2011 at 9:52 AM, Tom Lane t...@sss.pgh.pa.us wrote:
  That would cost twice as much shared memory for query strings, and
  twice
  as much time to update the strings, for what seems pretty marginal
  value.  I'm for just redefining the query field as current or last
  query.

  Not really.  You could just store it once in shared memory, and put
  the complexity in the view definition.

 I understood the proposal to be store the previous query in addition
 to the current-query-if-any.  If that's not what was meant, then my
 objection was incorrect.  However, like you, I'm pretty dubious of
 having two mostly-redundant fields in the view definition, just because
 of window width issues.

 The biggest reason I dislike the multi-field approach is because it limits
 us to only the [single] previous_query in the system with all the overhead
 we talked about  (memory, window width and messing with system catalogs in
 general).  That's actually why I implemented it the way I did, just by
 appending the last query on the end of the string when it's IDLE in
 transaction.

 Well, by appending it in that field, you require the end
 user/monitoring app to parse out the string anyway, so you're not
 exactly making life easier on the consumer of the information..


+1


 Marti wrote:

 I'd very much like to see a more generic solution: a runtime query log
 facility that can be queried in any way you want. pg_stat_statements
 comes close, but is limited too due to its (arbitrary, I find)
 deduplication -- you can't query for 10 last statements from process
 N since it has no notion of processes, just users and databases.

 This is what I'd really like to see (just haven't had time as it is a much
 bigger project).  The next question my devs ask is what were the last five
 queries that ran... can you show me an overview of an entire transaction
 etc...
   That being said, having the previous_query available feels like it fixes
 about 80% of the *problem*; transaction profiling, or looking back 10 / 15 /
 20 queries is [immensely] useful, but I find that the bigger need is the
 ability to short-circuit dba / dev back-n-forth by just saying Your app
 refused to commit/rollback after running query XYZ.

 This would be great, but as you say, it's a different project.


+1 (but I'd like to see that different project)

 Perhaps something could be made along the line of each backend keeping
 it's *own* set of old queries, and then making it available to a
 specific function (pg_get_last_queries_for_backend(nnn)) - since
 this is not the type of information you'd ask for often, it would be
 ok if getting this data took longer.. And you seldom want give me the
 last 5 queries for each backend at once.


 Robert Wrote:
 Yeah.  Otherwise, people who are parsing the hard-coded strings idle
 and idle in transaction are likely to get confused.

 I would be interested ( and frankly very surprised ) to find out if many
 monitoring tools are actually parsing that field.  Most that I see just dump
 whatever is in current_query to the user.  I would imaging that, so long as
 the server obeyed pgstat_track_activity_size most tools would behave nicely.

 Really? I'd assume every single monitoring tool that graphs how many
 active connections you have (vs idle vs idle in transaction) would do
 just this.


Having written and/or patched dozens of these types of things, your
spot on; all of the ones with anything other than the most brain dead
of monitoring parse for IDLE and IDLE in transaction. That said, I'm
happy to see the {active|idle|idle in txn} status field and
query_string fields show up and break backwards compatibility.
Updating the tools will be simple for those who need it, and make a
view to work around it will be simple for those who don't. Happy to
add an example view definition to the docs if it will help.

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-01 Thread Andrew Dunstan



On 11/01/2011 09:52 AM, Tom Lane wrote:

Simon Riggssi...@2ndquadrant.com  writes:

Why not leave it exactly as it is, and add a previous_query column?
That gives you exactly what you need without breaking anything.

That would cost twice as much shared memory for query strings, and twice
as much time to update the strings, for what seems pretty marginal
value.  I'm for just redefining the query field as current or last
query.


+1


I could go either way on whether to rename it.


Rename it please. current_query will just be wrong. I'd be inclined 
just to call it query or query_string and leave it to the docs to 
define the exact semantics.


cheers

andrew



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


Re: [HACKERS] IDLE in transaction introspection

2011-11-01 Thread Scott Mead
On Tue, Nov 1, 2011 at 10:40 AM, Tom Lane t...@sss.pgh.pa.us wrote:

 Robert Haas robertmh...@gmail.com writes:
  On Tue, Nov 1, 2011 at 9:52 AM, Tom Lane t...@sss.pgh.pa.us wrote:
  That would cost twice as much shared memory for query strings, and twice
  as much time to update the strings, for what seems pretty marginal
  value.  I'm for just redefining the query field as current or last
  query.

  Not really.  You could just store it once in shared memory, and put
  the complexity in the view definition.

 I understood the proposal to be store the previous query in addition
 to the current-query-if-any.  If that's not what was meant, then my
 objection was incorrect.  However, like you, I'm pretty dubious of
 having two mostly-redundant fields in the view definition, just because
 of window width issues.


The biggest reason I dislike the multi-field approach is because it limits
us to only the [single] previous_query in the system with all the overhead
we talked about  (memory, window width and messing with system catalogs in
general).  That's actually why I implemented it the way I did, just by
appending the last query on the end of the string when it's IDLE in
transaction.

Marti wrote:

I'd very much like to see a more generic solution: a runtime query log
 facility that can be queried in any way you want. pg_stat_statements
 comes close, but is limited too due to its (arbitrary, I find)
 deduplication -- you can't query for 10 last statements from process
 N since it has no notion of processes, just users and databases.


This is what I'd really like to see (just haven't had time as it is a much
bigger project).  The next question my devs ask is what were the last five
queries that ran... can you show me an overview of an entire transaction
etc...

  That being said, having the previous_query available feels like it fixes
about 80% of the *problem*; transaction profiling, or looking back 10 / 15
/ 20 queries is [immensely] useful, but I find that the bigger need is the
ability to short-circuit dba / dev back-n-forth by just saying Your app
refused to commit/rollback after running query XYZ.

Robert Wrote:
 Yeah.  Otherwise, people who are parsing the hard-coded strings idle
 and idle in transaction are likely to get confused.


I would be interested ( and frankly very surprised ) to find out if many
monitoring tools are actually parsing that field.  Most that I see just
dump whatever is in current_query to the user.  I would imaging that, so
long as the server obeyed pgstat_track_activity_size most tools would
behave nicely.


Now... all that being said, I've implemented the 'previous_query' column
and (maybe just for my own benefit), is the PostgreSQL community interested
in the patch?

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



regards, tom lane



Re: [HACKERS] IDLE in transaction introspection

2011-11-01 Thread Ross J. Reedstrom
On Tue, Nov 01, 2011 at 10:13:52AM -0400, Andrew Dunstan wrote:
 
 
 On 11/01/2011 09:52 AM, Tom Lane wrote:
 Simon Riggssi...@2ndquadrant.com  writes:
 Why not leave it exactly as it is, and add a previous_query column?
 That gives you exactly what you need without breaking anything.
 That would cost twice as much shared memory for query strings, and twice
 as much time to update the strings, for what seems pretty marginal
 value.  I'm for just redefining the query field as current or last
 query.
 
 +1
 
 I could go either way on whether to rename it.
 
 Rename it please. current_query will just be wrong. I'd be
 inclined just to call it query or query_string and leave it to
 the docs to define the exact semantics.

+1 on providing the most-recent-query info
+1 on the state/query split as a means
+1 rename from current_query (i.e. break it)
personalbikeshedcolor: query_string

Personal opinion on backwards compatability matches Robert's: things
that affect admin functionality are less of an issue than those that
impact user (i.e. coder) SQL. And definitely break it: I may chose to fix
it by bodging in a view for the old behavior myself, but that's
my choice. Perhaps provide an example view def in change notes if you
really want to.  For myself, when making fixes to monitor scripts for
this type of change, my reaction is probably Yes, finally, I'm not
regexing on the d*mn query string anymore!

Ross
P.S. though apparently it doesn't bother me enough to create and submit
a patch myself. :-(
-- 
Ross Reedstrom, Ph.D. reeds...@rice.edu
Systems Engineer  Admin, Research Scientistphone: 713-348-6166
Connexions  http://cnx.orgfax: 713-348-3665
Rice University MS-375, Houston, TX 77005
GPG Key fingerprint = F023 82C8 9B0E 2CC6 0D8E  F888 D3AE 810E 88F0 BEDE

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


Re: [HACKERS] IDLE in transaction introspection

2011-10-31 Thread Magnus Hagander
On Mon, Oct 31, 2011 at 22:37, Scott Mead sco...@openscg.com wrote:
 Hey all,

    So, I'm dealing with a a big ol' java app that has multiple roads on the
 way to IDLE in transaction.  We can reproduce the problem in a test
 environment, but the lead dev always asks can you just tell me the last
 query that it ran?
    So I wrote the attached patch, it just turns IDLE in transaction into:
  IDLE in transaction\n: Previous: last query executed.  After seeing
 how quickly our dev's fixed the issue once they saw prepared statement XYZ,
 I'm thinking that I'd like to be able to have this in prod, and... maybe
 (with the frequency of IIT questions posted here) someone else would find
 this useful.

  Just wondering what ya'll thought.  Any feedback (including a more
 efficient approach) is welcome.  (Patch against release 9.1.1 tarball).
 Thanks!

I think the idea in general is pretty useful, but I'd like to extend
on it. It would be even better to have a last query executed in the
general IDLE state as well, not just idle in transaction.

However, doing it the way you did it by adding it to the current query
is going to break a lot of tools. I think it's a better idea to create
a separate column called last query or something like that.

Actually, for the future, it might be useful to have a state column,
that holds the idle/in transaction/running status, instead of the
tools having to parse the query text to get that information...

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

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


Re: [HACKERS] IDLE in transaction introspection

2011-10-31 Thread Jaime Casanova
On Mon, Oct 31, 2011 at 4:45 PM, Magnus Hagander mag...@hagander.net wrote:

 Actually, for the future, it might be useful to have a state column,
 that holds the idle/in transaction/running status, instead of the
 tools having to parse the query text to get that information...


if we are going to create the state column let's do it now and
change current_query for last_query (so last query can be running, or
it was the last before enter in idle state)

-- 
Jaime Casanova         www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación

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


Re: [HACKERS] IDLE in transaction introspection

2011-10-31 Thread Robert Haas
On Mon, Oct 31, 2011 at 5:45 PM, Magnus Hagander mag...@hagander.net wrote:
 Actually, for the future, it might be useful to have a state column,
 that holds the idle/in transaction/running status, instead of the
 tools having to parse the query text to get that information...

+1 for doing it this way.  Splitting current_query into query and
state would be more elegant and easier to use all around.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] IDLE in transaction introspection

2011-10-31 Thread Scott Mead
On Mon, Oct 31, 2011 at 6:13 PM, Robert Haas robertmh...@gmail.com wrote:

 On Mon, Oct 31, 2011 at 5:45 PM, Magnus Hagander mag...@hagander.net
 wrote:
  Actually, for the future, it might be useful to have a state column,
  that holds the idle/in transaction/running status, instead of the
  tools having to parse the query text to get that information...

 +1 for doing it this way.  Splitting current_query into query and
 state would be more elegant and easier to use all around.


I'm all for splitting it out actually.  My concern was that I would break
the 'ba-gillion' monitoring tools that already have support for
pg_stat_activity if I dropped a column.  What if we had:

   'state' : idle | in transaction | running ( per Robert )
   'current_query' :  the most recent query (either last / currently
running)

   That may be a bit tougher to get across to people though (especially in
the case where state='IDLE').

 I'll rework this when I don't have trick-or-treaters coming to the front
door :)

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


 --
 Robert Haas
 EnterpriseDB: http://www.enterprisedb.com
 The Enterprise PostgreSQL Company



Re: [HACKERS] IDLE in transaction introspection

2011-10-31 Thread Scott Mead
On Mon, Oct 31, 2011 at 7:18 PM, Scott Mead sco...@openscg.com wrote:



 On Mon, Oct 31, 2011 at 6:13 PM, Robert Haas robertmh...@gmail.comwrote:

 On Mon, Oct 31, 2011 at 5:45 PM, Magnus Hagander mag...@hagander.net
 wrote:
  Actually, for the future, it might be useful to have a state column,
  that holds the idle/in transaction/running status, instead of the
  tools having to parse the query text to get that information...

 +1 for doing it this way.  Splitting current_query into query and
 state would be more elegant and easier to use all around.


 I'm all for splitting it out actually.  My concern was that I would break
 the 'ba-gillion' monitoring tools that already have support for
 pg_stat_activity if I dropped a column.  What if we had:

'state' : idle | in transaction | running ( per Robert )


   Sorry per Robert and Jaime


'current_query' :  the most recent query (either last / currently
 running)

That may be a bit tougher to get across to people though (especially in
 the case where state='IDLE').

  I'll rework this when I don't have trick-or-treaters coming to the front
 door :)

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


 --
 Robert Haas
 EnterpriseDB: http://www.enterprisedb.com
 The Enterprise PostgreSQL Company