Re: [PATCHES] [HACKERS] Overhead for stats_command_string et al, take 2
Bruce Momjian <[EMAIL PROTECTED]> writes: > If you put a contition test in set_ps_display(), the only clean way to > do this is for init_ps_display() to force update_process_title to true > before we call set_ps_display(), then reset it to its original value, > but that sounds pretty ugly. No, refactor the code. I was envisioning something called maybe transmit_ps_display that would contain the part of set_ps_display beginning at "Transmit new setting to kernel". Then both set_ps_display and init_ps_display would call that. regards, tom lane ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] [HACKERS] Overhead for stats_command_string et al, take 2
Bruce Momjian <[EMAIL PROTECTED]> writes:
> Tom Lane wrote:
>> This is an ugly patch. Why not *one* test of the GUC variable, inside
>> set_ps_display(), and no side-effects on callers? You would need to
>> force an initial update from init_ps_display, but that only requires a
>> small amount of code refactoring inside ps_status.c.
> Consider all the helper processes that set their process title. The
> only thing I can think of is to add a boolean to set_ps_display() so say
> whether this is per-command set or not. Is that your idea?
No, that's not what I said at all. Currently init_ps_display doesn't
actually force the display to update; it's left to the first
set_ps_display call to do that. If we made init_ps_display update the
status unconditionally, then set_ps_display could be a conditional
no-op, and in the helper process setup code
/* Identify myself via ps */
init_ps_display("autovacuum process", "", "");
set_ps_display("");
we could remove the now-unnecessary set_ps_display("") calls, but
the other set_ps_display() calls would stay exactly like they are.
regards, tom lane
---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match
Re: [PATCHES] [HACKERS] Overhead for stats_command_string et al, take 2
Bruce Momjian <[EMAIL PROTECTED]> writes: > Attached patch adds GUC 'update_process_title' to control ps display > updates per SQL command. Default to 'on'. GUC name OK? This is an ugly patch. Why not *one* test of the GUC variable, inside set_ps_display(), and no side-effects on callers? You would need to force an initial update from init_ps_display, but that only requires a small amount of code refactoring inside ps_status.c. The one place that might be worth having an external test on the GUC is in lock.c, but then it should bypass the entire business of copying, modifying, and restoring the title ... not just the two set_ps_display calls. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] Overhead for stats_command_string et al, take 2
Tom Lane wrote:
> Bruce Momjian <[EMAIL PROTECTED]> writes:
> > Yep, I see 8% here. I will add a patch to allow the ps display to be
> > turned off.
>
> I think we'd still want a backend to set the PS display once with its
> identification data (user/DB name and client address). It's just the
> transient activity updates that should stop.
Attached patch adds GUC 'update_process_title' to control ps display
updates per SQL command. Default to 'on'. GUC name OK?
--
Bruce Momjian [EMAIL PROTECTED]
EnterpriseDBhttp://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: doc/src/sgml/config.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/config.sgml,v
retrieving revision 1.66
diff -c -c -r1.66 config.sgml
*** doc/src/sgml/config.sgml 19 Jun 2006 01:51:21 - 1.66
--- doc/src/sgml/config.sgml 26 Jun 2006 19:59:53 -
***
*** 2888,2893
--- 2888,2908
+
+ update_process_title (boolean)
+
+update_process_title configuration parameter
+
+
+
+ Enables updating of the process title every time a new SQL command
+ is received by the server. The process title is typically viewed
+ by the ps command or in Windows using the Process
+ Explorer. Only superusers can change this setting.
+
+
+
+
stats_start_collector (boolean)
Index: src/backend/commands/async.c
===
RCS file: /cvsroot/pgsql/src/backend/commands/async.c,v
retrieving revision 1.131
diff -c -c -r1.131 async.c
*** src/backend/commands/async.c 25 Apr 2006 14:11:54 - 1.131
--- src/backend/commands/async.c 26 Jun 2006 19:59:56 -
***
*** 908,914
if (Trace_notify)
elog(DEBUG1, "ProcessIncomingNotify");
! set_ps_display("notify interrupt");
notifyInterruptOccurred = 0;
--- 908,915
if (Trace_notify)
elog(DEBUG1, "ProcessIncomingNotify");
! if (update_process_title)
! set_ps_display("notify interrupt");
notifyInterruptOccurred = 0;
***
*** 979,985
*/
pq_flush();
! set_ps_display("idle");
if (Trace_notify)
elog(DEBUG1, "ProcessIncomingNotify: done");
--- 980,987
*/
pq_flush();
! if (update_process_title)
! set_ps_display("idle");
if (Trace_notify)
elog(DEBUG1, "ProcessIncomingNotify: done");
Index: src/backend/postmaster/postmaster.c
===
RCS file: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.488
diff -c -c -r1.488 postmaster.c
*** src/backend/postmaster/postmaster.c 20 Jun 2006 22:52:00 - 1.488
--- src/backend/postmaster/postmaster.c 26 Jun 2006 20:00:03 -
***
*** 2714,2721
* title for ps. It's good to do this as early as possible in startup.
*/
init_ps_display(port->user_name, port->database_name, remote_ps_data);
! set_ps_display("authentication");
!
/*
* Now perform authentication exchange.
*/
--- 2714,2724
* title for ps. It's good to do this as early as possible in startup.
*/
init_ps_display(port->user_name, port->database_name, remote_ps_data);
! if (update_process_title)
! set_ps_display("authentication");
! else
! set_ps_display("");
!
/*
* Now perform authentication exchange.
*/
Index: src/backend/storage/lmgr/lock.c
===
RCS file: /cvsroot/pgsql/src/backend/storage/lmgr/lock.c,v
retrieving revision 1.164
diff -c -c -r1.164 lock.c
*** src/backend/storage/lmgr/lock.c 14 Apr 2006 03:38:55 - 1.164
--- src/backend/storage/lmgr/lock.c 26 Jun 2006 20:00:05 -
***
*** 1069,1075
new_status = (char *) palloc(len + 8 + 1);
memcpy(new_status, old_status, len);
strcpy(new_status + len, " waiting");
! set_ps_display(new_status);
new_status[len] = '\0'; /* truncate off " waiting" */
awaitedLock = locallock;
--- 1069,1076
new_status = (char *) palloc(len + 8 + 1);
memcpy(new_status, old_status, len);
strcpy(new_status + len, " waiting");
! if (update_process_title)
! set_ps_display(new_status);
new_status[len] = '\0'; /* truncate off " waiting" */
awaitedLock = locallock;
***
*** 1108,1114
awaitedLock = NULL;
! set_ps_display(new_status);
pfree(new_status);
LOCK_PRINT("WaitOnLock: wakeup on lock",
--- 1109,1116
awaitedLock = NULL;
! if (update_process_title)
! set_ps_display(new_status);
pfree(new_status);
LOCK_PRINT("WaitOnLock: wakeup on lock",
Index: src/backend/tcop/postgres.c
===
RCS file: /cvsroot/pgsql/src/backend/tcop/postgres.c,v
retrieving revision
Re: [PATCHES] [HACKERS] Overhead for stats_command_string et al, take 2
Tom Lane wrote: > Bruce Momjian <[EMAIL PROTECTED]> writes: > > Seeing stats_command_string with almost zero overhead is great news! > > Should we remove that setting and just have it enabled all > > the time? > > If you don't need it, you shouldn't have to pay any overhead for it, > I think. One could make an argument now for having stats_command_string > default to ON, though. The attached patch makes stats_command_string default to 'on', and updates the documentation. > Something that might also be interesting is an option to suppress > per-command ps_status reporting. On machines where updating ps status > takes a kernel call, there's now a pretty good argument why you might > want to turn that off and rely on pg_stat_activity instead. OK, can I get a timing report from someone with the title on/off that shows a difference? -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: doc/src/sgml/config.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/config.sgml,v retrieving revision 1.66 diff -c -c -r1.66 config.sgml *** doc/src/sgml/config.sgml 19 Jun 2006 01:51:21 - 1.66 --- doc/src/sgml/config.sgml 26 Jun 2006 17:13:05 - *** *** 2878,2884 Enables the collection of information on the currently executing command of each session, along with the time at ! which that command began execution. This parameter is off by default. Note that even when enabled, this information is not visible to all users, only to superusers and the user owning the session being reported on; so it should not represent a --- 2878,2884 Enables the collection of information on the currently executing command of each session, along with the time at ! which that command began execution. This parameter is on by default. Note that even when enabled, this information is not visible to all users, only to superusers and the user owning the session being reported on; so it should not represent a Index: doc/src/sgml/monitoring.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/monitoring.sgml,v retrieving revision 1.34 diff -c -c -r1.34 monitoring.sgml *** doc/src/sgml/monitoring.sgml 19 Jun 2006 01:51:21 - 1.34 --- doc/src/sgml/monitoring.sgml 26 Jun 2006 17:13:05 - *** *** 170,181 ! Since the parameters stats_command_string, ! stats_block_level, and stats_row_level default to false, very few statistics are collected in the default ! configuration. Enabling one or more of these configuration ! variables will significantly enhance the amount of useful data produced by the statistics facilities, at the expense of additional run-time overhead. --- 170,180 ! Since the parameters stats_block_level, and stats_row_level default to false, very few statistics are collected in the default ! configuration. Enabling either of these configuration ! variables will significantly increase the amount of useful data produced by the statistics facilities, at the expense of additional run-time overhead. *** *** 241,249 process ID, user OID, user name, current query, time at which the current query began execution, time at which the process was started, and client's address and port number. The columns ! that report data on the current query are only available if the parameter stats_command_string has been ! turned on. Furthermore, these columns read as null unless the user examining the view is a superuser or the same as the user owning the process being reported on. --- 240,248 process ID, user OID, user name, current query, time at which the current query began execution, time at which the process was started, and client's address and port number. The columns ! that report data on the current query are available unless the parameter stats_command_string has been ! turned off. Furthermore, these columns are only visible if the user examining the view is a superuser or the same as the user owning the process being reported on. *** *** 635,644 pg_stat_get_backend_activity(integer) text !Active command of the given server process (null if the !current user is not a superuser nor the same user as that of !the session being queried, or !stats_command_string is not on) --- 634,643 pg_stat_get_backend_activity(integer) text !Ac
Re: [PATCHES] [HACKERS] Overhead for stats_command_string et al, take 2
Bruce Momjian <[EMAIL PROTECTED]> writes: > Tom Lane wrote: >> Something that might also be interesting is an option to suppress >> per-command ps_status reporting. On machines where updating ps status >> takes a kernel call, there's now a pretty good argument why you might >> want to turn that off and rely on pg_stat_activity instead. > OK, can I get a timing report from someone with the title on/off that > shows a difference? IIRC, newer BSDen use a kernel call for this, so you should be able to measure it on your own machine. Just tweak ps_status.c to force it to select PS_USE_NONE instead of PS_USE_SETPROCTITLE to generate a comparison case. I'll try it on my old HPUX box too. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Overhead for stats_command_string et al, take 2
I wrote: > IIRC, newer BSDen use a kernel call for this, so you should be able to > measure it on your own machine. Just tweak ps_status.c to force it to > select PS_USE_NONE instead of PS_USE_SETPROCTITLE to generate a > comparison case. I'll try it on my old HPUX box too. On HPUX, I get a median time of 5.59 sec for CVS HEAD vs 5.36 sec with ps_status diked out, for the test case of 1 "SELECT 1;" as separate transactions, assert-disabled build. So, almost 10% overhead. Given that the transactions can't get any more trivial than this, that's about a worst-case number. Not sure if it's worth worrying about or not. However Kris Kennaway's report a couple weeks ago suggested things might be worse on BSD. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
