Обсуждение: autovacuum, reloptions, and hardcoded pg_class tupdesc

Hi,

So I've been progressing on revising the autovacuum patch to make it
work with the current reloptions.  We have a number of options:

1. Call heap_open() for every relation that we're going to check, and  examine the reloptions via the relcache.  I'm
notsure that I like this very much.  Right now we just plow  ahead using a pg_class seqscan, which avoids locking the
relations just for the sake of verifying whether they need work.  However, this  is the cleanest option in terms of
modularitybreakage. 
 
2. Play some dirty tricks in autovacuum and extract the reloptions from  the bare pg_class tuple ourselves.  I think
doingthis cleanly  requires exporting some internal functions like  GetPgClassDescriptor() from relcache.c.
 

3. Disallow setting reloptions for pg_class, which (I think) allows us  to avoid having to use GetPgClassDescriptor,
butthe rest of it is  still a dirty hack.
 

In particular, if we do either (2) or (3), we'll need to keep an eye on
that code whenever we modify RelationParseRelOptions.

Thoughts?

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Alvaro Herrera <alvherre@commandprompt.com> writes:
> So I've been progressing on revising the autovacuum patch to make it
> work with the current reloptions.  We have a number of options:

> 1. Call heap_open() for every relation that we're going to check, and
>    examine the reloptions via the relcache.
>    I'm not sure that I like this very much.

I don't like it at all, mainly because it implies taking locks on tables
that autovacuum doesn't need to be touching.  Even if it's only
AccessShareLock, it could result in problems vis-a-vis clients that are
holding exclusive table locks.

>    Right now we just plow
>    ahead using a pg_class seqscan, which avoids locking the relations
>    just for the sake of verifying whether they need work.

We should stick with that, and refactor the reloptions code as needed to
be able to work from just a pg_class tuple.  I'm envisioning a scheme
like:
bottom level: extract from pg_class tuple, return a palloc'd struct
relcache: logic to cache the result of the above
top level: exported function to return a cached options struct

The autovac scan could use the bottom-level API.
        regards, tom lane

Tom Lane wrote:
> Alvaro Herrera <alvherre@commandprompt.com> writes:

> >    Right now we just plow
> >    ahead using a pg_class seqscan, which avoids locking the relations
> >    just for the sake of verifying whether they need work.
>
> We should stick with that, and refactor the reloptions code as needed to
> be able to work from just a pg_class tuple.  I'm envisioning a scheme
> like:
>
>     bottom level: extract from pg_class tuple, return a palloc'd struct
>
>     relcache: logic to cache the result of the above
>
>     top level: exported function to return a cached options struct
>
> The autovac scan could use the bottom-level API.

I'm not sure that we have any use for the top level you propose; the
attached patch just uses the two lower levels, and I think it fits
autovacuum usage just fine.  Thanks for the idea.

--
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera wrote:

> I'm not sure that we have any use for the top level you propose; the
> attached patch just uses the two lower levels, and I think it fits
> autovacuum usage just fine.  Thanks for the idea.

Of course, there's no need to pass the relkind; it goes inside the
pg_class tuple.

--
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Alvaro Herrera wrote:
> Alvaro Herrera wrote:
> 
> > I'm not sure that we have any use for the top level you propose; the
> > attached patch just uses the two lower levels, and I think it fits
> > autovacuum usage just fine.  Thanks for the idea.
> 
> Of course, there's no need to pass the relkind; it goes inside the
> pg_class tuple.

Applied.


-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.