[HACKERS] [PATCH] Addition of some trivial auto vacuum logging

2011-09-27 Thread Royce Ausburn
Hi all,

I spent a bit of today diagnosing a problem where long held transactions were 
preventing auto vacuum from doing its job.  Eventually I had set 
log_autovacuum_min_duration to 0 to see what was going on.  Unfortunately it 
wasn't until I tried a VACUUM VERBOSE that I found there were dead tuples not 
being removed.  Had the unremoveable tuple count been included in the 
autovacuum log message life would have been a tiny bit easier.

I've been meaning for a while to dabble in postgres, so I thought this might be 
a good trivial thing for me to improve.  The attached patch adds extra detail 
the the existing autovacuum log message that is emitted when the 
log_autovacuum_min_duration threshold is met, exposing the unremovable dead 
tuple count similar to what you get from VACUUM VERBOSE.

Sample log output (my addition in bold):

LOG:  automatic vacuum of table test.public.test: index scans: 0
pages: 0 removed, 5 remain
tuples: 0 removed, 1000 remain, 999 dead but not removable
system usage: CPU 0.00s/0.00u sec elapsed 0.00 sec


My patch adds another member to the LVRelStats struct named 
undeleteable_dead_tuples.  lazy_scan_heap() sets undeleteable_dead_tuples to 
the value of lazy_scan_heap()'s local variable nkeep, which is the same 
variable that is used to emit the VACUUM VERBOSE unremoveable dead row count.

As this is my first patch to postgresql, I'm expecting I've done something 
wrong.  Please if you want me to fix something up, or just go away please say 
so ;)  I appreciate that this is a trivial patch, and perhaps doesn't add value 
except for my very specific use case… feel free to ignore it =)

--Royce





diff --git a/src/backend/commands/vacuumlazy.c 
b/src/backend/commands/vacuumlazy.c
index cf8337b..12f03d7 100644
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -91,6 +91,7 @@ typedef struct LVRelStats
double  scanned_tuples; /* counts only tuples on scanned pages 
*/
double  old_rel_tuples; /* previous value of pg_class.reltuples 
*/
double  new_rel_tuples; /* new estimated total # of tuples */
+   double  undeleteable_dead_tuples; /* count of dead tuples not 
yet removeable */
BlockNumber pages_removed;
double  tuples_deleted;
BlockNumber nonempty_pages; /* actually, last nonempty page + 1 */
@@ -256,7 +257,7 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
ereport(LOG,
(errmsg(automatic vacuum of table 
\%s.%s.%s\: index scans: %d\n
pages: %d removed, %d 
remain\n
-   tuples: %.0f removed, 
%.0f remain\n
+   tuples: %.0f removed, 
%.0f remain, %.0f dead but not removable\n
system usage: %s,

get_database_name(MyDatabaseId),

get_namespace_name(RelationGetNamespace(onerel)),
@@ -266,6 +267,7 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
vacrelstats-rel_pages,

vacrelstats-tuples_deleted,
new_rel_tuples,
+   
vacrelstats-undeleteable_dead_tuples,
pg_rusage_show(ru0;
}
 }
@@ -829,6 +831,7 @@ lazy_scan_heap(Relation onerel, LVRelStats *vacrelstats,
/* save stats for use later */
vacrelstats-scanned_tuples = num_tuples;
vacrelstats-tuples_deleted = tups_vacuumed;
+   vacrelstats-undeleteable_dead_tuples = nkeep;
 
/* now we can compute the new value for pg_class.reltuples */
vacrelstats-new_rel_tuples = vac_estimate_reltuples(onerel, false,



Re: [HACKERS] [PATCH] Addition of some trivial auto vacuum logging

2011-09-27 Thread Tom Lane
Royce Ausburn royce...@inomial.com writes:
  The attached patch adds extra detail the the existing autovacuum log message 
 that is emitted when the log_autovacuum_min_duration threshold is met, 
 exposing the unremovable dead tuple count similar to what you get from VACUUM 
 VERBOSE.

 Sample log output (my addition in bold):

 LOG:  automatic vacuum of table test.public.test: index scans: 0
   pages: 0 removed, 5 remain
   tuples: 0 removed, 1000 remain, 999 dead but not removable
   system usage: CPU 0.00s/0.00u sec elapsed 0.00 sec

This proposal seems rather ill-designed.  In the first place, these
numbers are quite unrelated to vacuum duration, and in the second place,
most people who might need the info don't have that setting turned on
anyway.

I wonder whether it wouldn't be more helpful to have a pg_stat_all_tables
column that reports the number of unremovable tuples as of the last
vacuum.  I've been known to object to more per-table stats counters
in the past on the basis of space required, but perhaps this one would
be worth its keep.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [PATCH] Addition of some trivial auto vacuum logging

2011-09-27 Thread Kevin Grittner
Royce Ausburn royce...@inomial.com wrote:
 
 As this is my first patch to postgresql, I'm expecting I've done
 something wrong.  Please if you want me to fix something up, or
 just go away please say so ;)  I appreciate that this is a trivial
 patch, and perhaps doesn't add value except for my very specific
 use case* feel free to ignore it =)
 
Thanks for offering this to the community.  I see you've already
gotten feedback on the patch, with a suggestion for a different
approach.  Don't let that discourage you -- very few patches get in
without needing to be modified based on review and feedback.
 
If you haven't already done so, please review this page and its
links:
 
http://www.postgresql.org/developer/
 
Of particular interest is the Developer FAQ:
 
http://wiki.postgresql.org/wiki/Developer_FAQ
 
You should also be aware of the development cycle, which (when not
in feature freeze for beta testing) involves alternating periods of
focused development and code review (the latter called CommitFests):
 
http://wiki.postgresql.org/wiki/CommitFest
 
We are now in the midst of a CF, so it would be great if you could
join in that as a reviewer.  Newly submitted patches should be
submitted to the open CF:
 
http://commitfest.postgresql.org/action/commitfest_view/open
 
You might want to consider what Tom said and submit a modified patch
for the next review cycle.
 
Welcome!
 
-Kevin

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [PATCH] Addition of some trivial auto vacuum logging

2011-09-27 Thread Royce Ausburn
Thanks for the tips guys. 

Just a question:  is the utility great enough to warrant me working further on 
this?  I have no real desire to implement this particular feature -- I simply 
saw an opportunity to cut my teeth on something easy.  I'd be happy to find 
something on the TODO list instead if this feature isn't really worthwhile.

Tom's suggestion looks like it's less trivial that I can do just yet, but I'll 
take a look and ask for help if I need it.

Cheers!

--Royce


On 28/09/2011, at 4:42 AM, Kevin Grittner wrote:

 Royce Ausburn royce...@inomial.com wrote:
 
 As this is my first patch to postgresql, I'm expecting I've done
  something wrong.  Please if you want me to fix something up, or
 just go away please say so ;)  I appreciate that this is a trivial
 patch, and perhaps doesn't add value except for my very specific
 use case* feel free to ignore it =)
 
 Thanks for offering this to the community.  I see you've already
 gotten feedback on the patch, with a suggestion for a different
 approach.  Don't let that discourage you -- very few patches get in
 without needing to be modified based on review and feedback.
 
 If you haven't already done so, please review this page and its
 links:
 
 http://www.postgresql.org/developer/
 
 Of particular interest is the Developer FAQ:
 
 http://wiki.postgresql.org/wiki/Developer_FAQ
 
 You should also be aware of the development cycle, which (when not
 in feature freeze for beta testing) involves alternating periods of
 focused development and code review (the latter called CommitFests):
 
 http://wiki.postgresql.org/wiki/CommitFest
 
 We are now in the midst of a CF, so it would be great if you could
 join in that as a reviewer.  Newly submitted patches should be
 submitted to the open CF:
 
 http://commitfest.postgresql.org/action/commitfest_view/open
 
 You might want to consider what Tom said and submit a modified patch
 for the next review cycle.
 
 Welcome!
 
 -Kevin


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [PATCH] Addition of some trivial auto vacuum logging

2011-09-27 Thread Stephen Frost
* Royce Ausburn (royce...@inomial.com) wrote:
 Just a question:  is the utility great enough to warrant me working further 
 on this?  I have no real desire to implement this particular feature -- I 
 simply saw an opportunity to cut my teeth on something easy.  I'd be happy to 
 find something on the TODO list instead if this feature isn't really 
 worthwhile.

Seeing as how it's already got one committer willing to consider it (and
that one tends to go the other direction on new features..), I'd
definitely say it's worthwhile.  That doesn't mean it's guaranteed to
get in, but I'd put the probability above 75% given that feedback.
That's pretty good overall. :)

 Tom's suggestion looks like it's less trivial that I can do just yet, but 
 I'll take a look and ask for help if I need it.

Don't let the notion of fiddling with the catalogs (system tables)
discourage you..  It's really not all *that* bad.  What you will need to
figure out (and which I don't recall offhand..) is if you can just
update those catalogs directly from VACUUM or if you need to go through
the statistics collecter (which involves a bit of UDP communication, but
hopefully we've abstracted that out enough that you won't have to deal
with it directly really..).

Looking at an existing example case where VACUUM is doing something that
updates the stat tables (such as under the 'ANALYZE' option) will help
out a lot, I'm sure.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] [PATCH] Addition of some trivial auto vacuum logging

2011-09-27 Thread Alvaro Herrera

Excerpts from Royce Ausburn's message of mar sep 27 21:28:26 -0300 2011:
 Thanks for the tips guys. 
 
 Just a question:  is the utility great enough to warrant me working further 
 on this?  I have no real desire to implement this particular feature -- I 
 simply saw an opportunity to cut my teeth on something easy.  I'd be happy to 
 find something on the TODO list instead if this feature isn't really 
 worthwhile.

First patches are always going to be small things.  If you try to tackle
something too large, chances are you'll never finish, despair utterly
and throw yourself off a nearby bridge.  Surely it's better to set
realistic goals, start small and build slowly from there.

 Tom's suggestion looks like it's less trivial that I can do just yet, but 
 I'll take a look and ask for help if I need it.

It's not that difficult.  Just do a search on git log
src/backend/postmaster/pgstat.c for patches that add a new column
somewhere.

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [PATCH] Addition of some trivial auto vacuum logging

2011-09-27 Thread Tom Lane
Alvaro Herrera alvhe...@commandprompt.com writes:
 Excerpts from Royce Ausburn's message of mar sep 27 21:28:26 -0300 2011:
 Tom's suggestion looks like it's less trivial that I can do just yet, but 
 I'll take a look and ask for help if I need it.

 It's not that difficult.  Just do a search on git log
 src/backend/postmaster/pgstat.c for patches that add a new column
 somewhere.

Yeah, I was just about to say the same thing.  Find a previous patch
that adds a feature similar to what you have in mind, and crib like mad.
We've added enough stats counters over time that you should have several
models to work from.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers