Re: [PATCH] Unremovable tuple monitoring - Mailing list pgsql-hackers

From Greg Smith
Subject Re: [PATCH] Unremovable tuple monitoring
Date
Msg-id 4E8BF8BF.2010803@2ndQuadrant.com
Whole thread Raw
In response to Re: [PATCH] Unremovable tuple monitoring (was: Addition of some trivial auto vacuum logging)  (Royce Ausburn <royce.ml@inomial.com>)
List pgsql-hackers
On 10/04/2011 03:45 PM, Royce Ausburn wrote:
> I think I get this stats stuff now. Unless someone here thinks it's 
> too hard for a new postgres dev's 2nd patch, I could take a stab. I 
> might take a look at it tonight to get a feel for how hard, and what 
> stats we could collect. I'll start a new thread for discussion.

Adding statistics and good monitoring points isn't hard to do, once you 
figure out how the statistics messaging works.  From looking at your 
patch, you seem to be over that part of the learning curve already.  The 
most time consuming part for vacuum logging patches is setting up the 
test cases and waiting for them to execute.  If you can provide a script 
that does that, it will aid in getting review off to a goo start.  
Basically, whatever you build to test the patch, you should think about 
packaging into a script you can hand to a reviewer/committer.  Normal 
approach is to build a test data set with something like 
generate_series, then create the situation the patch is supposed to log.

Just to clarify what Robert was suggesting a little further, good 
practice here is just to say "this patch needs a catversion bump", but 
not actually do it.  Committers should figure that out even if you don't 
mention it, but sometimes a goof is made; a little reminder doesn't hurt.

> I'm not sure what my next step should be.  I've added this patch to the open commit fest -- is that all for now until
thecommit fest begins review?
 
>    

Basically, we'll get to it next month.  I have my own autovacuum logging 
stuff I'm working on that I expect to slip to that one too, so I can 
easily take on reviewing yours then.  I just fixed the entry in the CF 
app to follow convention by listing your full name.

Main feedback for now on the patch is that few people ever use 
pg_stat_all_tables.  The new counter needs to go into 
pg_stat_user_tables and pg_stat_sys_tables if it's going to be visible 
to the people who are most likely to need it.  I updated the name of the 
patch on the CommitFest to read "Unremovable tuple count on 
pg_stat_*_tables" so the spec here is more clear.  I'd suggest chewing 
on the rest of your ideas, see what else falls out of this, and just 
make sure to submit another update just before the next CF starts.

-- 
Greg Smith   2ndQuadrant US    greg@2ndQuadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us



pgsql-hackers by date:

Previous
From: Amit Khandekar
Date:
Subject: Re: Re: [COMMITTERS] pgsql: Force strings passed to and from plperl to be in UTF8 encoding.
Next
From: Heikki Linnakangas
Date:
Subject: Re: Action requested - Application Softblock implemented | Issue report ID341057