Re: pg_autovacuum integration patch - Mailing list pgsql-patches

Some nitpicking on details:


The comment above AutoVacMain() claims:
!  * Main entry point for bgwriter process


I also see a bunch of // comments, I think those are not appreciated.


Haven't had time to look much at the actual functionality. Just did a
quick look-through for win32 showstoppers, to be honest - didn't find
any. (Doesn't mean they're not there, but..)


Also, isn't it a bit unnecessary to do:
sprintf(logbuffer,"foo bar %s",whatever);
elog(ERROR,logbuffer);

why not just
elog(ERROR,"foo bar %s",whatever);
[assuming I read the patch right - I still need some practice reading
patches...]

I also noticed:
!         elog(ERROR, "pg_autovacuum: GUC variable stats_row_level
must be enabled.");
!         elog(ERROR, "       Please fix the problems and try
again.");

If you use the ereport() call instead of elog, you can set the second
one as a HINT. Since it's really the same error, I don't think you want
multiple errors logged...


I'll leave it to others to comment on the actual code - I'll take the
easy way out and do nitpicking instead :-) I'll try to test this on
win32 as soon as I have my tree back in compiling order.

//Magnus


> -----Original Message-----
> From: Matthew T. O'Connor [mailto:matthew@zeut.net]
> Sent: Wednesday, June 16, 2004 8:19 AM
> To: PostgreSQL Patches
> Subject: [PATCHES] pg_autovacuum integration patch
>
> Ok, I have an 1st cut patch for pg_autovacuum integration
> into the backend.  Please apply this patch to CVS or at least
> review and let me know what I need to change to get it
> applied to CVS.
>
> This patch requires that pg_autovacuum.c and .h are moved
> from contrib to src/backend/postmaster and
> src/include/postmaster respectively.  I have also attached
> the pg_autovacuum.h file that needs to be put in
> src/include/catelog/ for the new pg_autovacuum system table.
>
> There is more to do for pg_autovacuum but I would like to get
> this patch included into CVS or at least get it rejected now
> so I can fix it before July 1.
>
> With this patch pg_autovacuum:
> * is a postmaster subprocess modeled after the bgwriter
> * uses elog for all output (I guessed at the appropriate elog levels)
> * used a new GUC variable to enable and disable pg_autovacuum
> * stores all it's volitile data in a new pg_autovacuum system table
> * allows admin to set per table thresholds
>
> Matthew O'Connor
>
>
>
>