Patch: Tie stats options to autovacuum in postgresql.conf

Started by David Wheelerover 19 years ago11 messages
#1David Wheeler
david@kineticode.com
1 attachment(s)

PostgreSQLers,

I just ran into an issue where a client thought that autovacuum was
running but it wasn't. This is because it's not fatal when autovacuum
is on but stats_start_collector and/or stats_row_level is off. I
suspect that there's a reason that it's not fatal, so I thought that
it might be useful to give folks just a little bit of help by telling
them in postgresql.conf that they need to enable them in order for
autovacuum to work.

If this patch is not correctly formatted or against the proper file,
please let me know and I'll make the necessary modifications.

Thanks,

David

Attachments:

vacuam_stats.patchapplication/octet-stream; name=vacuam_stats.patch; x-unix-mode=0644Download
Index: src/backend/utils/misc/postgresql.conf.sample
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/postgresql.conf.sample,v
retrieving revision 1.194
diff -u -r1.194 postgresql.conf.sample
--- src/backend/utils/misc/postgresql.conf.sample	25 Sep 2006 22:12:24 -0000	1.194
+++ src/backend/utils/misc/postgresql.conf.sample	28 Sep 2006 22:04:17 -0000
@@ -362,6 +362,8 @@
 #---------------------------------------------------------------------------
 
 #autovacuum = off			# enable autovacuum subprocess?
+					# be sure to also enable stats_start_collector
+					# and stats_row_level when autovacuum is on
 #autovacuum_naptime = 1min		# time between autovacuum runs
 #autovacuum_vacuum_threshold = 500	# min # of tuple updates before
 					# vacuum
#2Jim C. Nasby
jim@nasby.net
In reply to: David Wheeler (#1)
Re: Patch: Tie stats options to autovacuum in postgresql.conf

On Thu, Sep 28, 2006 at 03:07:39PM -0700, David Wheeler wrote:

PostgreSQLers,

I just ran into an issue where a client thought that autovacuum was
running but it wasn't. This is because it's not fatal when autovacuum
is on but stats_start_collector and/or stats_row_level is off. I
suspect that there's a reason that it's not fatal, so I thought that
it might be useful to give folks just a little bit of help by telling
them in postgresql.conf that they need to enable them in order for
autovacuum to work.

+1. I was just at a client today that had run into this problem.

Actually, I'm in favor of refusing to start if autovac is on but the
proper stats settings aren't. I'd rather that then people ending up with
bloated databases and crappy performance.
--
Jim Nasby jim@nasby.net
EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)

