Date: Thu, 12 Dec 2019 20:26:32 +0100
From: David Fetter <david@fetter.org>
To: Mark Dilger <hornschnorter@gmail.com>
Cc: PostgreSQL Development <pgsql-hackers@postgresql.org>,
	Christophe Pettus <xof@thebuild.com>
Subject: Re: Make autovacuum sort tables in descending order of xid_age
Message-ID: <20191212192631.GA29201@fetter.org>
References: <20191129180138.GD8731@fetter.org>
 <20191129222152.GE8731@fetter.org>
 <2488acea-328b-1328-d437-8c156bc8f7a4@gmail.com>
 <20191130222314.GF8731@fetter.org>
 <877ac074-e95d-bdee-09e7-c51f0ee0fde2@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <877ac074-e95d-bdee-09e7-c51f0ee0fde2@gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

On Thu, Dec 12, 2019 at 08:02:25AM -0800, Mark Dilger wrote:
> On 11/30/19 2:23 PM, David Fetter wrote:
> > On Sat, Nov 30, 2019 at 10:04:07AM -0800, Mark Dilger wrote:
> > > On 11/29/19 2:21 PM, David Fetter wrote:
> > > > On Fri, Nov 29, 2019 at 07:01:39PM +0100, David Fetter wrote:
> > > > > Folks,
> > > > > 
> > > > > Per a suggestion Christophe made, please find attached a patch to
> > > > > $Subject:
> > > > > 
> > > > > Apart from carefully fudging with pg_resetwal, and short of running in
> > > > > production for a few weeks, what would be some good ways to test this?
> > > > 
> > > > Per discussion on IRC with Sehrope Sarkuni, please find attached a
> > > > patch with one fewer bug, this one in the repalloc() calls.
> > > 
> > > Hello David,
> > > 
> > > Here are my initial thoughts.
> > > 
> > > Although you appear to be tackling the problem of vacuuming tables
> > > with older Xids first *per database*,
> > 
> > Yes, that's what's come up for me in production, but lately,
> > production has consisted of a single active DB maxing out hardware. I
> > can see how in other situations--multi-tenant, especially--it would
> > make more sense to sort the DBs first.
> 
> I notice you don't address that in your latest patch.  Do you have
> any thoughts on whether that needs to be handled in this patch?

My thought is that it doesn't.

> > > I have not tested this change, but I may do so later today or perhaps
> > > on Monday.
> 
> The code compiles cleanly and passes all regression tests, but I don't
> think those tests really cover what you are changing.  Have you been
> using any test framework for this?

I don't have one :/

> I wonder if you might add information about table size, table changes,
> and bloat to your RelFrozenXidAge struct and modify rfxa_comparator to
> use a heuristic to cost the (age, size, bloat, changed) grouping and
> sort on that cost, such that really large bloated tables with old xids
> might get vacuumed before smaller, less bloated tables that have
> even older xids. Sorting the tables based purely on xid_age seems to
> ignore other factors that are worth considering.  I do not have a
> formula for how those four factors should be weighted in the heuristic,
> but you are implicitly assigning three of them a weight of zero in
> your current patch.

I think it's vastly premature to come up with complex sorting systems
right now.  Just sorting in descending order of age should either have
or not have positive effects.

> relation_needs_vacanalyze currently checks the reltuples, n_dead_tuples
> and changes_since_analyze along with vac_scale_factor and
> anl_scale_factor for the relation, but only returns booleans dovacuum,
> doanalyze, and wraparound.

Yeah, I looked at that. It's for a vastly different purpose, namely
deciding what's an emergency and what's probably not, but needs
attention anyhow.  My goal was something a little finer-grained and, I
hope, a little easier to establish the (lack of) benefits because only
one thing is getting changed.

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate