Thread: [PATCH] Addition of some trivial auto vacuum logging

[PATCH] Addition of some trivial auto vacuum logging

From

Royce Ausburn

Date:

27 September 2011, 17:51:33

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: [PATCH] Addition of some trivial auto vacuum logging

From

Tom Lane

Date:

27 September 2011, 18:22:51

Royce Ausburn <royce.ml@inomial.com> writes:
>  The attached patch adds extra detail the the existing autovacuum log message that is emitted when the
log_autovacuum_min_durationthreshold is met, exposing the unremovable dead tuple count similar to what you get from
VACUUMVERBOSE.
 

> 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

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

From

"Kevin Grittner"

Date:

27 September 2011, 18:42:45

Royce Ausburn <royce.ml@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

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

From

Royce Ausburn

Date:

28 September 2011, 00:28:43

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
implementthis particular feature -- I simply saw an opportunity to cut my teeth on something easy.  I'd be happy to
findsomething 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.ml@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

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

From

Stephen Frost

Date:

28 September 2011, 00:36:37

* Royce Ausburn (royce.ml@inomial.com) wrote:
> Just a question:  is the utility great enough to warrant me working further on this?  I have no real desire to
implementthis particular feature -- I simply saw an opportunity to cut my teeth on something easy.  I'd be happy to
findsomething 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

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

From

Alvaro Herrera

Date:

28 September 2011, 00:40:41

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
implementthis particular feature -- I simply saw an opportunity to cut my teeth on something easy.  I'd be happy to
findsomething 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 <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

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

From

Tom Lane

Date:

28 September 2011, 01:17:40

Alvaro Herrera <alvherre@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
needit.
 

> 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

Re: [PATCH] Unremovable tuple monitoring (was: Addition of some trivial auto vacuum logging)

From

Royce Ausburn

Date:

03 October 2011, 13:18:07

On 28/09/2011, at 11:17 AM, Tom Lane wrote:

> Alvaro Herrera <alvherre@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
needit. 
>
>> 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.


This patch does as Tom suggested, adding a column to the pg_stat_all_tables view which exposes the number of
unremovabletuples in the last vacuum.  This patch does not include my previous work to log this information as part of
auto_vacuum'slogging. 

User visible additions:
New column pg_stat_all_tables.n_unremovable_tup
New function bigint pg_stat_get_unremovable_tuples(oid)

A few notes / questions:

- I'm not sure if I'm supposed to update CATALOG_VERSION_NO in catalog.h.  In this patch I have.

- I'm not sure about how I should be selecting an OID for my new stats function.  I used the unused_oids script and
pickedone that seemed reasonable. 

- The VACUUM FULL implementation in cluster.c doesn't do any stats updating similar to vacuumlazy.c, so I haven't don't
anythingthere… (is this right?  A vacuum full may also encounter unremovable tuples, right?) 

- I'm not usually a C developer, so peeps reviewing please watch for noob mistakes.

Cheers,

--Royce



diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index a19e3f0..af7b235 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -328,7 +328,8 @@ postgres: <replaceable>user</> <replaceable>database</> <replaceable>host</> <re
       belonging to the table), number of live rows fetched by index
       scans, numbers of row insertions, updates, and deletions,
       number of row updates that were HOT (i.e., no separate index update),
-      numbers of live and dead rows,
+      numbers of live and dead rows,
+      the number of dead tuples not removed in the last vacuum,
       the last time the table was non-<option>FULL</> vacuumed manually,
       the last time it was vacuumed by the autovacuum daemon,
       the last time it was analyzed manually,
@@ -764,6 +765,14 @@ postgres: <replaceable>user</> <replaceable>database</> <replaceable>host</> <re
        Number of dead rows in table
       </entry>
      </row>
+
+     <row>
+      <entry><literal><function>pg_stat_get_unremovable_tuples</function>(<type>oid</type>)</literal></entry>
+      <entry><type>bigint</type></entry>
+      <entry>
+       Number of dead rows not removed in the table's last vacuum
+      </entry>
+     </row>

      <row>
       <entry><literal><function>pg_stat_get_blocks_fetched</function>(<type>oid</type>)</literal></entry>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 2253ca8..9c18dc7 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -353,6 +353,7 @@ CREATE VIEW pg_stat_all_tables AS
             pg_stat_get_tuples_hot_updated(C.oid) AS n_tup_hot_upd,
             pg_stat_get_live_tuples(C.oid) AS n_live_tup,
             pg_stat_get_dead_tuples(C.oid) AS n_dead_tup,
