From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
cc: =?utf-8?B?T25kxZllaiBTdsSbdGzDrWs=?= <osvetlik@flexibee.eu>,
        pgsql-admin@postgresql.org
Subject: Re: Autovacuum of pg_database
In-reply-to: <20160505154341.GA177987@alvherre.pgsql>
References: <20160505154341.GA177987@alvherre.pgsql>
Comments: In-reply-to Alvaro Herrera <alvherre@2ndquadrant.com>
	message dated "Thu, 05 May 2016 12:43:41 -0300"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <13467.1462544192.0@sss.pgh.pa.us>
Date: Fri, 06 May 2016 10:17:04 -0400
Message-ID: <13485.1462544224@sss.pgh.pa.us>
Precedence: bulk
Sender: pgsql-admin-owner@postgresql.org

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <13467.1462544192.1@sss.pgh.pa.us>

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


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="autovac-shared-table-handling.patch";
	charset="us-ascii"
Content-ID: <13467.1462544192.2@sss.pgh.pa.us>
Content-Description: autovac-shared-table-handling.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/a=
utovacuum.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 whi=
ch is
!  * protected by AutovacuumScheduleLock (which is read-only for everyone e=
xcept
!  * 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 ****
  		}
=20=20
  		if (worker->wi_proc !=3D NULL)
! 			elog(DEBUG2, "autovac_balance_cost(pid=3D%u db=3D%u, rel=3D%u, dobalan=
ce=3D%s cost_limit=3D%d, cost_limit_base=3D%d, cost_delay=3D%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 ----
  		}
=20=20
  		if (worker->wi_proc !=3D NULL)
! 			elog(DEBUG2, "autovac_balance_cost(pid=3D%u db=3D%u, rel=3D%u%s, dobal=
ance=3D%s cost_limit=3D%d, cost_limit_base=3D%d, cost_delay=3D%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 =3D 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 =3D NIL;
+ 	List	   *table_shares =3D 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;
=20=20
  		relid =3D HeapTupleGetOid(tuple);
=20=20
  		/* Fetch reloptions and the pgstat entry for this table */
  		relopts =3D extract_autovac_opts(tuple, pg_class_desc);
! 		tabentry =3D get_pgstat_tabentry_relid(relid, classForm->relisshared,
  											 shared, dbentry);
=20=20
  		/* Check if it needs vacuum or analyze */
--- 2019,2029 ----
  			continue;
=20=20
  		relid =3D HeapTupleGetOid(tuple);
+ 		relisshared =3D classForm->relisshared;
=20=20
  		/* Fetch reloptions and the pgstat entry for this table */
  		relopts =3D extract_autovac_opts(tuple, pg_class_desc);
! 		tabentry =3D get_pgstat_tabentry_relid(relid, relisshared,
  											 shared, dbentry);
=20=20
  		/* 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 =3D lappend_oid(table_oids, relid);
=20=20
  			/*
  			 * 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 =3D lappend_oid(table_oids, relid);
+ 				table_shares =3D lappend_int(table_shares, relisshared);
+ 			}
=20=20
  			/*
  			 * Remember the association for the second pass.  Note: we must do
*************** do_autovacuum(void)
*** 2117,2122 ****
--- 2127,2133 ----
  		Form_pg_class classForm =3D (Form_pg_class) GETSTRUCT(tuple);
  		PgStat_StatTabEntry *tabentry;
  		Oid			relid;
+ 		bool		relisshared;
  		AutoVacOpts *relopts =3D NULL;
  		bool		dovacuum;
  		bool		doanalyze;
*************** do_autovacuum(void)
*** 2129,2134 ****
--- 2140,2146 ----
  			continue;
=20=20
  		relid =3D HeapTupleGetOid(tuple);
+ 		relisshared =3D classForm->relisshared;
=20=20
  		/*
  		 * fetch reloptions -- if this toast table does not have them, try the
*************** do_autovacuum(void)
*** 2146,2152 ****
  		}
=20=20
  		/* Fetch the pgstat entry for this table */
! 		tabentry =3D get_pgstat_tabentry_relid(relid, classForm->relisshared,
  											 shared, dbentry);
=20=20
  		relation_needs_vacanalyze(relid, relopts, classForm, tabentry,
--- 2158,2164 ----
  		}
=20=20
  		/* Fetch the pgstat entry for this table */
! 		tabentry =3D get_pgstat_tabentry_relid(relid, relisshared,
  											 shared, dbentry);
=20=20
  		relation_needs_vacanalyze(relid, relopts, classForm, tabentry,
*************** do_autovacuum(void)
*** 2155,2161 ****
--- 2167,2176 ----
=20=20
  		/* ignore analyze for toast tables */
  		if (dovacuum)
+ 		{
  			table_oids =3D lappend_oid(table_oids, relid);
+ 			table_shares =3D lappend_int(table_shares, relisshared);
+ 		}
  	}
=20=20
  	heap_endscan(relScan);
*************** do_autovacuum(void)
*** 2181,2189 ****
  	/*
  	 * Perform operations on collected tables.
  	 */
! 	foreach(cell, table_oids)
  	{
! 		Oid			relid =3D 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 =3D lfirst_oid(lco);
! 		bool		relisshared =3D (bool) lfirst_int(lcs);
  		autovac_table *tab;
  		bool		skipit;
  		int			stdVacuumCostDelay;
*************** do_autovacuum(void)
*** 2229,2243 ****
  			if (worker =3D=3D MyWorkerInfo)
  				continue;
=20=20
! 			/* ignore workers in other databases */
! 			if (worker->wi_dboid !=3D MyDatabaseId)
  				continue;
=20=20
! 			if (worker->wi_tableoid =3D=3D relid)
! 			{
! 				skipit =3D true;
! 				break;
! 			}
  		}
  		LWLockRelease(AutovacuumLock);
  		if (skipit)
--- 2245,2261 ----
  			if (worker =3D=3D MyWorkerInfo)
  				continue;
=20=20
! 			/* not a match if tableoid or shared-rel flag don't match */
! 			if (worker->wi_tableoid !=3D relid ||
! 				worker->wi_tableshared !=3D relisshared)
  				continue;
=20=20
! 			/* for unshared table, not a match unless same database */
! 			if (!relisshared && worker->wi_dboid !=3D MyDatabaseId)
! 				continue;
!=20
! 			skipit =3D 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 =3D relid;
+ 		MyWorkerInfo->wi_tableshared =3D relisshared;
  		LWLockRelease(AutovacuumScheduleLock);
=20=20
  		/*

------- =_aaaaaaaaaa0
Content-Type: text/plain
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0


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

------- =_aaaaaaaaaa0--