#3David E. Wheeler
david@kineticode.com
In reply to: Jim C. Nasby (#2)
Re: Patch: Tie stats options to autovacuum in postgresql.conf

On Sep 28, 2006, at 16:39, Jim C. Nasby wrote:

+1. I was just at a client today that had run into this problem.

Actually, I'm in favor of refusing to start if autovac is on but the
proper stats settings aren't. I'd rather that then people ending up
with
bloated databases and crappy performance.

I agree, but I figured that this was a start, at least.

Best,

David

#4Florian G. Pflug
fgp@phlo.org
In reply to: Jim C. Nasby (#2)
Re: Patch: Tie stats options to autovacuum in postgresql.conf

Jim C. Nasby wrote:

On Thu, Sep 28, 2006 at 03:07:39PM -0700, David Wheeler wrote:

PostgreSQLers,

I just ran into an issue where a client thought that autovacuum was
running but it wasn't. This is because it's not fatal when autovacuum
is on but stats_start_collector and/or stats_row_level is off. I
suspect that there's a reason that it's not fatal, so I thought that
it might be useful to give folks just a little bit of help by telling
them in postgresql.conf that they need to enable them in order for
autovacuum to work.

+1. I was just at a client today that had run into this problem.

Actually, I'm in favor of refusing to start if autovac is on but the
proper stats settings aren't. I'd rather that then people ending up with
bloated databases and crappy performance.

If think that setting autovacuum to on should even force
stats_collector and stats_row_level to on - together with a warning if
they would otherwise be off.

The risk of autovacuum being disabled by accident seems to risk a much
worse performance penatly then having the statistics collector running
by accident. Additionally, the statistics collector can easily be turned
off within seconds even _if_ it was on accidentally, but if vacuuming was
disabled by accident, the user might have to run "vacuum full" - with all
the concurrency issues that this implies..

greetings, Florian flug

#5Bruce Momjian
bruce@momjian.us
In reply to: David Wheeler (#1)
1 attachment(s)
Re: [HACKERS] Patch: Tie stats options to autovacuum in

Modified wording patch applied. Thanks.

---------------------------------------------------------------------------

David Wheeler wrote:

PostgreSQLers,

I just ran into an issue where a client thought that autovacuum was
running but it wasn't. This is because it's not fatal when autovacuum
is on but stats_start_collector and/or stats_row_level is off. I
suspect that there's a reason that it's not fatal, so I thought that
it might be useful to give folks just a little bit of help by telling
them in postgresql.conf that they need to enable them in order for
autovacuum to work.

If this patch is not correctly formatted or against the proper file,
please let me know and I'll make the necessary modifications.

Thanks,

David

[ Attachment, skipping... ]

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

--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Attachments:

/bjm/difftext/x-diffDownload
Index: src/backend/utils/misc/postgresql.conf.sample
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/misc/postgresql.conf.sample,v
retrieving revision 1.194
diff -c -c -r1.194 postgresql.conf.sample
*** src/backend/utils/misc/postgresql.conf.sample	25 Sep 2006 22:12:24 -0000	1.194
--- src/backend/utils/misc/postgresql.conf.sample	3 Oct 2006 00:37:48 -0000
***************
*** 362,367 ****
--- 362,369 ----
  #---------------------------------------------------------------------------
  
  #autovacuum = off			# enable autovacuum subprocess?
+ 					# 'on' requires stats_start_collector
+ 					# and stats_row_level to also be on
  #autovacuum_naptime = 1min		# time between autovacuum runs
  #autovacuum_vacuum_threshold = 500	# min # of tuple updates before
  					# vacuum
#6Bruce Momjian
bruce@momjian.us
In reply to: David E. Wheeler (#3)
Re: Patch: Tie stats options to autovacuum in

David E. Wheeler wrote:

On Sep 28, 2006, at 16:39, Jim C. Nasby wrote:

+1. I was just at a client today that had run into this problem.

Actually, I'm in favor of refusing to start if autovac is on but the
proper stats settings aren't. I'd rather that then people ending up
with
bloated databases and crappy performance.

I agree, but I figured that this was a start, at least.

The problem with refusing to start is that autovacuum is sighup, so you
might modify postgresql.conf and do a server restart, and then the
server doesn't start. Are people OK with that?

I did apply the postgresql.conf comment addition.

--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

#7David E. Wheeler
david@kineticode.com
In reply to: Bruce Momjian (#5)
Re: [HACKERS] Patch: Tie stats options to autovacuum in postgresql.conf

On Oct 2, 2006, at 17:40, Bruce Momjian wrote:

Modified wording patch applied. Thanks.

Great, thanks Bruce.

Best,

David

#8Jim Nasby
jim@nasby.net
In reply to: Bruce Momjian (#6)
Re: Patch: Tie stats options to autovacuum in

On Oct 2, 2006, at 8:41 PM, Bruce Momjian wrote:

David E. Wheeler wrote:

On Sep 28, 2006, at 16:39, Jim C. Nasby wrote:

+1. I was just at a client today that had run into this problem.

Actually, I'm in favor of refusing to start if autovac is on but the
proper stats settings aren't. I'd rather that then people ending up
with
bloated databases and crappy performance.

I agree, but I figured that this was a start, at least.

The problem with refusing to start is that autovacuum is sighup, so
you
might modify postgresql.conf and do a server restart, and then the
server doesn't start. Are people OK with that?

I did apply the postgresql.conf comment addition.

Hrm... how about if the options are incompatible on HUP we refuse to
pick up any new settings and complain loudly?

Perhaps it would be easier to just override stats_row_level if
autovac is on. Doesn't necessarily meet the least surprise test, but
it does protect newbies which I feel is more important in this case.
--
Jim Nasby jim@nasby.net
EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)

#9Bruce Momjian
bruce@momjian.us
In reply to: Jim Nasby (#8)
Re: Patch: Tie stats options to autovacuum in

Jim Nasby wrote:

On Oct 2, 2006, at 8:41 PM, Bruce Momjian wrote:

David E. Wheeler wrote:

On Sep 28, 2006, at 16:39, Jim C. Nasby wrote:

+1. I was just at a client today that had run into this problem.

Actually, I'm in favor of refusing to start if autovac is on but the
proper stats settings aren't. I'd rather that then people ending up
with
bloated databases and crappy performance.

I agree, but I figured that this was a start, at least.

The problem with refusing to start is that autovacuum is sighup, so
you
might modify postgresql.conf and do a server restart, and then the
server doesn't start. Are people OK with that?

I did apply the postgresql.conf comment addition.

Hrm... how about if the options are incompatible on HUP we refuse to
pick up any new settings and complain loudly?

We don't read postgresql.conf as a test and then set values.

--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

#10Jim Nasby
jim@nasby.net
In reply to: Bruce Momjian (#9)
Re: Patch: Tie stats options to autovacuum in

On Oct 2, 2006, at 9:17 PM, Bruce Momjian wrote:

Jim Nasby wrote:

On Oct 2, 2006, at 8:41 PM, Bruce Momjian wrote:

David E. Wheeler wrote:

On Sep 28, 2006, at 16:39, Jim C. Nasby wrote:

+1. I was just at a client today that had run into this problem.

Actually, I'm in favor of refusing to start if autovac is on
but the
proper stats settings aren't. I'd rather that then people
ending up
with
bloated databases and crappy performance.

I agree, but I figured that this was a start, at least.

The problem with refusing to start is that autovacuum is sighup, so
you
might modify postgresql.conf and do a server restart, and then the
server doesn't start. Are people OK with that?

I did apply the postgresql.conf comment addition.

Hrm... how about if the options are incompatible on HUP we refuse to
pick up any new settings and complain loudly?

We don't read postgresql.conf as a test and then set values.

IMHO we should... if something got botched in the config file I'd
rather have it complain and not change anything instead of taking
just some of the changes. This will be even more important once the
code goes in to return to default values if something gets commented
out in the config.
--
Jim Nasby jim@nasby.net
EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)

#11Tom Lane
tgl@sss.pgh.pa.us
In reply to: Jim Nasby (#10)
Re: Patch: Tie stats options to autovacuum in

Jim Nasby <jim@nasby.net> writes:

On Oct 2, 2006, at 9:17 PM, Bruce Momjian wrote:

Jim Nasby wrote:

Hrm... how about if the options are incompatible on HUP we refuse to
pick up any new settings and complain loudly?

We don't read postgresql.conf as a test and then set values.

IMHO we should... if something got botched in the config file I'd
rather have it complain and not change anything instead of taking
just some of the changes.

Apparently, neither of you have read the code nor experimented with this
behavior.

The core reason why GUC variables should not be interdependent (as Jim
proposed upthread) is exactly that it would break the ability of
ProcessConfigFile to validate the new settings before applying them.

regards, tom lane