Date: Thu, 2 Oct 2014 10:54:13 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Mark Kirkwood <mark.kirkwood@catalyst.net.nz>,
 Amit Kapila <amit.kapila16@gmail.com>,
 Haribabu Kommi <kommi.haribabu@gmail.com>,
 "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: Per table autovacuum vacuum cost limit behaviour
 strange
Message-ID: <20141002135412.GX5311@eldon.alvh.no-ip.org>
References: 
 <CAJrrPGdK=nhTHUCsURgXQyG6RH=rJ8b4CdYhzGhwH=UuzaO9Ng@mail.gmail.com>
 <CAA4eK1JALkZOLnGdpqQgKbb+vq0eri1b7ytDfZLA8E9ZM35NHw@mail.gmail.com>
 <CAJrrPGeEwsezwA6+DpA2Y7MBJmEnNWwD5Gi4Ve_EqCxHoyHj3Q@mail.gmail.com>
 <CAA4eK1LwFs9iM-rUdfOrZE5MvjSxMM9ZbPbzv1SeWgGYu-O=VQ@mail.gmail.com>
 <53672F63.3080400@catalyst.net.nz>
 <CAA4eK1KDAUP=g59ku_0ekDoRjaGfEamyadQn5rJVt3j5oErLXg@mail.gmail.com>
 <53686F22.8030300@catalyst.net.nz>
 <20140826161911.GE6343@eldon.alvh.no-ip.org>
 <CA+Tgmoav37msTP0pVsrEYz4LBYU6SETMZGYTgPo5AEOC0yCMbA@mail.gmail.com>
 <20140930221644.GR5311@eldon.alvh.no-ip.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="xs+9IvWevLaxKUtW"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20140930221644.GR5311@eldon.alvh.no-ip.org>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org


--xs+9IvWevLaxKUtW
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

Alvaro Herrera wrote:

> So in essence what we're going to do is that the balance mechanism
> considers only tables that don't have per-table configuration options;
> for those that do, we will use the values configured there without any
> changes.
> 
> I'll see about implementing this and making sure it finds its way to
> 9.4beta3.

Here's a patch that makes it work as proposed.

How do people feel about back-patching this?  On one hand it seems
there's a lot of fear of changing autovacuum behavior in back branches,
because for many production systems it has carefully been tuned; on the
other hand, it seems hard to believe that anyone has tuned the system to
work sanely given how insanely per-table options behave in the current
code.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--xs+9IvWevLaxKUtW
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="per_table_vacuum_para_v5.patch"

*** a/doc/src/sgml/maintenance.sgml
--- b/doc/src/sgml/maintenance.sgml
***************
*** 792,801 **** analyze threshold = analyze base threshold + analyze scale factor * number of tu
     </para>
  
     <para>
!     When multiple workers are running, the cost limit is
      <quote>balanced</quote> among all the running workers, so that the
!     total impact on the system is the same, regardless of the number
!     of workers actually running.
     </para>
    </sect2>
   </sect1>
--- 792,804 ----
     </para>
  
     <para>
!     When multiple workers are running, the cost delay parameters are
      <quote>balanced</quote> among all the running workers, so that the
!     total I/O impact on the system is the same regardless of the number
!     of workers actually running.  However, any workers processing tables whose
!     <literal>autovacuum_vacuum_cost_delay</> or
!     <literal>autovacuum_vacuum_cost_limit</> have been set are not considered
!     in the balancing algorithm.
     </para>
    </sect2>
   </sect1>
*** a/src/backend/postmaster/autovacuum.c
--- b/src/backend/postmaster/autovacuum.c
***************
*** 193,198 **** typedef struct autovac_table
--- 193,199 ----
  	int			at_multixact_freeze_table_age;
  	int			at_vacuum_cost_delay;
  	int			at_vacuum_cost_limit;
+ 	bool		at_dobalance;
  	bool		at_wraparound;
  	char	   *at_relname;
  	char	   *at_nspname;
