pgmonitor patch for query string

Started by Bruce Momjianalmost 25 years ago11 messages
#1Bruce Momjian
pgman@candle.pha.pa.us
1 attachment(s)

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

Attachments:

/bjm/difftext/plainDownload
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()
#2The Hermit Hacker
scrappy@hub.org
In reply to: Bruce Momjian (#1)
Re: pgmonitor patch for query string

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

#3Bruce Momjian
pgman@candle.pha.pa.us
In reply to: The Hermit Hacker (#2)
Re: pgmonitor patch for query string

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, Pennsylvania 19026
#4The Hermit Hacker
scrappy@hub.org
In reply to: Bruce Momjian (#3)
Re: pgmonitor patch for query string

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

#5Bruce Momjian
pgman@candle.pha.pa.us
In reply to: The Hermit Hacker (#4)
Re: pgmonitor patch for query string

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, Pennsylvania 19026
#6Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Bruce Momjian (#5)
Re: pgmonitor patch for query string

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, Pennsylvania 19026
#7Peter Eisentraut
peter_e@gmx.net
In reply to: Bruce Momjian (#1)
Re: pgmonitor patch for query string

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/

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: The Hermit Hacker (#4)
Re: pgmonitor patch for query string

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

#9The Hermit Hacker
scrappy@hub.org
In reply to: Peter Eisentraut (#7)
Re: pgmonitor patch for query string

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

#10Bruce Momjian
pgman@candle.pha.pa.us
In reply to: The Hermit Hacker (#9)
Re: pgmonitor patch for query string

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, Pennsylvania 19026
#11Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Tom Lane (#8)
Re: pgmonitor patch for query string

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  stat stat 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, Pennsylvania 19026