Re: [HACKERS] VACUUM and ANALYZE disagreeing on what reltuples means - Mailing list pgsql-hackers

Haribabu Kommi <kommi.haribabu@gmail.com> writes:
> The changes are fine and now it reports proper live tuples in both
> pg_class and stats. The other issue of continuous vacuum operation
> leading to decrease of number of live tuples is not related to this
> patch and that can be handled separately.

I did not like this patch too much, because it did nothing to fix
the underlying problem of confusion between "live tuples" and "total
tuples"; in fact, it made that worse, because now the comment on
LVRelStats.new_rel_tuples is a lie.  And there's at least one usage
of that field value where I think we do want total tuples not live
tuples: where we pass it to index AM cleanup functions.  Indexes
don't really care whether heap entries are live or not, only that
they're there.  Plus the VACUUM VERBOSE log message that uses the
field is supposed to be reporting total tuples not live tuples.

I hacked up the patch trying to make that better, finding along the
way that contrib/pgstattuple shared in the confusion about what
it was trying to count.  Results attached.

However, I'm not sure we're there yet, because there remains a fairly
nasty discrepancy even once we've gotten everyone onto the same page
about reltuples counting just live tuples: VACUUM and ANALYZE have
different definitions of what's "live".  In particular they do not treat
INSERT_IN_PROGRESS and DELETE_IN_PROGRESS tuples the same.  Should we
try to do something about that?  If so, what?  It looks like ANALYZE's
behavior is pretty tightly constrained, judging by the comments in
acquire_sample_rows.

Another problem is that it looks like CREATE INDEX will set reltuples
to the total number of heap entries it chose to index, because that
is what IndexBuildHeapScan counts.  Maybe we should adjust that?

            regards, tom lane

diff --git a/contrib/pgstattuple/pgstatapprox.c b/contrib/pgstattuple/pgstatapprox.c
index 5bf0613..68211c6 100644
*** a/contrib/pgstattuple/pgstatapprox.c
--- b/contrib/pgstattuple/pgstatapprox.c
*************** statapprox_heap(Relation rel, output_typ
*** 68,74 ****
      Buffer        vmbuffer = InvalidBuffer;
      BufferAccessStrategy bstrategy;
      TransactionId OldestXmin;
-     uint64        misc_count = 0;

      OldestXmin = GetOldestXmin(rel, PROCARRAY_FLAGS_VACUUM);
      bstrategy = GetAccessStrategy(BAS_BULKREAD);
--- 68,73 ----
*************** statapprox_heap(Relation rel, output_typ
*** 153,177 ****
              tuple.t_tableOid = RelationGetRelid(rel);

              /*
!              * We count live and dead tuples, but we also need to add up
!              * others in order to feed vac_estimate_reltuples.
               */
              switch (HeapTupleSatisfiesVacuum(&tuple, OldestXmin, buf))
              {
-                 case HEAPTUPLE_RECENTLY_DEAD:
-                     misc_count++;
-                     /* Fall through */
-                 case HEAPTUPLE_DEAD:
-                     stat->dead_tuple_len += tuple.t_len;
-                     stat->dead_tuple_count++;
-                     break;
                  case HEAPTUPLE_LIVE:
                      stat->tuple_len += tuple.t_len;
                      stat->tuple_count++;
                      break;
!                 case HEAPTUPLE_INSERT_IN_PROGRESS:
!                 case HEAPTUPLE_DELETE_IN_PROGRESS:
!                     misc_count++;
                      break;
                  default:
                      elog(ERROR, "unexpected HeapTupleSatisfiesVacuum result");
--- 152,172 ----
              tuple.t_tableOid = RelationGetRelid(rel);

              /*
!              * We follow VACUUM's lead in counting INSERT_IN_PROGRESS and
!              * DELETE_IN_PROGRESS tuples as "live".
               */
              switch (HeapTupleSatisfiesVacuum(&tuple, OldestXmin, buf))
              {
                  case HEAPTUPLE_LIVE:
+                 case HEAPTUPLE_INSERT_IN_PROGRESS:
+                 case HEAPTUPLE_DELETE_IN_PROGRESS:
                      stat->tuple_len += tuple.t_len;
                      stat->tuple_count++;
                      break;
!                 case HEAPTUPLE_DEAD:
!                 case HEAPTUPLE_RECENTLY_DEAD:
!                     stat->dead_tuple_len += tuple.t_len;
!                     stat->dead_tuple_count++;
                      break;
                  default:
                      elog(ERROR, "unexpected HeapTupleSatisfiesVacuum result");
*************** statapprox_heap(Relation rel, output_typ
*** 184,191 ****

      stat->table_len = (uint64) nblocks * BLCKSZ;

      stat->tuple_count = vac_estimate_reltuples(rel, false, nblocks, scanned,
!                                                stat->tuple_count + misc_count);

      /*
       * Calculate percentages if the relation has one or more pages.
--- 179,190 ----

      stat->table_len = (uint64) nblocks * BLCKSZ;

+     /*
+      * Extrapolate the live-tuple count to the whole table in the same way
+      * that VACUUM does.  (XXX shouldn't we also extrapolate tuple_len?)
+      */
      stat->tuple_count = vac_estimate_reltuples(rel, false, nblocks, scanned,
!                                                stat->tuple_count);

      /*
       * Calculate percentages if the relation has one or more pages.
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index ef60a58..87bba31 100644
*** a/doc/src/sgml/catalogs.sgml
--- b/doc/src/sgml/catalogs.sgml
*************** SCRAM-SHA-256$<replaceable><iteration
*** 1739,1746 ****
        <entry><type>float4</type></entry>
        <entry></entry>
        <entry>
!        Number of rows in the table.  This is only an estimate used by the
!        planner.  It is updated by <command>VACUUM</command>,
         <command>ANALYZE</command>, and a few DDL commands such as
         <command>CREATE INDEX</command>.
        </entry>
--- 1739,1746 ----
        <entry><type>float4</type></entry>
        <entry></entry>
        <entry>
!        Number of live rows in the table.  This is only an estimate used by
!        the planner.  It is updated by <command>VACUUM</command>,
         <command>ANALYZE</command>, and a few DDL commands such as
         <command>CREATE INDEX</command>.
        </entry>
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index cbd6e9b..067034d 100644
*** a/src/backend/commands/vacuum.c
--- b/src/backend/commands/vacuum.c
*************** vacuum_set_xid_limits(Relation rel,
*** 771,776 ****
--- 771,779 ----
   *        we take the old value of pg_class.reltuples as a measurement of the
   *        tuple density in the unscanned pages.
   *
+  *        Note: scanned_tuples should count only *live* tuples, since
+  *        pg_class.reltuples is defined that way.
+  *
   *        This routine is shared by VACUUM and ANALYZE.
   */
  double
diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index 6587db7..ed14db2 100644
*** a/src/backend/commands/vacuumlazy.c
--- b/src/backend/commands/vacuumlazy.c
*************** typedef struct LVRelStats
*** 115,122 ****
      BlockNumber frozenskipped_pages;    /* # of frozen pages we skipped */
      BlockNumber tupcount_pages; /* pages whose tuples we counted */
      double        scanned_tuples; /* counts only tuples on tupcount_pages */
!     double        old_rel_tuples; /* previous value of pg_class.reltuples */
      double        new_rel_tuples; /* new estimated total # of tuples */
      double        new_dead_tuples;    /* new estimated total # of dead tuples */
      BlockNumber pages_removed;
      double        tuples_deleted;
--- 115,123 ----
      BlockNumber frozenskipped_pages;    /* # of frozen pages we skipped */
      BlockNumber tupcount_pages; /* pages whose tuples we counted */
      double        scanned_tuples; /* counts only tuples on tupcount_pages */
!     double        old_live_tuples;    /* previous value of pg_class.reltuples */
      double        new_rel_tuples; /* new estimated total # of tuples */
+     double        new_live_tuples;    /* new estimated total # of live tuples */
      double        new_dead_tuples;    /* new estimated total # of dead tuples */
      BlockNumber pages_removed;
      double        tuples_deleted;
*************** lazy_vacuum_rel(Relation onerel, int opt
*** 196,202 ****
      TransactionId xidFullScanLimit;
      MultiXactId mxactFullScanLimit;
      BlockNumber new_rel_pages;
-     double        new_rel_tuples;
      BlockNumber new_rel_allvisible;
      double        new_live_tuples;
      TransactionId new_frozen_xid;
--- 197,202 ----
*************** lazy_vacuum_rel(Relation onerel, int opt
*** 245,251 ****
      vacrelstats = (LVRelStats *) palloc0(sizeof(LVRelStats));

      vacrelstats->old_rel_pages = onerel->rd_rel->relpages;
!     vacrelstats->old_rel_tuples = onerel->rd_rel->reltuples;
      vacrelstats->num_index_scans = 0;
      vacrelstats->pages_removed = 0;
      vacrelstats->lock_waiter_detected = false;
--- 245,251 ----
      vacrelstats = (LVRelStats *) palloc0(sizeof(LVRelStats));

      vacrelstats->old_rel_pages = onerel->rd_rel->relpages;
!     vacrelstats->old_live_tuples = onerel->rd_rel->reltuples;
      vacrelstats->num_index_scans = 0;
      vacrelstats->pages_removed = 0;
      vacrelstats->lock_waiter_detected = false;
*************** lazy_vacuum_rel(Relation onerel, int opt
*** 311,321 ****
       * since then we don't know for certain that all tuples have a newer xmin.
       */
      new_rel_pages = vacrelstats->rel_pages;
!     new_rel_tuples = vacrelstats->new_rel_tuples;
      if (vacrelstats->tupcount_pages == 0 && new_rel_pages > 0)
      {
          new_rel_pages = vacrelstats->old_rel_pages;
!         new_rel_tuples = vacrelstats->old_rel_tuples;
      }

      visibilitymap_count(onerel, &new_rel_allvisible, NULL);
--- 311,321 ----
       * since then we don't know for certain that all tuples have a newer xmin.
       */
      new_rel_pages = vacrelstats->rel_pages;
!     new_live_tuples = vacrelstats->new_live_tuples;
      if (vacrelstats->tupcount_pages == 0 && new_rel_pages > 0)
      {
          new_rel_pages = vacrelstats->old_rel_pages;
!         new_live_tuples = vacrelstats->old_live_tuples;
      }

      visibilitymap_count(onerel, &new_rel_allvisible, NULL);
*************** lazy_vacuum_rel(Relation onerel, int opt
*** 327,333 ****

      vac_update_relstats(onerel,
                          new_rel_pages,
!                         new_rel_tuples,
                          new_rel_allvisible,
                          vacrelstats->hasindex,
                          new_frozen_xid,
--- 327,333 ----

      vac_update_relstats(onerel,
                          new_rel_pages,
!                         new_live_tuples,
                          new_rel_allvisible,
                          vacrelstats->hasindex,
                          new_frozen_xid,
*************** lazy_vacuum_rel(Relation onerel, int opt
*** 335,344 ****
                          false);

      /* report results to the stats collector, too */
-     new_live_tuples = new_rel_tuples - vacrelstats->new_dead_tuples;
-     if (new_live_tuples < 0)
-         new_live_tuples = 0;    /* just in case */
-
      pgstat_report_vacuum(RelationGetRelid(onerel),
                           onerel->rd_rel->relisshared,
                           new_live_tuples,
--- 335,340 ----
*************** lazy_scan_heap(Relation onerel, int opti
*** 1275,1284 ****
      vacrelstats->new_dead_tuples = nkeep;

      /* now we can compute the new value for pg_class.reltuples */
!     vacrelstats->new_rel_tuples = vac_estimate_reltuples(onerel, false,
!                                                          nblocks,
!                                                          vacrelstats->tupcount_pages,
!                                                          num_tuples);

      /*
       * Release any remaining pin on visibility map page.
--- 1271,1284 ----
      vacrelstats->new_dead_tuples = nkeep;

      /* now we can compute the new value for pg_class.reltuples */
!     vacrelstats->new_live_tuples = vac_estimate_reltuples(onerel, false,
!                                                           nblocks,
!                                                           vacrelstats->tupcount_pages,
!                                                           num_tuples - nkeep);
!
!     /* indexes probably care more about the total number of heap entries */
!     vacrelstats->new_rel_tuples =
!         vacrelstats->new_live_tuples + vacrelstats->new_dead_tuples;

      /*
       * Release any remaining pin on visibility map page.
*************** lazy_vacuum_index(Relation indrel,
*** 1614,1620 ****
      ivinfo.analyze_only = false;
      ivinfo.estimated_count = true;
      ivinfo.message_level = elevel;
!     ivinfo.num_heap_tuples = vacrelstats->old_rel_tuples;
      ivinfo.strategy = vac_strategy;

      /* Do bulk deletion */
--- 1614,1621 ----
      ivinfo.analyze_only = false;
      ivinfo.estimated_count = true;
      ivinfo.message_level = elevel;
!     /* We can only provide an approximate value of num_heap_tuples here */
!     ivinfo.num_heap_tuples = vacrelstats->old_live_tuples;
      ivinfo.strategy = vac_strategy;

      /* Do bulk deletion */
*************** lazy_cleanup_index(Relation indrel,
*** 1645,1650 ****
--- 1646,1652 ----
      ivinfo.analyze_only = false;
      ivinfo.estimated_count = (vacrelstats->tupcount_pages < vacrelstats->rel_pages);
      ivinfo.message_level = elevel;
+     /* Now we can provide a better estimate of total # of surviving tuples */
      ivinfo.num_heap_tuples = vacrelstats->new_rel_tuples;
      ivinfo.strategy = vac_strategy;


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