Another small bug (pg_autovacuum)

Ouch... sorry, my fault. I'll fix this tomorrow (Friday) and submit a
patch, or if you want to submit a patch that would be fine. All you
have to do is change the the sql statements to put quotes around the
relation name.

Thanks for catching this.

Matthew T. O'Connor

On Thu, 2003-09-04 at 18:39, Adam Kavan wrote:

Now that I have pg_autovacuum working I've bumped into another small
bug. When pg_autovacuum goes to vacuum or analyze one of my tables it runs...

Also, has this been officially fixed? All I have heard so far is that
you commented out the check and now now it works for you.

I assume the attached patch is what you want done to fix this. Applied.

It quotes table names for vacuum and analyze, and uppercases the
keywords for clarity.

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

Matthew T. O'Connor wrote:

Ouch... sorry, my fault. I'll fix this tomorrow (Friday) and submit a
patch, or if you want to submit a patch that would be fine. All you
have to do is change the the sql statements to put quotes around the
relation name.

Thanks for catching this.

Matthew T. O'Connor

On Thu, 2003-09-04 at 18:39, Adam Kavan wrote:

Now that I have pg_autovacuum working I've bumped into another small
bug. When pg_autovacuum goes to vacuum or analyze one of my tables it runs...

analyze public.ConfigBackup

Because ConfigBackup is mixed case it cannot find the relation. I fixed
this by going to the function init_table_info and increasing the malloc for
new_tbl->table_name by 2 and adding "'s to either side of the table
name. Is there anything wrong with this approach? Is there a config I can
set to make this non-case sensitive?

Thanks again for your time.

--- Adam Kavan
--- akavan@cox.net

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faqs/FAQ.html

---------------------------(end of broadcast)---------------------------
TIP 9: the planner will ignore your desire to choose an index scan if your
joining column's datatypes do not match

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

On Wed, 2003-09-10 at 15:57, Bruce Momjian wrote:

I assume the attached patch is what you want done to fix this. Applied.

It quotes table names for vacuum and analyze, and uppercases the
keywords for clarity.

Yeah, this is basically what I meant, sorry I didn't get to it quicker.

However, I tested it out a little and the patch you made doesn't work
because it produces commands like:

VACUUM ANALYZE "public.FooBar"

Which doesn't work, so I made my own patch that creates commands like:

VACUUM ANALYZE "public"."FooBar"

This allows for mixed case schema names as well as tables.

Adam, can you please give this a test as you are the person who caught
the bug in the first place.

Thanks,

Matthew T. O'Connor

matthew@zeut.net ("Matthew T. O'Connor") wrote:

On Wed, 2003-09-10 at 15:57, Bruce Momjian wrote:

I assume the attached patch is what you want done to fix this. Applied.

It quotes table names for vacuum and analyze, and uppercases the
keywords for clarity.

Yeah, this is basically what I meant, sorry I didn't get to it quicker.

However, I tested it out a little and the patch you made doesn't work
because it produces commands like:

VACUUM ANALYZE "public.FooBar"

Which doesn't work, so I made my own patch that creates commands like:

VACUUM ANALYZE "public"."FooBar"

This allows for mixed case schema names as well as tables.

Adam, can you please give this a test as you are the person who caught
the bug in the first place.

Something I am feeling a little suspicious of is that I haven't seen,
in the logs, pg_autovacuum looking at pg_ tables.

I know that if we don't periodically vacuum such system tables as
pg_class, pg_attribute, pg_statistic, and pg_type, they can get to
"pretty evil size."

[Rummaging around...] These tables are being added for template1, but
apparently not for "main" databases. That looks like a bit of a fly
in the ointment...
--
(format nil "~S@~S" "cbbrowne" "ntlug.org")
http://www.ntlug.org/~cbbrowne/rdbms.html
Signs of a Klingon Programmer - 16. "Klingon programs don't do
accountancy. For that, you need a Ferengi."

At 12:03 AM 9/11/03 -0400, Matthew T. O'Connor wrote:

Adam, can you please give this a test as you are the person who caught
the bug in the first place.

Thanks,

Matthew T. O'Connor

I applied your patch and it works fine for me.

--- Adam Kavan 

Patch applied. You might want to look at pg_dump/dumputils.c::fmtId()
for a function that does smart quoting.

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

Matthew T. O'Connor wrote:

On Wed, 2003-09-10 at 15:57, Bruce Momjian wrote:

I assume the attached patch is what you want done to fix this. Applied.

It quotes table names for vacuum and analyze, and uppercases the
keywords for clarity.

Yeah, this is basically what I meant, sorry I didn't get to it quicker.

However, I tested it out a little and the patch you made doesn't work
because it produces commands like:

VACUUM ANALYZE "public.FooBar"

