Re: [HACKERS] invalid search_path complaints

2012-04-04 Thread Scott Mead
On Wed, Apr 4, 2012 at 12:02 PM, Tom Lane t...@sss.pgh.pa.us wrote:

 Scott Mead sco...@openscg.com writes:
  Personally, I feel that if unix will let you be stupid:
  $ export PATH=/usr/bin:/this/invalid/crazy/path
  $ echo $PATH
  /usr/bin:/this/invalid/crazy/path
  PG should trust that I'll get where I'm going eventually :)

 Well, that's an interesting analogy.  Are you arguing that we should
 always accept any syntactically-valid search_path setting, no matter
 whether the mentioned schemas exist?  It wouldn't be hard to do that.


   I think we should always accept a syntactically valid search_path.


 The fun stuff comes in when you try to say I want a warning in these
 contexts but not those, because (a) the behavior you think you want
 turns out to be pretty squishy, and (b) it's not always clear from the
 implementation level what the context is.


ISTM that just issuing a warning whenever you set the search_path (no
matter which context) feels valid (and better than the above *nix
behavior).  I would personally be opposed to seeing it on login however.

--Scott




regards, tom lane



Re: [HACKERS] invalid search_path complaints

2012-04-03 Thread Scott Mead
On Tue, Apr 3, 2012 at 2:37 PM, Tom Lane t...@sss.pgh.pa.us wrote:

 Robert Haas robertmh...@gmail.com writes:
  So we have an established precedent that it is right to warn about
  things that are sketchy at the time that they are defined, but not
  every time they are used.

 Sure, but we don't have that option available to us here --- or more
 accurately, ALTER USER/DATABASE SET *does* warn if the search_path value
 looks like it might be invalid according to the current context, but
 that helps little for this problem.  What's important is whether the
 value is valid when we attempt to apply it.


 Basically, I don't think you've made a strong case for changing this
 behavior; nor have you explained what you think we should do instead.

 I have a legacy application now that relies heavily on multiple databases
and multiple schemas.  The issue I have is that we have postgres deployed
very widely and have a cookie-cutter script for everything.  We know for
example:

  (each schema exists only in its respective DB)

   user oltp should be able to see schemaA in db1, schemaB in db2 and
schemaC in db3
   user reporting should be able to see biSchema in db1, reporting schema
in db2 and schemaC in db3

This is across a large multitude of databases and hosts.  One of the
things I've loved about this is the ability to hide certain things from
users ( of course they can do a \dn and fully-qualify, which is why we have
permissions too, but I really appreciate the 'hidden-ness' of my tables ).

Because our schemas are all over the place, now I've got to setup a
hard-coded search_path in postgresql.conf which feels even worse to me than
the per-user setup.

Personally, I feel that if unix will let you be stupid:
$ export PATH=/usr/bin:/this/invalid/crazy/path
$ echo $PATH
/usr/bin:/this/invalid/crazy/path

PG should trust that I'll get where I'm going eventually :)

Just my two cents.

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




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

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

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



[HACKERS] IDLE in transaction introspection

2011-10-31 Thread Scott Mead
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!

--
Scott Mead
  OpenSCG


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