Thread: pgmonitor patch for query string

pgmonitor patch for query string

From
Bruce Momjian
Date:
I would like to apply the following patch to the CVS tree.  It allows
pgmonitor to show query strings even if the backend is not compiled with
debug symbols.

It does this by creating a global variable 'debug_query_string' and
assigning it when the query begins and clearing it when the query ends.
It needs to be a global symbol so gdb can find it without debug symbols.

Seems like a very safe patch, and it allows pgmonitor to be much more
useful until we get a shared memory solution in 7.2.

Is this OK with everyone?

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
Index: src/backend/tcop/postgres.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/tcop/postgres.c,v
retrieving revision 1.211
diff -c -r1.211 postgres.c
*** src/backend/tcop/postgres.c    2001/03/14 15:14:35    1.211
--- src/backend/tcop/postgres.c    2001/03/14 16:29:26
***************
*** 74,79 ****
--- 74,81 ----
  extern int optind;
  extern char *optarg;

+ char *debug_query_string;        /* used by pgmonitor */
+
  /*
   * for ps display
   */
***************
*** 621,626 ****
--- 623,630 ----
      List       *parsetree_list,
                 *parsetree_item;

+     debug_query_string = query_string;    /* used by pgmonitor */
+
      /*
       * Start up a transaction command.  All queries generated by the
       * query_string will be in this same command block, *unless* we find
***************
*** 855,860 ****
--- 859,866 ----
       */
      if (xact_started)
          finish_xact_command();
