Re: Autovacuum of pg_database - Mailing list pgsql-admin

Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> These are all shared catalogs.  There are others, so you may still see
> more.  We got another report for pg_database
> https://www.postgresql.org/message-id/A9D40BB7-CFD6-46AF-A0A1-249F04878A2A%40amazon.com
> so I suppose there really is a bug.  I don't know what's going on there.

I think it's pretty obvious: autovacuum.c's rule for detecting whether
some other worker is already processing table X is wrong when X is a
shared table.  I propose the attached patch.

            regards, tom lane

diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index e2859df..4bc5d42 100644
*** a/src/backend/postmaster/autovacuum.c
--- b/src/backend/postmaster/autovacuum.c
*************** typedef struct autovac_table
*** 203,215 ****
   * wi_links        entry into free list or running list
   * wi_dboid        OID of the database this worker is supposed to work on
   * wi_tableoid    OID of the table currently being vacuumed, if any
   * wi_proc        pointer to PGPROC of the running worker, NULL if not started
   * wi_launchtime Time at which this worker was launched
   * wi_cost_*    Vacuum cost-based delay parameters current in this worker
   *
!  * All fields are protected by AutovacuumLock, except for wi_tableoid which is
!  * protected by AutovacuumScheduleLock (which is read-only for everyone except
!  * that worker itself).
   *-------------
   */
  typedef struct WorkerInfoData
--- 203,216 ----
   * wi_links        entry into free list or running list
   * wi_dboid        OID of the database this worker is supposed to work on
   * wi_tableoid    OID of the table currently being vacuumed, if any
+  * wi_tableshared    true if the table currently being vacuumed is a shared rel
   * wi_proc        pointer to PGPROC of the running worker, NULL if not started
   * wi_launchtime Time at which this worker was launched
   * wi_cost_*    Vacuum cost-based delay parameters current in this worker
   *
!  * All fields are protected by AutovacuumLock, except for wi_tableoid and
!  * wi_tableshared which are protected by AutovacuumScheduleLock (and are
!  * read-only for everyone except that worker itself).
   *-------------
   */
  typedef struct WorkerInfoData
*************** typedef struct WorkerInfoData
*** 217,222 ****
--- 218,224 ----
      dlist_node    wi_links;
      Oid            wi_dboid;
      Oid            wi_tableoid;
+     bool        wi_tableshared;
      PGPROC       *wi_proc;
      TimestampTz wi_launchtime;
      bool        wi_dobalance;
*************** autovac_balance_cost(void)
*** 1791,1798 ****
          }

          if (worker->wi_proc != NULL)
!             elog(DEBUG2, "autovac_balance_cost(pid=%u db=%u, rel=%u, dobalance=%s cost_limit=%d, cost_limit_base=%d,
cost_delay=%d)",
                   worker->wi_proc->pid, worker->wi_dboid, worker->wi_tableoid,
                   worker->wi_dobalance ? "yes" : "no",
                   worker->wi_cost_limit, worker->wi_cost_limit_base,
                   worker->wi_cost_delay);
--- 1793,1801 ----
          }

          if (worker->wi_proc != NULL)
!             elog(DEBUG2, "autovac_balance_cost(pid=%u db=%u, rel=%u%s, dobalance=%s cost_limit=%d,
cost_limit_base=%d,cost_delay=%d)", 
                   worker->wi_proc->pid, worker->wi_dboid, worker->wi_tableoid,
+                  worker->wi_tableshared ? " (shared)" : "",
                   worker->wi_dobalance ? "yes" : "no",
                   worker->wi_cost_limit, worker->wi_cost_limit_base,
                   worker->wi_cost_delay);
*************** do_autovacuum(void)
*** 1885,1893 ****
      HeapScanDesc relScan;
      Form_pg_database dbForm;
      List       *table_oids = NIL;
      HASHCTL        ctl;
      HTAB       *table_toast_map;
!     ListCell   *volatile cell;
      PgStat_StatDBEntry *shared;
      PgStat_StatDBEntry *dbentry;
      BufferAccessStrategy bstrategy;
--- 1888,1898 ----
      HeapScanDesc relScan;
      Form_pg_database dbForm;
      List       *table_oids = NIL;
+     List       *table_shares = NIL;
      HASHCTL        ctl;
      HTAB       *table_toast_map;
!     ListCell   *volatile lco;
!     ListCell   *volatile lcs;
      PgStat_StatDBEntry *shared;
      PgStat_StatDBEntry *dbentry;
      BufferAccessStrategy bstrategy;
*************** do_autovacuum(void)
*** 2004,2009 ****
--- 2009,2015 ----
          PgStat_StatTabEntry *tabentry;
          AutoVacOpts *relopts;
          Oid            relid;
+         bool        relisshared;
          bool        dovacuum;
          bool        doanalyze;
          bool        wraparound;
*************** do_autovacuum(void)
*** 2013,2022 ****
              continue;

          relid = HeapTupleGetOid(tuple);

          /* Fetch reloptions and the pgstat entry for this table */
          relopts = extract_autovac_opts(tuple, pg_class_desc);
!         tabentry = get_pgstat_tabentry_relid(relid, classForm->relisshared,
                                               shared, dbentry);

          /* Check if it needs vacuum or analyze */
--- 2019,2029 ----
              continue;

          relid = HeapTupleGetOid(tuple);
+         relisshared = classForm->relisshared;

          /* Fetch reloptions and the pgstat entry for this table */
          relopts = extract_autovac_opts(tuple, pg_class_desc);
!         tabentry = get_pgstat_tabentry_relid(relid, relisshared,
                                               shared, dbentry);

          /* Check if it needs vacuum or analyze */
*************** do_autovacuum(void)
*** 2069,2077 ****
          }
          else
          {
!             /* relations that need work are added to table_oids */
              if (dovacuum || doanalyze)
                  table_oids = lappend_oid(table_oids, relid);

              /*
               * Remember the association for the second pass.  Note: we must do
--- 2076,2087 ----
          }
          else
          {
!             /* relations that need work are added to table_oids/table_shares */
              if (dovacuum || doanalyze)
+             {
                  table_oids = lappend_oid(table_oids, relid);
+                 table_shares = lappend_int(table_shares, relisshared);
+             }

              /*
               * Remember the association for the second pass.  Note: we must do
*************** do_autovacuum(void)
*** 2117,2122 ****
--- 2127,2133 ----
          Form_pg_class classForm = (Form_pg_class) GETSTRUCT(tuple);
          PgStat_StatTabEntry *tabentry;
          Oid            relid;
+         bool        relisshared;
          AutoVacOpts *relopts = NULL;
          bool        dovacuum;
          bool        doanalyze;
*************** do_autovacuum(void)
*** 2129,2134 ****
--- 2140,2146 ----
              continue;

          relid = HeapTupleGetOid(tuple);
+         relisshared = classForm->relisshared;

          /*
           * fetch reloptions -- if this toast table does not have them, try the
*************** do_autovacuum(void)
*** 2146,2152 ****
          }

          /* Fetch the pgstat entry for this table */
!         tabentry = get_pgstat_tabentry_relid(relid, classForm->relisshared,
                                               shared, dbentry);

          relation_needs_vacanalyze(relid, relopts, classForm, tabentry,
--- 2158,2164 ----
          }

          /* Fetch the pgstat entry for this table */
!         tabentry = get_pgstat_tabentry_relid(relid, relisshared,
                                               shared, dbentry);

          relation_needs_vacanalyze(relid, relopts, classForm, tabentry,
*************** do_autovacuum(void)
*** 2155,2161 ****
--- 2167,2176 ----

          /* ignore analyze for toast tables */
          if (dovacuum)
+         {
              table_oids = lappend_oid(table_oids, relid);
+             table_shares = lappend_int(table_shares, relisshared);
+         }
      }

      heap_endscan(relScan);
*************** do_autovacuum(void)
*** 2181,2189 ****
      /*
       * Perform operations on collected tables.
       */
!     foreach(cell, table_oids)
      {
!         Oid            relid = lfirst_oid(cell);
          autovac_table *tab;
          bool        skipit;
          int            stdVacuumCostDelay;
--- 2196,2205 ----
      /*
       * Perform operations on collected tables.
       */
!     forboth(lco, table_oids, lcs, table_shares)
      {
!         Oid            relid = lfirst_oid(lco);
!         bool        relisshared = (bool) lfirst_int(lcs);
          autovac_table *tab;
          bool        skipit;
          int            stdVacuumCostDelay;
*************** do_autovacuum(void)
*** 2229,2243 ****
              if (worker == MyWorkerInfo)
                  continue;

!             /* ignore workers in other databases */
!             if (worker->wi_dboid != MyDatabaseId)
                  continue;

!             if (worker->wi_tableoid == relid)
!             {
!                 skipit = true;
!                 break;
!             }
          }
          LWLockRelease(AutovacuumLock);
          if (skipit)
--- 2245,2261 ----
              if (worker == MyWorkerInfo)
                  continue;

!             /* not a match if tableoid or shared-rel flag don't match */
!             if (worker->wi_tableoid != relid ||
!                 worker->wi_tableshared != relisshared)
                  continue;

!             /* for unshared table, not a match unless same database */
!             if (!relisshared && worker->wi_dboid != MyDatabaseId)
!                 continue;
!
!             skipit = true;
!             break;
          }
          LWLockRelease(AutovacuumLock);
          if (skipit)
*************** do_autovacuum(void)
*** 2271,2276 ****
--- 2289,2295 ----
           * the lock so that other workers don't vacuum it concurrently.
           */
          MyWorkerInfo->wi_tableoid = relid;
+         MyWorkerInfo->wi_tableshared = relisshared;
          LWLockRelease(AutovacuumScheduleLock);

          /*