Which doesn't work, so I made my own patch that creates commands like:

VACUUM ANALYZE "public"."FooBar"

This allows for mixed case schema names as well as tables.

Adam, can you please give this a test as you are the person who caught
the bug in the first place.

Thanks,

Matthew T. O'Connor

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

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

On Thu, 2003-09-11 at 15:02, Bruce Momjian wrote:

Patch applied. You might want to look at pg_dump/dumputils.c::fmtId()
for a function that does smart quoting.

OK, thanks.

On Thu, 2003-09-11 at 08:12, Christopher Browne wrote:

Something I am feeling a little suspicious of is that I haven't seen,
in the logs, pg_autovacuum looking at pg_ tables.

I know that if we don't periodically vacuum such system tables as
pg_class, pg_attribute, pg_statistic, and pg_type, they can get to
"pretty evil size."

[Rummaging around...] These tables are being added for template1, but
apparently not for "main" databases. That looks like a bit of a fly
in the ointment...

I designed it that way. It was my understanding that all of the system
tables pg_class etc... are shared tables, available in all databases,
but actually stored as only one central set of real tables. Hence
vacuuming pg_class from template1 helps every database that accesses
pg_class.

Did I make a design error?

Matthew T. O'Connor wrote:

On Thu, 2003-09-11 at 08:12, Christopher Browne wrote:

Something I am feeling a little suspicious of is that I haven't seen,
in the logs, pg_autovacuum looking at pg_ tables.

I know that if we don't periodically vacuum such system tables as
pg_class, pg_attribute, pg_statistic, and pg_type, they can get to
"pretty evil size."

[Rummaging around...] These tables are being added for template1, but
apparently not for "main" databases. That looks like a bit of a fly
in the ointment...

I designed it that way. It was my understanding that all of the system
tables pg_class etc... are shared tables, available in all databases,
but actually stored as only one central set of real tables. Hence
vacuuming pg_class from template1 helps every database that accesses
pg_class.

Did I make a design error?

Oops, no. Only a few pg_* tables are "global". pg_class isn't. In
fact, I am not sure how someone tells which are global. A grep in
/src/include/catalog shows:

$ grep BKI_SHARED_RELATION *.h
pg_database.h:CATALOG(pg_database) BOOTSTRAP BKI_SHARED_RELATION
pg_group.h:CATALOG(pg_group) BOOTSTRAP BKI_SHARED_RELATION BKI_WITHOUT_OIDS
pg_shadow.h:CATALOG(pg_shadow) BOOTSTRAP BKI_SHARED_RELATION BKI_WITHOUT_OIDS

so those are the only shared ones. I found a query to do it too:

test=> select * from pg_class where relisshared = 't' and relkind = 'r';
relname | relnamespace | reltype | relowner | relam | relfilenode | relpages | reltuples | reltoastrelid | reltoastidxid | relhasindex | relisshared | relkind | relnatts | relchecks | reltriggers | relukeys | relfkeys | relrefs | relhasoids | relhaspkey | relhasrules | relhassubclass | relacl
-------------+--------------+---------+----------+-------+-------------+----------+-----------+---------------+---------------+-------------+-------------+---------+----------+-----------+-------------+----------+----------+---------+------------+------------+-------------+----------------+------------------------------------
pg_shadow | 11 | 86 | 1 | 0 | 1260 | 1 | 1 | 16677 | 0 | t | t | r | 8 | 0 | 1 | 0 | 0 | 0 | f | f | f | f | {postgres=a*r*w*d*R*x*t*/postgres}
pg_database | 11 | 88 | 1 | 0 | 1262 | 1 | 2 | 16662 | 0 | t | t | r | 11 | 0 | 0 | 0 | 0 | 0 | t | f | f | f | {=r/postgres}
pg_group | 11 | 87 | 1 | 0 | 1261 | 0 | 0 | 16668 | 0 | t | t | r | 3 | 0 | 1 | 0 | 0 | 0 | f | f | f | f | {=r/postgres}
(3 rows)

so those are the only ones that should be template1-only. All other
pg_* tables should be vacuumed in individual database.

I will wait for a patch. :-)

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

matthew@zeut.net ("Matthew T. O'Connor") writes:

On Thu, 2003-09-11 at 08:12, Christopher Browne wrote:

Something I am feeling a little suspicious of is that I haven't seen,
in the logs, pg_autovacuum looking at pg_ tables.

I know that if we don't periodically vacuum such system tables as
pg_class, pg_attribute, pg_statistic, and pg_type, they can get to
"pretty evil size."

[Rummaging around...] These tables are being added for template1, but
apparently not for "main" databases. That looks like a bit of a fly
in the ointment...

I designed it that way. It was my understanding that all of the system
tables pg_class etc... are shared tables, available in all databases,
but actually stored as only one central set of real tables. Hence
vacuuming pg_class from template1 helps every database that accesses
pg_class.