+            pg_stat_get_unremovable_tuples(C.oid) AS n_unremovable_tup,
             pg_stat_get_last_vacuum_time(C.oid) as last_vacuum,
             pg_stat_get_last_autovacuum_time(C.oid) as last_autovacuum,
             pg_stat_get_last_analyze_time(C.oid) as last_analyze,
diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index cf8337b..140fe92 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        unremovable_tuples; /* count of dead tuples not yet removable */
     BlockNumber pages_removed;
     double        tuples_deleted;
     BlockNumber nonempty_pages; /* actually, last nonempty page + 1 */
@@ -245,7 +246,8 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
     /* report results to the stats collector, too */
     pgstat_report_vacuum(RelationGetRelid(onerel),
                          onerel->rd_rel->relisshared,
-                         new_rel_tuples);
+                         new_rel_tuples,
+                         vacrelstats->unremovable_tuples);

     /* and log the action if appropriate */
     if (IsAutoVacuumWorkerProcess() && Log_autovacuum_min_duration >= 0)
@@ -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->unremovable_tuples = nkeep;

     /* now we can compute the new value for pg_class.reltuples */
     vacrelstats->new_rel_tuples = vac_estimate_reltuples(onerel, false,
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 9132db7..40d1107 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -1251,7 +1251,7 @@ pgstat_report_autovac(Oid dboid)
  * ---------
  */
 void
-pgstat_report_vacuum(Oid tableoid, bool shared, PgStat_Counter tuples)
+pgstat_report_vacuum(Oid tableoid, bool shared, PgStat_Counter tuples, PgStat_Counter m_unremovable_tuples)
 {
     PgStat_MsgVacuum msg;

@@ -1264,6 +1264,7 @@ pgstat_report_vacuum(Oid tableoid, bool shared, PgStat_Counter tuples)
     msg.m_autovacuum = IsAutoVacuumWorkerProcess();
     msg.m_vacuumtime = GetCurrentTimestamp();
     msg.m_tuples = tuples;
+    msg.m_unremovable_tuples = m_unremovable_tuples;
     pgstat_send(&msg, sizeof(msg));
 }

@@ -4202,6 +4203,7 @@ pgstat_recv_vacuum(PgStat_MsgVacuum *msg, int len)
     tabentry = pgstat_get_tab_entry(dbentry, msg->m_tableoid, true);

     tabentry->n_live_tuples = msg->m_tuples;
+    tabentry->n_unremovable_tuples = msg->m_unremovable_tuples;
     /* Resetting dead_tuples to 0 is an approximation ... */
     tabentry->n_dead_tuples = 0;

diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 7792b33..fb60fc5 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -33,6 +33,7 @@ extern Datum pg_stat_get_tuples_deleted(PG_FUNCTION_ARGS);
 extern Datum pg_stat_get_tuples_hot_updated(PG_FUNCTION_ARGS);
 extern Datum pg_stat_get_live_tuples(PG_FUNCTION_ARGS);
 extern Datum pg_stat_get_dead_tuples(PG_FUNCTION_ARGS);
+extern Datum pg_stat_get_unremovable_tuples(PG_FUNCTION_ARGS);
 extern Datum pg_stat_get_blocks_fetched(PG_FUNCTION_ARGS);
 extern Datum pg_stat_get_blocks_hit(PG_FUNCTION_ARGS);
 extern Datum pg_stat_get_last_vacuum_time(PG_FUNCTION_ARGS);
@@ -256,6 +257,22 @@ pg_stat_get_dead_tuples(PG_FUNCTION_ARGS)
     PG_RETURN_INT64(result);
 }

+Datum
+pg_stat_get_unremovable_tuples(PG_FUNCTION_ARGS)
+{
+    Oid            relid = PG_GETARG_OID(0);
+    int64        result;
+    PgStat_StatTabEntry *tabentry;
+
+    if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
+        result = 0;
+    else
+        result = (int64) (tabentry->n_unremovable_tuples);
+
+    PG_RETURN_INT64(result);
+}
+
+

 Datum
 pg_stat_get_blocks_fetched(PG_FUNCTION_ARGS)
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index f3c8bb4..950f1f2 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -53,6 +53,6 @@
  */

 /*                            yyyymmddN */
-#define CATALOG_VERSION_NO    201109071
+#define CATALOG_VERSION_NO    201110031

 #endif
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 96f43fe..69f6415 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -2526,6 +2526,8 @@ DATA(insert OID = 2878 (  pg_stat_get_live_tuples    PGNSP PGUID 12 1 0 0 0 f f f t
 DESCR("statistics: number of live tuples");
 DATA(insert OID = 2879 (  pg_stat_get_dead_tuples    PGNSP PGUID 12 1 0 0 0 f f f t f s 1 0 20 "26" _null_ _null_
_null__null_ pg_stat_get_dead_tuples _null_ _null_ _null_ )); 
 DESCR("statistics: number of dead tuples");
+DATA(insert OID = 3122 (  pg_stat_get_unremovable_tuples    PGNSP PGUID 12 1 0 0 0 f f f t f s 1 0 20 "26" _null_
_null__null_ _null_ pg_stat_get_unremovable_tuples _null_ _null_ _null_ )); 
+DESCR("statistics: number of dead tuples not yet removable");
 DATA(insert OID = 1934 (  pg_stat_get_blocks_fetched    PGNSP PGUID 12 1 0 0 0 f f f t f s 1 0 20 "26" _null_ _null_
_null__null_ pg_stat_get_blocks_fetched _null_ _null_ _null_ )); 
 DESCR("statistics: number of blocks fetched");
 DATA(insert OID = 1935 (  pg_stat_get_blocks_hit        PGNSP PGUID 12 1 0 0 0 f f f t f s 1 0 20 "26" _null_ _null_
_null__null_ pg_stat_get_blocks_hit _null_ _null_ _null_ )); 
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 20c4d43..e1b082e 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -326,6 +326,7 @@ typedef struct PgStat_MsgVacuum
     bool        m_autovacuum;
     TimestampTz m_vacuumtime;
     PgStat_Counter m_tuples;
+    PgStat_Counter m_unremovable_tuples;
 } PgStat_MsgVacuum;


@@ -539,6 +540,7 @@ typedef struct PgStat_StatTabEntry

     PgStat_Counter n_live_tuples;
     PgStat_Counter n_dead_tuples;
+    PgStat_Counter n_unremovable_tuples;
     PgStat_Counter changes_since_analyze;

     PgStat_Counter blocks_fetched;
@@ -706,7 +708,7 @@ extern void pgstat_reset_single_counter(Oid objectid, PgStat_Single_Reset_Type t

 extern void pgstat_report_autovac(Oid dboid);
 extern void pgstat_report_vacuum(Oid tableoid, bool shared,
-                     PgStat_Counter tuples);
+                     PgStat_Counter tuples, PgStat_Counter unremovable_tuples);
 extern void pgstat_report_analyze(Relation rel,
                       PgStat_Counter livetuples, PgStat_Counter deadtuples);

Attachment

unremovable_tuples.txt

Re: [PATCH] Unremovable tuple monitoring (was: Addition of some trivial auto vacuum logging)

From

Robert Haas

Date:

04 October 2011, 12:26:40

On Mon, Oct 3, 2011 at 9:17 AM, Royce Ausburn <royce.ml@inomial.com> wrote:
> - I'm not sure if I'm supposed to update CATALOG_VERSION_NO in catalog.h.  In this patch I have.

Generally that is left to the committer, as the correct value depends
on the value at the time of commit, not the time you submit the patch;
and including it in the patch tends to result in failing hunks, since
the value changes fairly frequently.

> - I'm not sure about how I should be selecting an OID for my new stats function.  I used the unused_oids script and
pickedone that seemed reasonable. 

That's the way to do it.

> - The VACUUM FULL implementation in cluster.c doesn't do any stats updating similar to vacuumlazy.c, so I haven't
don'tanything there… (is this right?  A vacuum full may also encounter unremovable tuples, right?) 

We've occasionally heard grumblings about making cluster do more stats
updating, but your patch should just go along with whatever's being
done now in similar cases.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [PATCH] Unremovable tuple monitoring (was: Addition of some trivial auto vacuum logging)

From

Royce Ausburn

Date:

04 October 2011, 22:45:55

On 04/10/2011, at 11:26 PM, Robert Haas wrote:

> On Mon, Oct 3, 2011 at 9:17 AM, Royce Ausburn <royce.ml@inomial.com> wrote:
>> - I'm not sure if I'm supposed to update CATALOG_VERSION_NO in catalog.h.  In this patch I have.
>
> Generally that is left to the committer, as the correct value depends
> on the value at the time of commit, not the time you submit the patch;
> and including it in the patch tends to result in failing hunks, since
> the value changes fairly frequently.

>> - The VACUUM FULL implementation in cluster.c doesn't do any stats updating similar to vacuumlazy.c, so I haven't
don'tanything there… (is this right?  A vacuum full may also encounter unremovable tuples, right?) 
>
> We've occasionally heard grumblings about making cluster do more stats
> updating, but your patch should just go along with whatever's being
> done now in similar cases.

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
couldtake 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
starta new thread for discussion. 

Attached is v2 of my patch that doesn't update CATALOG_VERSION_NO.  I've also fixed the name of an argument to
pgstat_report_vacuumwhich I don't think was particularly good, and I've replace the word "tuple" with "row" in some
docsI added for consistency. 

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? 

--Royce



diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index a19e3f0..8692580 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -328,7 +328,8 @@ postgres: <replaceable>user</> <replaceable>database</> <replaceable>host</> <re
       belonging to the table), number of live rows fetched by index
       scans, numbers of row insertions, updates, and deletions,
       number of row updates that were HOT (i.e., no separate index update),
