Thread: fillfactor hides autovacuum parameters in 8.4.0
PostgreSQL version: 8.4.0
Operating system: all versions
If we set FILLFACTOR to a table, autovacuum parameters in postgresql.conf
will not affect to the table; default values are always used.
In 8.4.0, we create StdRdOptions if a relation has some fields in
pg_class.reloption. Then, unspecified fields are filled with the
default values. The values are always hard-coded default values and
don't reflect current GUC settings.
For example:
pg_class.reloptions = {fillfactor=70}
makes
StdRdOptions { fillfactor=70, vacuum_scale_factor=0.2, ... }
~~~~~~~~~~~~~~~~~~~~~~~
always default
To fix the bug, each field in StdRdOptions should have "not-specified" flag.
If not specified, autovacuum should use current GUC settings instead of
reloptions. Is it possible to change the default values of reloptions
to some magic number (-1 or so) ?
There might be another idea that we fill StdRdOptions with current GUC
settings instead of hard-coded default values. It looks cleaner, but
we need to treat dynamic default values and invalidate reloptions
if we reload settings.
How do we fix it?
Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center
Itagaki Takahiro escreveu: > In 8.4.0, we create StdRdOptions if a relation has some fields in > pg_class.reloption. Then, unspecified fields are filled with the > default values. The values are always hard-coded default values and > don't reflect current GUC settings. > Hey, how I couldn't notice that. :( > To fix the bug, each field in StdRdOptions should have "not-specified" flag. > If not specified, autovacuum should use current GUC settings instead of > reloptions. Is it possible to change the default values of reloptions > to some magic number (-1 or so) ? > BTW, we agreed that magic numbers is a hack in pg_autovacuum and we wouldn't use it in reloptions [1]. > There might be another idea that we fill StdRdOptions with current GUC > settings instead of hard-coded default values. It looks cleaner, but > we need to treat dynamic default values and invalidate reloptions > if we reload settings. > +1. I'm afraid we need to touch too much code for a fix. Let's do it for 8.5. I'll try to come up with a patch. A not-so-invasive code is to transform all AutoVacOpts elements in pointers and to leave the default assignment to relation_needs_vacanalyze(). If nobody steps up I'll give it a stab later. [1] http://archives.postgresql.org/pgsql-hackers/2009-02/msg00303.php -- Euler Taveira de Oliveira http://www.timbira.com/
Itagaki Takahiro wrote: > PostgreSQL version: 8.4.0 > Operating system: all versions > > If we set FILLFACTOR to a table, autovacuum parameters in postgresql.conf > will not affect to the table; default values are always used. > > In 8.4.0, we create StdRdOptions if a relation has some fields in > pg_class.reloption. Then, unspecified fields are filled with the > default values. The values are always hard-coded default values and > don't reflect current GUC settings. Hmm, I'm fairly sure the code had this right originally ... maybe it got drowned in some of the infinite reworking of that patch and I neglected to test it. I'll have a look tomorrow. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Itagaki Takahiro wrote: > To fix the bug, each field in StdRdOptions should have "not-specified" flag. > If not specified, autovacuum should use current GUC settings instead of > reloptions. Is it possible to change the default values of reloptions > to some magic number (-1 or so) ? Ah. After checking the code I remember that this was right at some point, but it was lost when the parsing step was made to use a table (fillRelOptions). This behavior could be restored by having the fill routine have a callback of some sort; we could use that to set a boolean in StdRdOptions that indicated whether they are set in reloptions or are default values. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera <alvherre@commandprompt.com> wrote: > we could use that to set a boolean > in StdRdOptions that indicated whether they are set in reloptions or are > default values. Do you mean we will have a boolean field *for each* field in StdRdOptions? We should remember whether a field was specified or not independntly. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center
Here is a patch to fix a bug in handling default values in reloptions. This fix should be applied to HEAD and 8.4.0. I used 'magic number -1' to propagate "not-specified" information to autovacuum process. It might look strange because the default value is out of range of the reloption, but I think it has less impact to the codes comapred with other solutions (dynamic default values etc.). > To fix the bug, each field in StdRdOptions should have "not-specified" flag. > If not specified, autovacuum should use current GUC settings instead of > reloptions. Is it possible to change the default values of reloptions > to some magic number (-1 or so) ? Regards, --- ITAGAKI Takahiro NTT Open Source Software Center
Attachment
Itagaki Takahiro wrote: > > Alvaro Herrera <alvherre@commandprompt.com> wrote: > > > we could use that to set a boolean > > in StdRdOptions that indicated whether they are set in reloptions or are > > default values. > > Do you mean we will have a boolean field *for each* field in StdRdOptions? No, I was thinking in a single value for all of autovac options ... > We should remember whether a field was specified or not independntly. I'm not sure what you are suggesting ...? -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera <alvherre@commandprompt.com> wrote: > > We should remember whether a field was specified or not independntly. > I'm not sure what you are suggesting ...? Please imagine that: =# SET autovacuum_vacuum_scale_factor = 0.05; =# ALTER TABLE tbl SET (autovacuum_vacuum_threshold = 10); AutoVacOpts.vacuum_threshold should be 10 (comes from reloptions), but AutoVacOpts.vacuum_scale_factor should be <DEFAULT> (comes from GUC). If we use boolean flags, we need booleans for each fields in AutoVacOpts. (vacuum_threshold_is_default, vacuum_scale_factor_is_default, ...) Regards, --- ITAGAKI Takahiro NTT Open Source Software Center
Itagaki Takahiro wrote: > > Alvaro Herrera <alvherre@commandprompt.com> wrote: > > > > We should remember whether a field was specified or not independntly. > > I'm not sure what you are suggesting ...? > > Please imagine that: > > =# SET autovacuum_vacuum_scale_factor = 0.05; > =# ALTER TABLE tbl SET (autovacuum_vacuum_threshold = 10); Uh, but this doesn't work because you can't SET inside a session and have autovacuum read it. Only the values that you change with ALTER TABLE can be read by autovacuum; the rest of the values come from postgresql.conf. But I'm very dizzy today so perhaps I'm missing something obvious. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Itagaki Takahiro wrote: > > Here is a patch to fix a bug in handling default values in reloptions. > This fix should be applied to HEAD and 8.4.0. > > I used 'magic number -1' to propagate "not-specified" information to > autovacuum process. It might look strange because the default value is > out of range of the reloption, but I think it has less impact to the > codes comapred with other solutions (dynamic default values etc.). I realized that any other solution here is going to be more complex and thus less appropriate for backpatch. I still don't like this very much because it doesn't seem to offer enough flexibility to user-specified reloptions; but any patch we come up with here is going to be applicable to CVS HEAD. So I'm going to apply your patch to both 8.4 and HEAD; we can always improve it later, I guess. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Alvaro Herrera <alvherre@commandprompt.com> wrote: > So I'm going to apply your patch to both 8.4 and HEAD; we can always > improve it later, I guess. Thank you for your applying. I think the fix is ugly, too. We need to introduce cleaner solution for 8.5. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center