Did I make a design error?

[rummaging around... Where's a suitable system? There it is...]

database3=# vacuum verbose pg_class;
NOTICE: --Relation pg_class--
NOTICE: Index pg_class_oid_index: Pages 11399; Tuples 165: Deleted 1408.
CPU 2.00s/0.41u sec elapsed 2.51 sec.
NOTICE: Index pg_class_relname_index: Pages 30604; Tuples 171: Deleted 1408.
CPU 5.81s/1.07u sec elapsed 7.07 sec.
NOTICE: Removed 1408 tuples in 24 pages.
CPU 0.01s/0.02u sec elapsed 0.20 sec.
NOTICE: Pages 360: Changed 1, Empty 0; Tup 165: Vac 1408, Keep 0, UnUsed 21110.
Total CPU 7.82s/1.50u sec elapsed 9.79 sec.
VACUUM
database3=# \c database1
You are now connected to database database1
database1=# vacuum verbose pg_class;
NOTICE: --Relation pg_class--
NOTICE: Pages 22768: Changed 0, Empty 0; Tup 158: Vac 0, Keep 0, UnUsed 1434149.
Total CPU 1.35s/0.97u sec elapsed 2.38 sec.
VACUUM
database1=# \c template1
You are now connected to database template1.
template1=# vacuum verbose pg_class;
NOTICE: --Relation pg_class--
NOTICE: Pages 2: Changed 0, Empty 0; Tup 101: Vac 0, Keep 0, UnUsed 27.
Total CPU 0.00s/0.00u sec elapsed 0.00 sec.
VACUUM

No, they aren't shared. (And why yes, some of those databases could
indeed use more regular vacuuming. :-))

Note, that was a 7.2.4 database, so pg_autovacuum won't work there
yet. C'est la vie.

At any rate, template1.pg_class seems pretty distinct from
database1.pg_class which is distinct from database3.pg_class. I think
there's a bit of an error there.
--
(reverse (concatenate 'string "ofni.smrytrebil" "@" "enworbbc"))
<http://dev6.int.libertyrms.com/&gt;
Christopher Browne
(416) 646 3304 x124 (land)

"Matthew T. O'Connor" <matthew@zeut.net> writes:

On Thu, 2003-09-11 at 08:12, Christopher Browne wrote:

[Rummaging around...] These tables are being added for template1, but
apparently not for "main" databases. That looks like a bit of a fly
in the ointment...

I designed it that way. It was my understanding that all of the system
tables pg_class etc... are shared tables, available in all databases,
but actually stored as only one central set of real tables.

You are very badly mistaken.

Only the tables marked "relisshared" in pg_class (currently pg_shadow,
pg_group, pg_database, and their indexes and toast tables) are shared
across a cluster. The rest have separate copies per-database.

regards, tom lane

On Thu, 2003-09-11 at 17:11, Tom Lane wrote:

"Matthew T. O'Connor" <matthew@zeut.net> writes:

I designed it that way. It was my understanding that all of the system
tables pg_class etc... are shared tables, available in all databases,
but actually stored as only one central set of real tables.

You are very badly mistaken.

Only the tables marked "relisshared" in pg_class (currently pg_shadow,
pg_group, pg_database, and their indexes and toast tables) are shared
across a cluster. The rest have separate copies per-database.

hrm.... OK. Patch forthcoming....

"Matthew T. O'Connor" <matthew@zeut.net> writes:

On Thu, 2003-09-11 at 17:11, Tom Lane wrote:

"Matthew T. O'Connor" <matthew@zeut.net> writes:

I designed it that way. It was my understanding that all of the system
tables pg_class etc... are shared tables, available in all databases,
but actually stored as only one central set of real tables.

Only the tables marked "relisshared" in pg_class (currently pg_shadow,
pg_group, pg_database, and their indexes and toast tables) are shared
across a cluster. The rest have separate copies per-database.

hrm.... OK. Patch forthcoming....

BTW, I am not sure it is a good idea to suppress "redundant" vacuuming
of shared tables in the first place. The trouble with doing so is that
if you only vacuum pg_shadow through template1, then only template1 will
ever have up-to-date statistics about it. That's not good.

You might be able to get away with doing actual vacuums only through
template1, and doing just ANALYZEs every so often in other DBs.

regards, tom lane

On Thu, 2003-09-11 at 18:25, Tom Lane wrote:

BTW, I am not sure it is a good idea to suppress "redundant" vacuuming
of shared tables in the first place. The trouble with doing so is that
if you only vacuum pg_shadow through template1, then only template1 will
ever have up-to-date statistics about it. That's not good.

You might be able to get away with doing actual vacuums only through
template1, and doing just ANALYZEs every so often in other DBs.

ok I will see what I can do about that. So I assume that the vacuumdb
script handle this just does redundant vacuums / analyzes on shared
tables so that it doesn't have a problem with this.

If we can supress "redundant" vacuuming I think that would be a good
thing as pg_autovacuum is supposed to make the required vacuuming as
efficient as possible.

On Thu, 2003-09-11 at 18:25, Tom Lane wrote:

"Matthew T. O'Connor" <matthew@zeut.net> writes:

hrm.... OK. Patch forthcoming....

BTW, I am not sure it is a good idea to suppress "redundant" vacuuming
of shared tables in the first place. The trouble with doing so is that
if you only vacuum pg_shadow through template1, then only template1 will
ever have up-to-date statistics about it. That's not good.

You might be able to get away with doing actual vacuums only through
template1, and doing just ANALYZEs every so often in other DBs.

I made a patch to fix this, but in testing it I noticed that the stats
system doesn't work on shared tables as I was expecting it too (as my
latest patch requires it too :-). It treats instances of shared tables
in separate databases as totally unique tables. This makes it hard to
know how much activity has really gone on for a shared table.

Is the behavior of the following example expected / desired?

template1=# select ... (query details snipped)
relname | relisshared | n_tup_ins | n_tup_upd | n_tup_del
-------------+-------------+-----------+-----------+-----------
pg_database | t | 28 | 0 | 28
(1 row)

template1=# create database foo; drop database foo;
CREATE DATABASE
DROP DATABASE
template1=# select
relname | relisshared | n_tup_ins | n_tup_upd | n_tup_del
-------------+-------------+-----------+-----------+-----------
pg_database | t | 29 | 0 | 29
(1 row)

template1=# \c matthew
You are now connected to database "matthew".
matthew=# select
relname | relisshared | n_tup_ins | n_tup_upd | n_tup_del
-------------+-------------+-----------+-----------+-----------
pg_database | t | 2 | 0 | 2
(1 row)

matthew=# create database foo; drop database foo;
CREATE DATABASE
DROP DATABASE
matthew=# select
relname | relisshared | n_tup_ins | n_tup_upd | n_tup_del
-------------+-------------+-----------+-----------+-----------
pg_database | t | 3 | 0 | 3
(1 row)

matthew=# \c template1
You are now connected to database "template1".
template1=# select
relname | relisshared | n_tup_ins | n_tup_upd | n_tup_del
-------------+-------------+-----------+-----------+-----------
pg_database | t | 29 | 0 | 29
(1 row)

Matthew T. O'Connor wrote:

On Thu, 2003-09-11 at 18:25, Tom Lane wrote:

"Matthew T. O'Connor" <matthew@zeut.net> writes:

hrm.... OK. Patch forthcoming....

BTW, I am not sure it is a good idea to suppress "redundant" vacuuming
of shared tables in the first place. The trouble with doing so is that
if you only vacuum pg_shadow through template1, then only template1 will
ever have up-to-date statistics about it. That's not good.

You might be able to get away with doing actual vacuums only through
template1, and doing just ANALYZEs every so often in other DBs.

I made a patch to fix this, but in testing it I noticed that the stats
system doesn't work on shared tables as I was expecting it too (as my
latest patch requires it too :-). It treats instances of shared tables
in separate databases as totally unique tables. This makes it hard to
know how much activity has really gone on for a shared table.

Is the behavior of the following example expected / desired?

I suspect is just a bug because no one noticed it before. As I
understand it, the stats system is recorded per-database. We could add
stuff so the global tables are only recorded in template1 or perhaps
record in template1 but report template1's numbers for all databases.

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

"Matthew T. O'Connor" <matthew@zeut.net> writes:

I made a patch to fix this, but in testing it I noticed that the stats
system doesn't work on shared tables as I was expecting it too (as my
latest patch requires it too :-). It treats instances of shared tables
in separate databases as totally unique tables.

Hmm. The bufmgr and lower levels handle shared tables by always
associating a database number of zero with them, but I'll bet that the
pg_stats stuff does not do that. I'd call that a bug, yes. Jan, any
thoughts on how complex to fix?

regards, tom lane

Tom Lane wrote:

"Matthew T. O'Connor" <matthew@zeut.net> writes:

I made a patch to fix this, but in testing it I noticed that the stats
system doesn't work on shared tables as I was expecting it too (as my
latest patch requires it too :-). It treats instances of shared tables
in separate databases as totally unique tables.

Hmm. The bufmgr and lower levels handle shared tables by always
associating a database number of zero with them, but I'll bet that the
pg_stats stuff does not do that. I'd call that a bug, yes. Jan, any
thoughts on how complex to fix?

Need to take a look into, but I agree that this is a bug.

Jan

--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck@Yahoo.com #