Thread: log_min_duration_statement units
Someone complained that the default units for log_min_duration_statement was not mentioned in postgresql.conf; this applied patch fixes that. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/backend/utils/misc/postgresql.conf.sample =================================================================== RCS file: /cvsroot/pgsql/src/backend/utils/misc/postgresql.conf.sample,v retrieving revision 1.255 diff -c -c -r1.255 postgresql.conf.sample *** src/backend/utils/misc/postgresql.conf.sample 6 Apr 2009 18:40:47 -0000 1.255 --- src/backend/utils/misc/postgresql.conf.sample 6 Apr 2009 18:58:02 -0000 *************** *** 306,312 **** # fatal # panic (effectively off) ! #log_min_duration_statement = -1 # -1 is disabled, 0 logs all statements # and their durations, > 0 logs only # statements running at least this time. --- 306,312 ---- # fatal # panic (effectively off) ! #log_min_duration_statement = -1ms # -1 is disabled, 0 logs all statements # and their durations, > 0 logs only # statements running at least this time.
Bruce Momjian <bruce@momjian.us> writes:
> ! #log_min_duration_statement = -1 # -1 is disabled, 0 logs all statements
> ! #log_min_duration_statement = -1ms # -1 is disabled, 0 logs all statements
This is not sensible -- there are no units for the "-1" setting. I
seem to recall that it won't even work; did you test uncommenting that?
regards, tom lane
Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > ! #log_min_duration_statement = -1 # -1 is disabled, 0 logs all statements
>
> > ! #log_min_duration_statement = -1ms # -1 is disabled, 0 logs all statements
>
> This is not sensible -- there are no units for the "-1" setting. I
> seem to recall that it won't even work; did you test uncommenting that?
I tested it uncommented and manually and it worked fine:
test=> set log_min_duration_statement = '-1ms';
SET
test=> show log_min_duration_statement;
log_min_duration_statement
----------------------------
-1
(1 row)
It seems illogical to document it this way but a non-EDB trainer emailed
me saying students are getting confused about the default units, and I
can see how they would get confused.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Bruce Momjian escreveu: > Someone complained that the default units for log_min_duration_statement > was not mentioned in postgresql.conf; this applied patch fixes that. > IMHO -1ms doesn't make much sense. Also, some other parameters don't have units too. They are: archive_timeout, autovacuum_vacuum_cost_delay, log_autovacuum_min_duration, log_temp_files, statement_timeout, tcp_keepalives_idle, and tcp_keepalives_interval. What about mention the unit in the comment that follows it? The last two already have it. -- Euler Taveira de Oliveira http://www.timbira.com/
Bruce Momjian <bruce@momjian.us> writes:
> It seems illogical to document it this way
Yeah, that's what I thought, so why did you do it that way?
You could do something like
#log_min_duration_statement = -1 # -1 is disabled, 0 logs all statements
# default unit is ms
regards, tom lane
Euler Taveira de Oliveira wrote: > Bruce Momjian escreveu: > > Someone complained that the default units for log_min_duration_statement > > was not mentioned in postgresql.conf; this applied patch fixes that. > > > IMHO -1ms doesn't make much sense. Also, some other parameters don't have > units too. They are: archive_timeout, autovacuum_vacuum_cost_delay, > log_autovacuum_min_duration, log_temp_files, statement_timeout, > tcp_keepalives_idle, and tcp_keepalives_interval. What about mention the unit > in the comment that follows it? The last two already have it. Thanks for the list! Here is an applied patch that cleans these up, plus fixes log_min_duration_statement. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian wrote: > Euler Taveira de Oliveira wrote: > > Bruce Momjian escreveu: > > > Someone complained that the default units for log_min_duration_statement > > > was not mentioned in postgresql.conf; this applied patch fixes that. > > > > > IMHO -1ms doesn't make much sense. Also, some other parameters don't have > > units too. They are: archive_timeout, autovacuum_vacuum_cost_delay, > > log_autovacuum_min_duration, log_temp_files, statement_timeout, > > tcp_keepalives_idle, and tcp_keepalives_interval. What about mention the unit > > in the comment that follows it? The last two already have it. > > Thanks for the list! Here is an applied patch that cleans these up, > plus fixes log_min_duration_statement. Oops, here is the patch; Euler, would you look to see if you can find anything else missing; you seem to be good at that. :-) -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/backend/utils/misc/guc.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/utils/misc/guc.c,v retrieving revision 1.499 diff -c -c -r1.499 guc.c *** src/backend/utils/misc/guc.c 2 Apr 2009 19:57:19 -0000 1.499 --- src/backend/utils/misc/guc.c 6 Apr 2009 20:56:57 -0000 *************** *** 1530,1536 **** { {"statement_timeout", PGC_USERSET, CLIENT_CONN_STATEMENT, ! gettext_noop("Sets the maximum allowed duration of any statement."), gettext_noop("A value of 0 turns off the timeout."), GUC_UNIT_MS }, --- 1530,1536 ---- { {"statement_timeout", PGC_USERSET, CLIENT_CONN_STATEMENT, ! gettext_noop("Sets the maximum duration of any statement, in milliseconds."), gettext_noop("A value of 0 turns off the timeout."), GUC_UNIT_MS }, *************** *** 1684,1690 **** { {"log_autovacuum_min_duration", PGC_SIGHUP, LOGGING_WHAT, ! gettext_noop("Sets the minimum execution time above which " "autovacuum actions will be logged."), gettext_noop("Zero prints all actions. -1 turns autovacuum logging off."), GUC_UNIT_MS --- 1684,1690 ---- { {"log_autovacuum_min_duration", PGC_SIGHUP, LOGGING_WHAT, ! gettext_noop("Sets the minimum execution milliseconds above which " "autovacuum actions will be logged."), gettext_noop("Zero prints all actions. -1 turns autovacuum logging off."), GUC_UNIT_MS *************** *** 1871,1877 **** { {"tcp_keepalives_idle", PGC_USERSET, CLIENT_CONN_OTHER, ! gettext_noop("Time between issuing TCP keepalives."), gettext_noop("A value of 0 uses the system default."), GUC_UNIT_S }, --- 1871,1877 ---- { {"tcp_keepalives_idle", PGC_USERSET, CLIENT_CONN_OTHER, ! gettext_noop("Seconds between issuing TCP keepalives."), gettext_noop("A value of 0 uses the system default."), GUC_UNIT_S }, *************** *** 1881,1887 **** { {"tcp_keepalives_interval", PGC_USERSET, CLIENT_CONN_OTHER, ! gettext_noop("Time between TCP keepalive retransmits."), gettext_noop("A value of 0 uses the system default."), GUC_UNIT_S }, --- 1881,1887 ---- { {"tcp_keepalives_interval", PGC_USERSET, CLIENT_CONN_OTHER, ! gettext_noop("Seconds between TCP keepalive retransmits."), gettext_noop("A value of 0 uses the system default."), GUC_UNIT_S }, Index: src/backend/utils/misc/postgresql.conf.sample =================================================================== RCS file: /cvsroot/pgsql/src/backend/utils/misc/postgresql.conf.sample,v retrieving revision 1.257 diff -c -c -r1.257 postgresql.conf.sample *** src/backend/utils/misc/postgresql.conf.sample 6 Apr 2009 19:03:04 -0000 1.257 --- src/backend/utils/misc/postgresql.conf.sample 6 Apr 2009 20:56:57 -0000 *************** *** 122,128 **** # - Cost-Based Vacuum Delay - ! #vacuum_cost_delay = 0 # 0-100 milliseconds #vacuum_cost_page_hit = 1 # 0-10000 credits #vacuum_cost_page_miss = 10 # 0-10000 credits #vacuum_cost_page_dirty = 20 # 0-10000 credits --- 122,128 ---- # - Cost-Based Vacuum Delay - ! #vacuum_cost_delay = 0ms # 0-100 milliseconds #vacuum_cost_page_hit = 1 # 0-10000 credits #vacuum_cost_page_miss = 10 # 0-10000 credits #vacuum_cost_page_dirty = 20 # 0-10000 credits *************** *** 175,181 **** # (change requires restart) #archive_command = '' # command to use to archive a logfile segment #archive_timeout = 0 # force a logfile segment switch after this ! # time; 0 is off #------------------------------------------------------------------------------ --- 175,181 ---- # (change requires restart) #archive_command = '' # command to use to archive a logfile segment #archive_timeout = 0 # force a logfile segment switch after this ! # number of seconds; 0 is off #------------------------------------------------------------------------------ *************** *** 306,314 **** # fatal # panic (effectively off) ! #log_min_duration_statement = -1ms # -1 is disabled, 0 logs all statements # and their durations, > 0 logs only ! # statements running at least this time. #silent_mode = off # DO NOT USE without syslog or # logging_collector --- 306,315 ---- # fatal # panic (effectively off) ! #log_min_duration_statement = -1 # -1 is disabled, 0 logs all statements # and their durations, > 0 logs only ! # statements running at least this number ! # of milliseconds #silent_mode = off # DO NOT USE without syslog or # logging_collector *************** *** 346,352 **** #log_lock_waits = off # log lock waits >= deadlock_timeout #log_statement = 'none' # none, ddl, mod, all #log_temp_files = -1 # log temporary files equal or larger ! # than specified size; # -1 disables, 0 logs all temp files #log_timezone = unknown # actually, defaults to TZ environment # setting --- 347,353 ---- #log_lock_waits = off # log lock waits >= deadlock_timeout #log_statement = 'none' # none, ddl, mod, all #log_temp_files = -1 # log temporary files equal or larger ! # than the specified size in kilobytes; # -1 disables, 0 logs all temp files #log_timezone = unknown # actually, defaults to TZ environment # setting *************** *** 382,388 **** # requires track_counts to also be on. #log_autovacuum_min_duration = -1 # -1 disables, 0 logs all actions and # their durations, > 0 logs only ! # actions running at least that time. #autovacuum_max_workers = 3 # max number of autovacuum subprocesses #autovacuum_naptime = 1min # time between autovacuum runs #autovacuum_vacuum_threshold = 50 # min number of row updates before --- 383,390 ---- # requires track_counts to also be on. #log_autovacuum_min_duration = -1 # -1 disables, 0 logs all actions and # their durations, > 0 logs only ! # actions running at least this number ! # of milliseconds. #autovacuum_max_workers = 3 # max number of autovacuum subprocesses #autovacuum_naptime = 1min # time between autovacuum runs #autovacuum_vacuum_threshold = 50 # min number of row updates before *************** *** 393,401 **** #autovacuum_analyze_scale_factor = 0.1 # fraction of table size before analyze #autovacuum_freeze_max_age = 200000000 # maximum XID age before forced vacuum # (change requires restart) ! #autovacuum_vacuum_cost_delay = 20 # default vacuum cost delay for ! # autovacuum, -1 means use ! # vacuum_cost_delay #autovacuum_vacuum_cost_limit = -1 # default vacuum cost limit for # autovacuum, -1 means use # vacuum_cost_limit --- 395,403 ---- #autovacuum_analyze_scale_factor = 0.1 # fraction of table size before analyze #autovacuum_freeze_max_age = 200000000 # maximum XID age before forced vacuum # (change requires restart) ! #autovacuum_vacuum_cost_delay = 20ms # default vacuum cost delay for ! # autovacuum, in milliseconds; ! # -1 means use vacuum_cost_delay #autovacuum_vacuum_cost_limit = -1 # default vacuum cost limit for # autovacuum, -1 means use # vacuum_cost_limit *************** *** 415,421 **** #default_transaction_isolation = 'read committed' #default_transaction_read_only = off #session_replication_role = 'origin' ! #statement_timeout = 0 # 0 is disabled #vacuum_freeze_min_age = 50000000 #vacuum_freeze_table_age = 150000000 #xmlbinary = 'base64' --- 417,423 ---- #default_transaction_isolation = 'read committed' #default_transaction_read_only = off #session_replication_role = 'origin' ! #statement_timeout = 0 # in milliseconds, 0 is disabled #vacuum_freeze_min_age = 50000000 #vacuum_freeze_table_age = 150000000 #xmlbinary = 'base64'
Bruce Momjian escreveu: > Oops, here is the patch; Euler, would you look to see if you can find > anything else missing; you seem to be good at that. :-) > My English is not as good as yours but here is another try. Personally, I prefer the second one but... -- Euler Taveira de Oliveira http://www.timbira.com/
Attachment
Euler Taveira de Oliveira wrote: > Bruce Momjian escreveu: > > Oops, here is the patch; Euler, would you look to see if you can find > > anything else missing; you seem to be good at that. :-) > > > My English is not as good as yours but here is another try. Personally, I > prefer the second one but... Great, I used your second version. I had already done some of the ones you found, but you had many more. I also used your "0 disables" wording consistently in the file. Thanks for the help. Committed. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/backend/utils/misc/guc.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/utils/misc/guc.c,v retrieving revision 1.500 diff -c -c -r1.500 guc.c *** src/backend/utils/misc/guc.c 6 Apr 2009 21:00:52 -0000 1.500 --- src/backend/utils/misc/guc.c 7 Apr 2009 22:15:25 -0000 *************** *** 1316,1322 **** { /* This is PGC_SIGHUP so all backends have the same value. */ {"deadlock_timeout", PGC_SIGHUP, LOCK_MANAGEMENT, ! gettext_noop("Sets the time to wait on a lock before checking for deadlock."), NULL, GUC_UNIT_MS }, --- 1316,1322 ---- { /* This is PGC_SIGHUP so all backends have the same value. */ {"deadlock_timeout", PGC_SIGHUP, LOCK_MANAGEMENT, ! gettext_noop("Sets the time to wait on a lock before checking for deadlock, in milliseconds."), NULL, GUC_UNIT_MS }, *************** *** 1406,1412 **** { {"maintenance_work_mem", PGC_USERSET, RESOURCES_MEM, ! gettext_noop("Sets the maximum memory to be used for maintenance operations."), gettext_noop("This includes operations such as VACUUM and CREATE INDEX."), GUC_UNIT_KB }, --- 1406,1412 ---- { {"maintenance_work_mem", PGC_USERSET, RESOURCES_MEM, ! gettext_noop("Sets the maximum memory to be used for maintenance operations, in kilobytes."), gettext_noop("This includes operations such as VACUUM and CREATE INDEX."), GUC_UNIT_KB }, *************** *** 1569,1575 **** { {"authentication_timeout", PGC_SIGHUP, CONN_AUTH_SECURITY, ! gettext_noop("Sets the maximum allowed time to complete client authentication."), NULL, GUC_UNIT_S }, --- 1569,1575 ---- { {"authentication_timeout", PGC_SIGHUP, CONN_AUTH_SECURITY, ! gettext_noop("Sets the maximum allowed time to complete client authentication, in seconds."), NULL, GUC_UNIT_S }, *************** *** 1599,1605 **** { {"checkpoint_timeout", PGC_SIGHUP, WAL_CHECKPOINTS, ! gettext_noop("Sets the maximum time between automatic WAL checkpoints."), NULL, GUC_UNIT_S }, --- 1599,1605 ---- { {"checkpoint_timeout", PGC_SIGHUP, WAL_CHECKPOINTS, ! gettext_noop("Sets the maximum time between automatic WAL checkpoints, in seconds."), NULL, GUC_UNIT_S }, *************** *** 1632,1638 **** { {"wal_writer_delay", PGC_SIGHUP, WAL_SETTINGS, ! gettext_noop("WAL writer sleep time between WAL flushes."), NULL, GUC_UNIT_MS }, --- 1632,1638 ---- { {"wal_writer_delay", PGC_SIGHUP, WAL_SETTINGS, ! gettext_noop("WAL writer sleep time between WAL flushes, in milliseconds."), NULL, GUC_UNIT_MS }, *************** *** 1673,1680 **** { {"log_min_duration_statement", PGC_SUSET, LOGGING_WHEN, ! gettext_noop("Sets the minimum execution time above which " ! "statements will be logged."), gettext_noop("Zero prints all queries. -1 turns this feature off."), GUC_UNIT_MS }, --- 1673,1680 ---- { {"log_min_duration_statement", PGC_SUSET, LOGGING_WHEN, ! gettext_noop("Sets the minimum execution time (in milliseconds) above " ! "which statements will be logged."), gettext_noop("Zero prints all queries. -1 turns this feature off."), GUC_UNIT_MS }, *************** *** 1684,1690 **** { {"log_autovacuum_min_duration", PGC_SIGHUP, LOGGING_WHAT, ! gettext_noop("Sets the minimum execution milliseconds above which " "autovacuum actions will be logged."), gettext_noop("Zero prints all actions. -1 turns autovacuum logging off."), GUC_UNIT_MS --- 1684,1690 ---- { {"log_autovacuum_min_duration", PGC_SIGHUP, LOGGING_WHAT, ! gettext_noop("Sets the minimum execution in milliseconds above which " "autovacuum actions will be logged."), gettext_noop("Zero prints all actions. -1 turns autovacuum logging off."), GUC_UNIT_MS *************** *** 1695,1701 **** { {"bgwriter_delay", PGC_SIGHUP, RESOURCES, ! gettext_noop("Background writer sleep time between rounds."), NULL, GUC_UNIT_MS }, --- 1695,1701 ---- { {"bgwriter_delay", PGC_SIGHUP, RESOURCES, ! gettext_noop("Background writer sleep time between rounds, in milliseconds."), NULL, GUC_UNIT_MS }, *************** *** 1827,1833 **** { {"autovacuum_naptime", PGC_SIGHUP, AUTOVACUUM, ! gettext_noop("Time to sleep between autovacuum runs."), NULL, GUC_UNIT_S }, --- 1827,1833 ---- { {"autovacuum_naptime", PGC_SIGHUP, AUTOVACUUM, ! gettext_noop("Time to sleep between autovacuum runs, in seconds."), NULL, GUC_UNIT_S }, Index: src/backend/utils/misc/postgresql.conf.sample =================================================================== RCS file: /cvsroot/pgsql/src/backend/utils/misc/postgresql.conf.sample,v retrieving revision 1.258 diff -c -c -r1.258 postgresql.conf.sample *** src/backend/utils/misc/postgresql.conf.sample 6 Apr 2009 21:00:52 -0000 1.258 --- src/backend/utils/misc/postgresql.conf.sample 7 Apr 2009 22:15:25 -0000 *************** *** 136,142 **** # - Asynchronous Behavior - ! #effective_io_concurrency = 1 # 1-1000, or 0 to disable prefetching #------------------------------------------------------------------------------ --- 136,142 ---- # - Asynchronous Behavior - ! #effective_io_concurrency = 1 # 1-1000. 0 disables prefetching #------------------------------------------------------------------------------ *************** *** 167,173 **** #checkpoint_segments = 3 # in logfile segments, min 1, 16MB each #checkpoint_timeout = 5min # range 30s-1h #checkpoint_completion_target = 0.5 # checkpoint target duration, 0.0 - 1.0 ! #checkpoint_warning = 30s # 0 is off # - Archiving - --- 167,173 ---- #checkpoint_segments = 3 # in logfile segments, min 1, 16MB each #checkpoint_timeout = 5min # range 30s-1h #checkpoint_completion_target = 0.5 # checkpoint target duration, 0.0 - 1.0 ! #checkpoint_warning = 30s # 0 disables # - Archiving - *************** *** 175,181 **** # (change requires restart) #archive_command = '' # command to use to archive a logfile segment #archive_timeout = 0 # force a logfile segment switch after this ! # number of seconds; 0 is off #------------------------------------------------------------------------------ --- 175,181 ---- # (change requires restart) #archive_command = '' # command to use to archive a logfile segment #archive_timeout = 0 # force a logfile segment switch after this ! # number of seconds; 0 disables #------------------------------------------------------------------------------ *************** *** 253,262 **** # off, meaning append to existing files # in all cases. #log_rotation_age = 1d # Automatic rotation of logfiles will ! # happen after that time. 0 to disable. #log_rotation_size = 10MB # Automatic rotation of logfiles will # happen after that much log output. ! # 0 to disable. # These are relevant when logging to syslog: #syslog_facility = 'LOCAL0' --- 253,262 ---- # off, meaning append to existing files # in all cases. #log_rotation_age = 1d # Automatic rotation of logfiles will ! # happen after that time. 0 disables. #log_rotation_size = 10MB # Automatic rotation of logfiles will # happen after that much log output. ! # 0 disables. # These are relevant when logging to syslog: #syslog_facility = 'LOCAL0'
Bruce Momjian wrote: > Euler Taveira de Oliveira wrote: > > Bruce Momjian escreveu: > > > Oops, here is the patch; Euler, would you look to see if you can find > > > anything else missing; you seem to be good at that. :-) > > > > > My English is not as good as yours but here is another try. Personally, I > > prefer the second one but... > > Great, I used your second version. I had already done some of the ones > you found, but you had many more. I also used your "0 disables" wording > consistently in the file. Thanks for the help. Committed. Huh ... I thought the units were removed from the descriptions when the whole unit thing got introduced in guc.c. This seems more a regression than anything else -- see guc.c rev 1.361. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera wrote:
> Bruce Momjian wrote:
> > Euler Taveira de Oliveira wrote:
> > > Bruce Momjian escreveu:
> > > > Oops, here is the patch; Euler, would you look to see if you can find
> > > > anything else missing; you seem to be good at that. :-)
> > > >
> > > My English is not as good as yours but here is another try. Personally, I
> > > prefer the second one but...
> >
> > Great, I used your second version. I had already done some of the ones
> > you found, but you had many more. I also used your "0 disables" wording
> > consistently in the file. Thanks for the help. Committed.
>
> Huh ... I thought the units were removed from the descriptions when the
> whole unit thing got introduced in guc.c. This seems more a regression
> than anything else -- see guc.c rev 1.361.
Which is why I originally did:
#log_min_duration_statement = -1ms # -1 is disabled, 0 logs all statements
but people complained so I went with adding it to the description. The
units are only to document the default if no units are supplied.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
> Euler Taveira de Oliveira wrote:
>> My English is not as good as yours but here is another try. Personally, I
>> prefer the second one but...
> Great, I used your second version. I had already done some of the ones
> you found, but you had many more. I also used your "0 disables" wording
> consistently in the file. Thanks for the help. Committed.
You know, it suddenly strikes me that this is going in largely the wrong
direction. Wasn't a key part of the reasoning for the GUC units support
to *eliminate* the need for people to know what the underlying
variable's unit is? I certainly think that putting the unit info into
the text descriptions is a seriously bad idea. It makes an already
overly wide view even wider, and the information is 100% redundant with
the "unit" column of the pg_settings view.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > Euler Taveira de Oliveira wrote: > >> My English is not as good as yours but here is another try. Personally, I > >> prefer the second one but... > > > Great, I used your second version. I had already done some of the ones > > you found, but you had many more. I also used your "0 disables" wording > > consistently in the file. Thanks for the help. Committed. > > You know, it suddenly strikes me that this is going in largely the wrong > direction. Wasn't a key part of the reasoning for the GUC units support > to *eliminate* the need for people to know what the underlying > variable's unit is? I certainly think that putting the unit info into > the text descriptions is a seriously bad idea. It makes an already > overly wide view even wider, and the information is 100% redundant with > the "unit" column of the pg_settings view. Right, the problem particularly is with the -1/special values that don't have a real unit. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
> Tom Lane wrote:
>> You know, it suddenly strikes me that this is going in largely the wrong
>> direction.
> Right, the problem particularly is with the -1/special values that don't
> have a real unit.
I don't object to what you did to postgresql.conf.sample, but I think
you should revert the changes to the descriptions in guc.c.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > Tom Lane wrote: > >> You know, it suddenly strikes me that this is going in largely the wrong > >> direction. > > > Right, the problem particularly is with the -1/special values that don't > > have a real unit. > > I don't object to what you did to postgresql.conf.sample, but I think > you should revert the changes to the descriptions in guc.c. Agreed; attached patch applied. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: guc.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/utils/misc/guc.c,v retrieving revision 1.499 retrieving revision 1.501 diff -c -c -r1.499 -r1.501 *** guc.c 2 Apr 2009 19:57:19 -0000 1.499 --- guc.c 7 Apr 2009 22:22:19 -0000 1.501 *************** *** 10,16 **** * Written by Peter Eisentraut <peter_e@gmx.net>. * * IDENTIFICATION ! * $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.499 2009/04/02 19:57:19 momjian Exp $ * *-------------------------------------------------------------------- */ --- 10,16 ---- * Written by Peter Eisentraut <peter_e@gmx.net>. * * IDENTIFICATION ! * $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.501 2009/04/07 22:22:19 momjian Exp $ * *-------------------------------------------------------------------- */ *************** *** 1316,1322 **** { /* This is PGC_SIGHUP so all backends have the same value. */ {"deadlock_timeout", PGC_SIGHUP, LOCK_MANAGEMENT, ! gettext_noop("Sets the time to wait on a lock before checking for deadlock."), NULL, GUC_UNIT_MS }, --- 1316,1322 ---- { /* This is PGC_SIGHUP so all backends have the same value. */ {"deadlock_timeout", PGC_SIGHUP, LOCK_MANAGEMENT, ! gettext_noop("Sets the time to wait on a lock before checking for deadlock, in milliseconds."), NULL, GUC_UNIT_MS }, *************** *** 1406,1412 **** { {"maintenance_work_mem", PGC_USERSET, RESOURCES_MEM, ! gettext_noop("Sets the maximum memory to be used for maintenance operations."), gettext_noop("This includes operations such as VACUUM and CREATE INDEX."), GUC_UNIT_KB }, --- 1406,1412 ---- { {"maintenance_work_mem", PGC_USERSET, RESOURCES_MEM, ! gettext_noop("Sets the maximum memory to be used for maintenance operations, in kilobytes."), gettext_noop("This includes operations such as VACUUM and CREATE INDEX."), GUC_UNIT_KB }, *************** *** 1530,1536 **** { {"statement_timeout", PGC_USERSET, CLIENT_CONN_STATEMENT, ! gettext_noop("Sets the maximum allowed duration of any statement."), gettext_noop("A value of 0 turns off the timeout."), GUC_UNIT_MS }, --- 1530,1536 ---- { {"statement_timeout", PGC_USERSET, CLIENT_CONN_STATEMENT, ! gettext_noop("Sets the maximum duration of any statement, in milliseconds."), gettext_noop("A value of 0 turns off the timeout."), GUC_UNIT_MS }, *************** *** 1569,1575 **** { {"authentication_timeout", PGC_SIGHUP, CONN_AUTH_SECURITY, ! gettext_noop("Sets the maximum allowed time to complete client authentication."), NULL, GUC_UNIT_S }, --- 1569,1575 ---- { {"authentication_timeout", PGC_SIGHUP, CONN_AUTH_SECURITY, ! gettext_noop("Sets the maximum allowed time to complete client authentication, in seconds."), NULL, GUC_UNIT_S }, *************** *** 1599,1605 **** { {"checkpoint_timeout", PGC_SIGHUP, WAL_CHECKPOINTS, ! gettext_noop("Sets the maximum time between automatic WAL checkpoints."), NULL, GUC_UNIT_S }, --- 1599,1605 ---- { {"checkpoint_timeout", PGC_SIGHUP, WAL_CHECKPOINTS, ! gettext_noop("Sets the maximum time between automatic WAL checkpoints, in seconds."), NULL, GUC_UNIT_S }, *************** *** 1632,1638 **** { {"wal_writer_delay", PGC_SIGHUP, WAL_SETTINGS, ! gettext_noop("WAL writer sleep time between WAL flushes."), NULL, GUC_UNIT_MS }, --- 1632,1638 ---- { {"wal_writer_delay", PGC_SIGHUP, WAL_SETTINGS, ! gettext_noop("WAL writer sleep time between WAL flushes, in milliseconds."), NULL, GUC_UNIT_MS }, *************** *** 1673,1680 **** { {"log_min_duration_statement", PGC_SUSET, LOGGING_WHEN, ! gettext_noop("Sets the minimum execution time above which " ! "statements will be logged."), gettext_noop("Zero prints all queries. -1 turns this feature off."), GUC_UNIT_MS }, --- 1673,1680 ---- { {"log_min_duration_statement", PGC_SUSET, LOGGING_WHEN, ! gettext_noop("Sets the minimum execution time (in milliseconds) above " ! "which statements will be logged."), gettext_noop("Zero prints all queries. -1 turns this feature off."), GUC_UNIT_MS }, *************** *** 1684,1690 **** { {"log_autovacuum_min_duration", PGC_SIGHUP, LOGGING_WHAT, ! gettext_noop("Sets the minimum execution time above which " "autovacuum actions will be logged."), gettext_noop("Zero prints all actions. -1 turns autovacuum logging off."), GUC_UNIT_MS --- 1684,1690 ---- { {"log_autovacuum_min_duration", PGC_SIGHUP, LOGGING_WHAT, ! gettext_noop("Sets the minimum execution in milliseconds above which " "autovacuum actions will be logged."), gettext_noop("Zero prints all actions. -1 turns autovacuum logging off."), GUC_UNIT_MS *************** *** 1695,1701 **** { {"bgwriter_delay", PGC_SIGHUP, RESOURCES, ! gettext_noop("Background writer sleep time between rounds."), NULL, GUC_UNIT_MS }, --- 1695,1701 ---- { {"bgwriter_delay", PGC_SIGHUP, RESOURCES, ! gettext_noop("Background writer sleep time between rounds, in milliseconds."), NULL, GUC_UNIT_MS }, *************** *** 1827,1833 **** { {"autovacuum_naptime", PGC_SIGHUP, AUTOVACUUM, ! gettext_noop("Time to sleep between autovacuum runs."), NULL, GUC_UNIT_S }, --- 1827,1833 ---- { {"autovacuum_naptime", PGC_SIGHUP, AUTOVACUUM, ! gettext_noop("Time to sleep between autovacuum runs, in seconds."), NULL, GUC_UNIT_S }, *************** *** 1871,1877 **** { {"tcp_keepalives_idle", PGC_USERSET, CLIENT_CONN_OTHER, ! gettext_noop("Time between issuing TCP keepalives."), gettext_noop("A value of 0 uses the system default."), GUC_UNIT_S }, --- 1871,1877 ---- { {"tcp_keepalives_idle", PGC_USERSET, CLIENT_CONN_OTHER, ! gettext_noop("Seconds between issuing TCP keepalives."), gettext_noop("A value of 0 uses the system default."), GUC_UNIT_S }, *************** *** 1881,1887 **** { {"tcp_keepalives_interval", PGC_USERSET, CLIENT_CONN_OTHER, ! gettext_noop("Time between TCP keepalive retransmits."), gettext_noop("A value of 0 uses the system default."), GUC_UNIT_S }, --- 1881,1887 ---- { {"tcp_keepalives_interval", PGC_USERSET, CLIENT_CONN_OTHER, ! gettext_noop("Seconds between TCP keepalive retransmits."), gettext_noop("A value of 0 uses the system default."), GUC_UNIT_S },
Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian <bruce@momjian.us> writes:
> > > Tom Lane wrote:
> > >> You know, it suddenly strikes me that this is going in largely the wrong
> > >> direction.
> >
> > > Right, the problem particularly is with the -1/special values that don't
> > > have a real unit.
> >
> > I don't object to what you did to postgresql.conf.sample, but I think
> > you should revert the changes to the descriptions in guc.c.
>
> Agreed; attached patch applied.
However, keep in mind that units are _not_ displayed for zero values:
test=> show statement_timeout;
statement_timeout
-------------------
0
(1 row)
test=> set statement_timeout = 100;
SET
test=> show statement_timeout;
statement_timeout
-------------------
100ms
(1 row)
because of this test in guc.c:
else if (use_units && result > 0 &&
(record->flags & GUC_UNIT_TIME))
pg_settings does show the units. Changing this to display the units
might be odd since many zero values mean 'disabled'.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
> However, keep in mind that units are _not_ displayed for zero values:
Well, zero is zero, so that seems fine.
regards, tom lane