-      numbers of live and dead rows,
+      numbers of live and dead rows,
+      the number of dead rows not removed in the last vacuum,
       the last time the table was non-<option>FULL</> vacuumed manually,
       the last time it was vacuumed by the autovacuum daemon,
       the last time it was analyzed manually,
@@ -764,6 +765,14 @@ postgres: <replaceable>user</> <replaceable>database</> <replaceable>host</> <re
        Number of dead rows in table
       </entry>
      </row>
+
+     <row>
+      <entry><literal><function>pg_stat_get_unremovable_tuples</function>(<type>oid</type>)</literal></entry>
+      <entry><type>bigint</type></entry>
+      <entry>
+       Number of dead rows not removed in the table's last vacuum
+      </entry>
+     </row>

      <row>
       <entry><literal><function>pg_stat_get_blocks_fetched</function>(<type>oid</type>)</literal></entry>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 2253ca8..9c18dc7 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -353,6 +353,7 @@ CREATE VIEW pg_stat_all_tables AS
             pg_stat_get_tuples_hot_updated(C.oid) AS n_tup_hot_upd,
             pg_stat_get_live_tuples(C.oid) AS n_live_tup,
             pg_stat_get_dead_tuples(C.oid) AS n_dead_tup,
+            pg_stat_get_unremovable_tuples(C.oid) AS n_unremovable_tup,
             pg_stat_get_last_vacuum_time(C.oid) as last_vacuum,
             pg_stat_get_last_autovacuum_time(C.oid) as last_autovacuum,
             pg_stat_get_last_analyze_time(C.oid) as last_analyze,
diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index cf8337b..140fe92 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        unremovable_tuples; /* count of dead tuples not yet removable */
     BlockNumber pages_removed;
     double        tuples_deleted;
     BlockNumber nonempty_pages; /* actually, last nonempty page + 1 */
@@ -245,7 +246,8 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
     /* report results to the stats collector, too */
     pgstat_report_vacuum(RelationGetRelid(onerel),
                          onerel->rd_rel->relisshared,
-                         new_rel_tuples);
+                         new_rel_tuples,
+                         vacrelstats->unremovable_tuples);

     /* and log the action if appropriate */
     if (IsAutoVacuumWorkerProcess() && Log_autovacuum_min_duration >= 0)
@@ -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->unremovable_tuples = nkeep;

     /* now we can compute the new value for pg_class.reltuples */
     vacrelstats->new_rel_tuples = vac_estimate_reltuples(onerel, false,
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 9132db7..d974a96 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -1251,7 +1251,7 @@ pgstat_report_autovac(Oid dboid)
  * ---------
  */
 void
-pgstat_report_vacuum(Oid tableoid, bool shared, PgStat_Counter tuples)
+pgstat_report_vacuum(Oid tableoid, bool shared, PgStat_Counter tuples, PgStat_Counter unremovable_tuples)
 {
     PgStat_MsgVacuum msg;

@@ -1264,6 +1264,7 @@ pgstat_report_vacuum(Oid tableoid, bool shared, PgStat_Counter tuples)
     msg.m_autovacuum = IsAutoVacuumWorkerProcess();
     msg.m_vacuumtime = GetCurrentTimestamp();
     msg.m_tuples = tuples;
+    msg.m_unremovable_tuples = unremovable_tuples;
     pgstat_send(&msg, sizeof(msg));
 }

@@ -4202,6 +4203,7 @@ pgstat_recv_vacuum(PgStat_MsgVacuum *msg, int len)
     tabentry = pgstat_get_tab_entry(dbentry, msg->m_tableoid, true);

     tabentry->n_live_tuples = msg->m_tuples;
+    tabentry->n_unremovable_tuples = msg->m_unremovable_tuples;
     /* Resetting dead_tuples to 0 is an approximation ... */
     tabentry->n_dead_tuples = 0;

diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 7792b33..fb60fc5 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -33,6 +33,7 @@ extern Datum pg_stat_get_tuples_deleted(PG_FUNCTION_ARGS);
 extern Datum pg_stat_get_tuples_hot_updated(PG_FUNCTION_ARGS);
 extern Datum pg_stat_get_live_tuples(PG_FUNCTION_ARGS);
 extern Datum pg_stat_get_dead_tuples(PG_FUNCTION_ARGS);
+extern Datum pg_stat_get_unremovable_tuples(PG_FUNCTION_ARGS);
 extern Datum pg_stat_get_blocks_fetched(PG_FUNCTION_ARGS);
 extern Datum pg_stat_get_blocks_hit(PG_FUNCTION_ARGS);
 extern Datum pg_stat_get_last_vacuum_time(PG_FUNCTION_ARGS);
@@ -256,6 +257,22 @@ pg_stat_get_dead_tuples(PG_FUNCTION_ARGS)
     PG_RETURN_INT64(result);
 }

+Datum
+pg_stat_get_unremovable_tuples(PG_FUNCTION_ARGS)
+{
+    Oid            relid = PG_GETARG_OID(0);
+    int64        result;
+    PgStat_StatTabEntry *tabentry;
+
+    if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
+        result = 0;
+    else
+        result = (int64) (tabentry->n_unremovable_tuples);
+
+    PG_RETURN_INT64(result);
+}
+
+

 Datum
 pg_stat_get_blocks_fetched(PG_FUNCTION_ARGS)
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 96f43fe..69f6415 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -2526,6 +2526,8 @@ DATA(insert OID = 2878 (  pg_stat_get_live_tuples    PGNSP PGUID 12 1 0 0 0 f f f t
 DESCR("statistics: number of live tuples");
 DATA(insert OID = 2879 (  pg_stat_get_dead_tuples    PGNSP PGUID 12 1 0 0 0 f f f t f s 1 0 20 "26" _null_ _null_
_null__null_ pg_stat_get_dead_tuples _null_ _null_ _null_ )); 
 DESCR("statistics: number of dead tuples");
+DATA(insert OID = 3122 (  pg_stat_get_unremovable_tuples    PGNSP PGUID 12 1 0 0 0 f f f t f s 1 0 20 "26" _null_
_null__null_ _null_ pg_stat_get_unremovable_tuples _null_ _null_ _null_ )); 
+DESCR("statistics: number of dead tuples not yet removable");
 DATA(insert OID = 1934 (  pg_stat_get_blocks_fetched    PGNSP PGUID 12 1 0 0 0 f f f t f s 1 0 20 "26" _null_ _null_
_null__null_ pg_stat_get_blocks_fetched _null_ _null_ _null_ )); 
 DESCR("statistics: number of blocks fetched");
 DATA(insert OID = 1935 (  pg_stat_get_blocks_hit        PGNSP PGUID 12 1 0 0 0 f f f t f s 1 0 20 "26" _null_ _null_
_null__null_ pg_stat_get_blocks_hit _null_ _null_ _null_ )); 
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 20c4d43..e1b082e 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -326,6 +326,7 @@ typedef struct PgStat_MsgVacuum
     bool        m_autovacuum;
     TimestampTz m_vacuumtime;
     PgStat_Counter m_tuples;
+    PgStat_Counter m_unremovable_tuples;
 } PgStat_MsgVacuum;


@@ -539,6 +540,7 @@ typedef struct PgStat_StatTabEntry

     PgStat_Counter n_live_tuples;
     PgStat_Counter n_dead_tuples;
+    PgStat_Counter n_unremovable_tuples;
     PgStat_Counter changes_since_analyze;

     PgStat_Counter blocks_fetched;
@@ -706,7 +708,7 @@ extern void pgstat_reset_single_counter(Oid objectid, PgStat_Single_Reset_Type t

 extern void pgstat_report_autovac(Oid dboid);
 extern void pgstat_report_vacuum(Oid tableoid, bool shared,
-                     PgStat_Counter tuples);
+                     PgStat_Counter tuples, PgStat_Counter unremovable_tuples);
 extern void pgstat_report_analyze(Relation rel,
                       PgStat_Counter livetuples, PgStat_Counter deadtuples);