***************
*** 223,228 **** typedef struct WorkerInfoData
--- 224,230 ----
  	Oid			wi_tableoid;
  	PGPROC	   *wi_proc;
  	TimestampTz wi_launchtime;
+ 	bool		wi_dobalance;
  	int			wi_cost_delay;
  	int			wi_cost_limit;
  	int			wi_cost_limit_base;
***************
*** 1716,1721 **** FreeWorkerInfo(int code, Datum arg)
--- 1718,1724 ----
  		MyWorkerInfo->wi_tableoid = InvalidOid;
  		MyWorkerInfo->wi_proc = NULL;
  		MyWorkerInfo->wi_launchtime = 0;
+ 		MyWorkerInfo->wi_dobalance = false;
  		MyWorkerInfo->wi_cost_delay = 0;
  		MyWorkerInfo->wi_cost_limit = 0;
  		MyWorkerInfo->wi_cost_limit_base = 0;
***************
*** 1776,1788 **** autovac_balance_cost(void)
  	if (vac_cost_limit <= 0 || vac_cost_delay <= 0)
  		return;
  
! 	/* caculate the total base cost limit of active workers */
  	cost_total = 0.0;
  	dlist_foreach(iter, &AutoVacuumShmem->av_runningWorkers)
  	{
  		WorkerInfo	worker = dlist_container(WorkerInfoData, wi_links, iter.cur);
  
  		if (worker->wi_proc != NULL &&
  			worker->wi_cost_limit_base > 0 && worker->wi_cost_delay > 0)
  			cost_total +=
  				(double) worker->wi_cost_limit_base / worker->wi_cost_delay;
--- 1779,1792 ----
  	if (vac_cost_limit <= 0 || vac_cost_delay <= 0)
  		return;
  
! 	/* calculate the total base cost limit of participating active workers */
  	cost_total = 0.0;
  	dlist_foreach(iter, &AutoVacuumShmem->av_runningWorkers)
  	{
  		WorkerInfo	worker = dlist_container(WorkerInfoData, wi_links, iter.cur);
  
  		if (worker->wi_proc != NULL &&
+ 			worker->wi_dobalance &&
  			worker->wi_cost_limit_base > 0 && worker->wi_cost_delay > 0)
  			cost_total +=
  				(double) worker->wi_cost_limit_base / worker->wi_cost_delay;
***************
*** 1801,1806 **** autovac_balance_cost(void)
--- 1805,1811 ----
  		WorkerInfo	worker = dlist_container(WorkerInfoData, wi_links, iter.cur);
  
  		if (worker->wi_proc != NULL &&
+ 			worker->wi_dobalance &&
  			worker->wi_cost_limit_base > 0 && worker->wi_cost_delay > 0)
  		{
  			int			limit = (int)
***************
*** 2285,2290 **** do_autovacuum(void)
--- 2290,2296 ----
  
  		/* advertise my cost delay parameters for the balancing algorithm */
  		MyWorkerInfo->wi_cost_delay = tab->at_vacuum_cost_delay;
+ 		MyWorkerInfo->wi_dobalance = tab->at_dobalance;
  		MyWorkerInfo->wi_cost_limit = tab->at_vacuum_cost_limit;
  		MyWorkerInfo->wi_cost_limit_base = tab->at_vacuum_cost_limit;
  
***************
*** 2579,2584 **** table_recheck_autovac(Oid relid, HTAB *table_toast_map,
--- 2585,2598 ----
  		tab->at_relname = NULL;
  		tab->at_nspname = NULL;
  		tab->at_datname = NULL;
+ 
+ 		/*
+ 		 * If any of the cost delay parameters has been set individually for
+ 		 * this table, disable the balancing algorithm.
+ 		 */
+ 		tab->at_dobalance =
+ 			!(avopts && (avopts->vacuum_cost_limit > 0 ||
+ 						 avopts->vacuum_cost_delay > 0));
  	}
  
  	heap_freetuple(classTup);

--xs+9IvWevLaxKUtW
Content-Type: text/plain
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0


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

--xs+9IvWevLaxKUtW--