Bug in pg_autovacuum ?
I'm having a problem with pg_autovacuum against both 7.3.2 and 7.4.1 on
Redhat 9 and Fedora Core 1. I'm using pg_autovacuum from 7.4.1,
everything comes from the postgresql.org RPMs.
If the number of tuples is sufficiently high, pg reports 'reltuples'
back in TABLE_STATS_QUERY in scientific notation instead of an integer.
Example:
4.35351e+06 instead of 4353514
pg_autovacuum then uses atoi() to reach the incorrect conclusion that
there are 4 tuples. Obviously, if that table gets more than a couple of
updates, it gets vacuumed a bit too often. :)
Changing from atoi() to atof() solves the problem completely.
new_tbl->reltuples =
atof(PQgetvalue(res, row, PQfnumber(res, "reltuples")));
new_tbl->relpages =
atof(PQgetvalue(res, row, PQfnumber(res, "relpages")));
I'm not sure how I can be the only person running into this. :)
Cott Lang <cott@internetstaff.com> writes:
If the number of tuples is sufficiently high, pg reports 'reltuples'
back in TABLE_STATS_QUERY in scientific notation instead of an integer.
Right, because that column is actually a float4.
Changing from atoi() to atof() solves the problem completely.
new_tbl->reltuples =
atof(PQgetvalue(res, row, PQfnumber(res, "reltuples")));
new_tbl->relpages =
atof(PQgetvalue(res, row, PQfnumber(res, "relpages")));
I should think this would break in different ways once reltuples exceeds
INT_MAX. A full fix would require changing new_tbl->reltuples to be
float or double, and coping with any downstream changes that implies.
Also, relpages *is* an integer, though it's best interpreted as an
unsigned one. (Ditto for relid.) Looks like this code is 0-for-3 on
getting the datatypes right :-(
regards, tom lane
Would someone review these problems and submit a patch? Thanks.
---------------------------------------------------------------------------
Tom Lane wrote:
Cott Lang <cott@internetstaff.com> writes:
If the number of tuples is sufficiently high, pg reports 'reltuples'
back in TABLE_STATS_QUERY in scientific notation instead of an integer.Right, because that column is actually a float4.
Changing from atoi() to atof() solves the problem completely.
new_tbl->reltuples =
atof(PQgetvalue(res, row, PQfnumber(res, "reltuples")));new_tbl->relpages =
atof(PQgetvalue(res, row, PQfnumber(res, "relpages")));I should think this would break in different ways once reltuples exceeds
INT_MAX. A full fix would require changing new_tbl->reltuples to be
float or double, and coping with any downstream changes that implies.Also, relpages *is* an integer, though it's best interpreted as an
unsigned one. (Ditto for relid.) Looks like this code is 0-for-3 on
getting the datatypes right :-(regards, tom lane
---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Yeah, I'll take a look at it and submit a patch. Sorry I didn't see it
sooner, but I don't read the bugs mailing list.
Show quoted text
On Wed, 2004-02-11 at 17:29, Bruce Momjian wrote:
Would someone review these problems and submit a patch? Thanks.
---------------------------------------------------------------------------
Tom Lane wrote:
Cott Lang <cott@internetstaff.com> writes:
If the number of tuples is sufficiently high, pg reports 'reltuples'
back in TABLE_STATS_QUERY in scientific notation instead of an integer.Right, because that column is actually a float4.
Changing from atoi() to atof() solves the problem completely.
new_tbl->reltuples =
atof(PQgetvalue(res, row, PQfnumber(res, "reltuples")));new_tbl->relpages =
atof(PQgetvalue(res, row, PQfnumber(res, "relpages")));I should think this would break in different ways once reltuples exceeds
INT_MAX. A full fix would require changing new_tbl->reltuples to be
float or double, and coping with any downstream changes that implies.Also, relpages *is* an integer, though it's best interpreted as an
unsigned one. (Ditto for relid.) Looks like this code is 0-for-3 on
getting the datatypes right :-(regards, tom lane
---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster
I have my original changes + Tom's recommended changes applied to 7.4.1
if you're interested.
Show quoted text
On Wed, 2004-02-11 at 15:57, Matthew T. O'Connor wrote:
Yeah, I'll take a look at it and submit a patch. Sorry I didn't see it
sooner, but I don't read the bugs mailing list.On Wed, 2004-02-11 at 17:29, Bruce Momjian wrote:
Would someone review these problems and submit a patch? Thanks.
---------------------------------------------------------------------------
Tom Lane wrote:
Cott Lang <cott@internetstaff.com> writes:
If the number of tuples is sufficiently high, pg reports 'reltuples'
back in TABLE_STATS_QUERY in scientific notation instead of an integer.Right, because that column is actually a float4.
Changing from atoi() to atof() solves the problem completely.
new_tbl->reltuples =
atof(PQgetvalue(res, row, PQfnumber(res, "reltuples")));new_tbl->relpages =
atof(PQgetvalue(res, row, PQfnumber(res, "relpages")));I should think this would break in different ways once reltuples exceeds
INT_MAX. A full fix would require changing new_tbl->reltuples to be
float or double, and coping with any downstream changes that implies.Also, relpages *is* an integer, though it's best interpreted as an
unsigned one. (Ditto for relid.) Looks like this code is 0-for-3 on
getting the datatypes right :-(regards, tom lane
---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster
Cott Lang wrote:
I have my original changes + Tom's recommended changes applied to 7.4.1
if you're interested.
Sure, shoot them over to hackers or patches. The pg_autovacuum author
is looking into this.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073On Wed, 2004-02-11 at 16:25, Bruce Momjian wrote:
Sure, shoot them over to hackers or patches. The pg_autovacuum author
is looking into this.
Here they are. They've worked well for me, but someone wiser in the ways
of C should certainly look them over. :)
Attachments:
autovac.patchtext/x-patch; charset=; name=autovac.patchDownload
? autovac.patch
? pg_autovacuum
Index: pg_autovacuum.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/contrib/pg_autovacuum/pg_autovacuum.c,v
retrieving revision 1.13
diff -u -r1.13 pg_autovacuum.c
--- pg_autovacuum.c 8 Dec 2003 21:54:00 -0000 1.13
+++ pg_autovacuum.c 12 Feb 2004 00:25:57 -0000
@@ -118,7 +118,7 @@
new_tbl->curr_vacuum_count = new_tbl->CountAtLastVacuum;
new_tbl->relid = atoi(PQgetvalue(res, row, PQfnumber(res, "oid")));
- new_tbl->reltuples = atoi(PQgetvalue(res, row, PQfnumber(res, "reltuples")));
+ new_tbl->reltuples = atof(PQgetvalue(res, row, PQfnumber(res, "reltuples")));
new_tbl->relpages = atoi(PQgetvalue(res, row, PQfnumber(res, "relpages")));
if (strcmp("t", PQgetvalue(res, row, PQfnumber(res, "relisshared"))))
@@ -159,7 +159,7 @@
if (res != NULL)
{
tbl->reltuples =
- atoi(PQgetvalue(res, 0, PQfnumber(res, "reltuples")));
+ atof(PQgetvalue(res, 0, PQfnumber(res, "reltuples")));
tbl->relpages = atoi(PQgetvalue(res, 0, PQfnumber(res, "relpages")));
/*
@@ -363,7 +363,7 @@
log_entry(logbuffer);
sprintf(logbuffer, " relid: %i; relisshared: %i", tbl->relid, tbl->relisshared);
log_entry(logbuffer);
- sprintf(logbuffer, " reltuples: %i; relpages: %i", tbl->reltuples, tbl->relpages);
+ sprintf(logbuffer, " reltuples: %.0f; relpages: %i", tbl->reltuples, tbl->relpages);
log_entry(logbuffer);
sprintf(logbuffer, " curr_analyze_count: %li; cur_delete_count: %li",
tbl->curr_analyze_count, tbl->curr_vacuum_count);
Index: pg_autovacuum.h
===================================================================
RCS file: /projects/cvsroot/pgsql-server/contrib/pg_autovacuum/pg_autovacuum.h,v
retrieving revision 1.8
diff -u -r1.8 pg_autovacuum.h
--- pg_autovacuum.h 1 Dec 2003 23:19:33 -0000 1.8
+++ pg_autovacuum.h 12 Feb 2004 00:25:57 -0000
@@ -84,10 +84,12 @@
{
char *schema_name,
*table_name;
- int relid,
- reltuples,
- relisshared,
- relpages;
+ unsigned int relid,
+ relpages;
+
+ int relisshared;
+ double reltuples;
+
long analyze_threshold,
vacuum_threshold;
long CountAtLastAnalyze; /* equal to: inserts + updates as
Not sure if anyone reported to you but we fixed these in current CVS and
will have the fix in 7.4.3.
---------------------------------------------------------------------------
Cott Lang wrote:
On Wed, 2004-02-11 at 16:25, Bruce Momjian wrote:
Sure, shoot them over to hackers or patches. The pg_autovacuum author
is looking into this.Here they are. They've worked well for me, but someone wiser in the ways
of C should certainly look them over. :)
[ Attachment, skipping... ]
---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073