+
+     debug_query_string = NULL;        /* used by pgmonitor */
  }

  /*
***************
*** 1718,1723 ****
--- 1724,1731 ----

      if (sigsetjmp(Warn_restart, 1) != 0)
      {
+         debug_query_string = NULL;        /* used by pgmonitor */
+
          /*
           * NOTE: if you are tempted to add more code in this if-block,
           * consider the probability that it should be in AbortTransaction()

Re: pgmonitor patch for query string

From
The Hermit Hacker
Date:
not with me it isn't ... it doesn't fix a bug, it doesn't go in ... save
it for after v7.1 is released ...


On Wed, 14 Mar 2001, Bruce Momjian wrote:

> I would like to apply the following patch to the CVS tree.  It allows
> pgmonitor to show query strings even if the backend is not compiled with
> debug symbols.
>
> It does this by creating a global variable 'debug_query_string' and
> assigning it when the query begins and clearing it when the query ends.
> It needs to be a global symbol so gdb can find it without debug symbols.
>
> Seems like a very safe patch, and it allows pgmonitor to be much more
> useful until we get a shared memory solution in 7.2.
>
> Is this OK with everyone?
>
> --
>   Bruce Momjian                        |  http://candle.pha.pa.us
>   pgman@candle.pha.pa.us               |  (610) 853-3000
>   +  If your life is a hard drive,     |  830 Blythe Avenue
>   +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
>

Marc G. Fournier                   ICQ#7615664               IRC Nick: Scrappy
Systems Administrator @ hub.org
primary: scrappy@hub.org           secondary: scrappy@{freebsd|postgresql}.org



Re: pgmonitor patch for query string

From
Bruce Momjian
Date:
> 
> not with me it isn't ... it doesn't fix a bug, it doesn't go in ... save
> it for after v7.1 is released ...

You are saying save it for 7.2, right?  That will certainly be months
away.  Without this patch, pgmonitor's 'query' button will only work if
the postgres binary was compiled with debug symbols.

You are basically saying no, even thought it has no risks, right?

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
853-3000+  If your life is a hard drive,     |  830 Blythe Avenue +  Christ can be your backup.        |  Drexel Hill,
Pennsylvania19026
 


Re: pgmonitor patch for query string

From
The Hermit Hacker
Date:
On Wed, 14 Mar 2001, Bruce Momjian wrote:

> >
> > not with me it isn't ... it doesn't fix a bug, it doesn't go in ... save
> > it for after v7.1 is released ...
>
> You are saying save it for 7.2, right?  That will certainly be months
> away.  Without this patch, pgmonitor's 'query' button will only work if
> the postgres binary was compiled with debug symbols.

Put it up as a patch to v7.1, and include it as part of 7.1.1 ...

> You are basically saying no, even thought it has no risks, right?

I'm saying no because it doesn't fix any known bugs, it *adds* another
feature ... we are *months* too late in the cycle for that ...



Re: pgmonitor patch for query string

From
Bruce Momjian
Date:
> On Wed, 14 Mar 2001, Bruce Momjian wrote:
> 
> > >
> > > not with me it isn't ... it doesn't fix a bug, it doesn't go in ... save
> > > it for after v7.1 is released ...
> >
> > You are saying save it for 7.2, right?  That will certainly be months
> > away.  Without this patch, pgmonitor's 'query' button will only work if
> > the postgres binary was compiled with debug symbols.
> 
> Put it up as a patch to v7.1, and include it as part of 7.1.1 ...

Oh.  It would just be easier to say that I support 7.1 rather than
7.1.1, though that is certainly much better than 7.2.  :-)


> > You are basically saying no, even thought it has no risks, right?
> 
> I'm saying no because it doesn't fix any known bugs, it *adds* another
> feature ... we are *months* too late in the cycle for that ...

I agree we are many months back, and I am a little upset about it
myself.  

I just don't see any delay or risk in applying the patch.  

However, 7.1.1 is fine, so I will just wait for that one.  I am working
on a lock-status display option now too, so I can put them both in 7.1.1
maybe with the pgmonitor script in CVS too.

Sounds like a plan.

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
853-3000+  If your life is a hard drive,     |  830 Blythe Avenue +  Christ can be your backup.        |  Drexel Hill,
Pennsylvania19026
 


Re: pgmonitor patch for query string

From
Bruce Momjian
Date:
> Bruce Momjian writes:
> 
> > It does this by creating a global variable 'debug_query_string' and
> > assigning it when the query begins and clearing it when the query ends.
> 
> You can find out the current query for a given backend by configuring the
> server with "debug_print_query on" and "log_pids on" and running
> 
> sed -n "/[$pid]/"'s/^.*query: \(.*\)$/\1/p' $logfile
> 
> This doesn't tell you whether the query is still running, but ps tells you
> that.  In fact, it might be an idea to add a logging option that prints
> something like "query finished in xxx ms".  We actually have something
> similar hidden under show_query_stats, but the formatting needs to be made
> more convenient and possibly less verbose.  But at least this way you have
> it for the record, and not only on the screen.

Yes, I thought of that idea.  I wasn't sure I would be able to find the
log file in any installation-independent way, or even if a log file was
even being kept.

And I was afraid some people wouldn't want to log all queries.

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
853-3000+  If your life is a hard drive,     |  830 Blythe Avenue +  Christ can be your backup.        |  Drexel Hill,
Pennsylvania19026
 


Re: pgmonitor patch for query string

From
Peter Eisentraut
Date:
Bruce Momjian writes:

> It does this by creating a global variable 'debug_query_string' and
> assigning it when the query begins and clearing it when the query ends.

You can find out the current query for a given backend by configuring the
server with "debug_print_query on" and "log_pids on" and running

sed -n "/[$pid]/"'s/^.*query: \(.*\)$/\1/p' $logfile

This doesn't tell you whether the query is still running, but ps tells you
that.  In fact, it might be an idea to add a logging option that prints
something like "query finished in xxx ms".  We actually have something
similar hidden under show_query_stats, but the formatting needs to be made
more convenient and possibly less verbose.  But at least this way you have
it for the record, and not only on the screen.

-- 
Peter Eisentraut      peter_e@gmx.net       http://yi.org/peter-e/



Re: pgmonitor patch for query string

From
Tom Lane
Date:
The Hermit Hacker <scrappy@hub.org> writes:
> I'm saying no because it doesn't fix any known bugs, it *adds* another
> feature ... we are *months* too late in the cycle for that ...

I thought it was a pretty good idea even without any consideration for
Bruce's monitor program.  The advantage is that one would be able to
extract the current query from a crashed backend's core dump, even if
the backend had been compiled without debug symbols.  Right now you can
only find out the query if you had compiled with debug, because you have
to be able to look at local variables of functions.  And there are an
awful lot of people who don't use debug-enabled builds...

Given that and the low-risk nature of the patch, I vote for it.
        regards, tom lane


Re: pgmonitor patch for query string

From
The Hermit Hacker
Date:
On Wed, 14 Mar 2001, Peter Eisentraut wrote:

> Bruce Momjian writes:
>
> > It does this by creating a global variable 'debug_query_string' and
> > assigning it when the query begins and clearing it when the query ends.
>
> You can find out the current query for a given backend by configuring the
> server with "debug_print_query on" and "log_pids on" and running
>
> sed -n "/[$pid]/"'s/^.*query: \(.*\)$/\1/p' $logfile
>
> This doesn't tell you whether the query is still running, but ps tells you
> that.  In fact, it might be an idea to add a logging option that prints
> something like "query finished in xxx ms".  We actually have something
> similar hidden under show_query_stats, but the formatting needs to be made
> more convenient and possibly less verbose.  But at least this way you have
> it for the record, and not only on the screen.

I *definitely* like this one ... I've been doing wrappers around my
pg_exec() calls in PHP to do some stats generation to work on "slow
queries", but having it in the backend would be more exact ... and easier
to use then having to modify your apps ...



Re: pgmonitor patch for query string

From
Bruce Momjian
Date:
> > This doesn't tell you whether the query is still running, but ps tells you
> > that.  In fact, it might be an idea to add a logging option that prints
> > something like "query finished in xxx ms".  We actually have something
> > similar hidden under show_query_stats, but the formatting needs to be made
> > more convenient and possibly less verbose.  But at least this way you have
> > it for the record, and not only on the screen.
> 
> I *definitely* like this one ... I've been doing wrappers around my
> pg_exec() calls in PHP to do some stats generation to work on "slow
> queries", but having it in the backend would be more exact ... and easier
> to use then having to modify your apps ...
> 

Added to TODO:
* Allow logging of query durations

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
853-3000+  If your life is a hard drive,     |  830 Blythe Avenue +  Christ can be your backup.        |  Drexel Hill,
Pennsylvania19026
 


Re: pgmonitor patch for query string

From
Bruce Momjian
Date:
> > I don't understand the attraction of the UDP stuff.  If we have the
> > stuff in shared memory, we can add a collector program that gathers info
> > from shared memory and allows others to access it, right?
> 
>     There  are a couple of problems with shared memory. First you
>     have to decide a size. That'll limit what you  can  put  into
>     and  if  you  want  to  put things per table (#scans, #block-
>     fetches, #cache-hits, ...), you might run out of  mem  either
>     way with complicated, multy-thousand table schemas.
> 
>     And  the  above  illustrates too that the data structs in the
>     shmem wouldn't be just some simple arrays of counters. So  we
>     have  to  deal  with locking for both, readers and writers of
>     the statistics.

[ Jan, previous email was not sent to list, my mistake.]

OK, I understand the problem with pre-defined size.  That is why I was
looking for a way to dump the information out to a flat file somehow.

I think no matter how we deal with this, we will need some way to turn
on/off such reporting.  We can write into shared memory with little
penalty, but network or filesystem output is not going to be near-zero
cost.

OK, how about a shared buffer area that gets written in a loop so a
separate collection program can grab the info if it wants it, and if
not, it just gets overwritten later.  It can even be per-backend:
       loops start                      end (loop to start)       ----- [-----------------------------]           5
statstat stat stat stat stat                        |^^^                        current pointer
 
--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
853-3000+  If your life is a hard drive,     |  830 Blythe Avenue +  Christ can be your backup.        |  Drexel Hill,
Pennsylvania19026