Thread: Tweaking the planner's heuristics for small/empty tables

Tweaking the planner's heuristics for small/empty tables

From
Tom Lane
Date:
There's a thread over in pgsql-performance
http://archives.postgresql.org/pgsql-performance/2011-07/msg00046.php
in which the main conclusion was that we need to take a fresh look at the
heuristics the planner uses when dealing with small or empty relations.
The code in question is in estimate_rel_size() in plancat.c:
           curpages = RelationGetNumberOfBlocks(rel);
           /*            * HACK: if the relation has never yet been vacuumed, use a            * minimum estimate of 10
pages. This emulates a desirable aspect            * of pre-8.0 behavior, which is that we wouldn't assume a newly
     * created relation is really small, which saves us from making            * really bad plans during initial data
loading. (The plans are            * not wrong when they are made, but if they are cached and used            * again
afterthe table has grown a lot, they are bad.) It would            * be better to force replanning if the table size
haschanged a            * lot since the plan was made ... but we don't currently have any            * infrastructure
forredoing cached plans at all, so we have to            * kluge things here instead.            *            * We
approximate"never vacuumed" by "has relpages = 0", which            * means this will also fire on genuinely empty
relations.   Not            * great, but fortunately that's a seldom-seen case in the real            * world, and it
shouldn'tdegrade the quality of the plan too            * much anyway to err in this direction.            */
if(curpages < 10 && rel->rd_rel->relpages == 0)               curpages = 10;
 

That comment is of 8.0 vintage, and it needs to be updated, because it's
now the case that there *is* an automatic path for refreshing plans when
table sizes change.  Once the number of updates exceeds the auto-analyze
threshold, autovac will run an ANALYZE, which will update the relation's
pg_class row, which will force a relcache inval, which will cause the
plancache.c code to mark any cached plans using the relation as needing
to be rebuilt.

So that raises the question of whether we shouldn't just drop the
if-statement entirely.  I experimented with that a bit, and soon found
that it resulted in some probably-undesirable changes in the regression
test results.  In particular the planner seemed to be switching from
indexscan to seqscan plans for accesses to very small tables, which may
not be a good tradeoff.  I'm a bit loath to twiddle the behavior here
without extensive testing, since for the most part we've not had many
complaints about the planner's behavior for small tables.

Another reason not to rely completely on the auto-analyze update path is
that it doesn't work for temp tables, since autovac can't access another
session's temp tables.  It's also worth noting that auto-analyze will
never kick in at all for tables of less than about 60 rows, unless there
is update/delete traffic on them.

The issue that came up in pgsql-performance was that this if-statement
was firing for an empty inheritance parent table, causing a scan on the
parent to look significantly more expensive than it really was, and in
fact a good bit more expensive than the actually-useful index probe on
its child table.  This bogus estimate thus discouraged the planner from
using a nestloop with the partitioned table on the inside, which in
reality was the most appropriate plan.  So we could tackle that issue
with a pretty narrowly-focused test: disable the if-statement for
inheritance parent tables, a la
           if (curpages < 10 &&               rel->rd_rel->relpages == 0 &&               !rel->rd_rel->relhassubclass)
             curpages = 10;
 

This is justifiable on the grounds that an inheritance parent table is
much more likely to be meant to stay empty than an ordinary table.

Another thing that struck me while looking at the code is that the
curpages clamp is applied to indexes too, which seems like a thinko.
A table occupying a few pages wouldn't likely have an index as big as
the table itself is.

So what I'm currently thinking about is a change like this:
           if (curpages < 10 &&               rel->rd_rel->relpages == 0 &&               !rel->rd_rel->relhassubclass
&&              rel->rd_rel->relkind != RELKIND_INDEX)               curpages = 10;
 

plus a suitable rewrite of the comment.  This seems like a safe enough
change to apply to 9.1.  Going forward we might want to change it more,
but I think it'd require some real-world testing.

Thoughts?
        regards, tom lane


Re: Tweaking the planner's heuristics for small/empty tables

From
"Kevin Grittner"
Date:
Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Another thing that struck me while looking at the code is that the
> curpages clamp is applied to indexes too, which seems like a
> thinko. A table occupying a few pages wouldn't likely have an
> index as big as the table itself is.
But not zero pages, either.
> So what I'm currently thinking about is a change like this:
>
>             if (curpages < 10 &&
>                 rel->rd_rel->relpages == 0 &&
>                 !rel->rd_rel->relhassubclass &&
>                 rel->rd_rel->relkind != RELKIND_INDEX)
>                 curpages = 10;
Rather than assume ten heap pages and zero index pages, how about
something like:   if (curpages < 10 &&       rel->rd_rel->relpages == 0 &&       !rel->rd_rel->relhassubclass)
curpages= (rel->rd_rel->relkind == RELKIND_INDEX) ? 1 : 10;
 
> This seems like a safe enough change to apply to 9.1.  Going
> forward we might want to change it more, but I think it'd require
> some real-world testing.
I'd be a little nervous about a change which put indexes all the way
to zero without serious testing; otherwise, agreed.
-Kevin


Re: Tweaking the planner's heuristics for small/empty tables

From
Tom Lane
Date:
"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Another thing that struck me while looking at the code is that the
>> curpages clamp is applied to indexes too, which seems like a
>> thinko. A table occupying a few pages wouldn't likely have an
>> index as big as the table itself is.
> But not zero pages, either.

Huh?  I think you misread the test.  Keep in mind that the starting
value of curpages is the actual current size of the relation (as
reported by lseek(SEEK_END)).  The clamp action is triggering if
pg_class.relpages is zero, indicating (approximately) that we've never
yet run vacuum on the relation.

BTW, in some quick testing it seems like a newly-created index will
start out with relpages nonzero anyway, making the point moot; so adding
a relkind test here is really more documentation than anything else.
        regards, tom lane


Re: Tweaking the planner's heuristics for small/empty tables

From
Bruce Momjian
Date:
Tom Lane wrote:
> Another reason not to rely completely on the auto-analyze update path is
> that it doesn't work for temp tables, since autovac can't access another
> session's temp tables.  It's also worth noting that auto-analyze will
> never kick in at all for tables of less than about 60 rows, unless there
> is update/delete traffic on them.

We could exclude temp tables in the 'if' test.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + It's impossible for everything to be true. +


Re: Tweaking the planner's heuristics for small/empty tables

From
Tom Lane
Date:
Bruce Momjian <bruce@momjian.us> writes:
> Tom Lane wrote:
>> Another reason not to rely completely on the auto-analyze update path is
>> that it doesn't work for temp tables, since autovac can't access another
>> session's temp tables.  It's also worth noting that auto-analyze will
>> never kick in at all for tables of less than about 60 rows, unless there
>> is update/delete traffic on them.

> We could exclude temp tables in the 'if' test.

I considered whether to do things differently depending on the table's
temp status, but it's not clear what we could do better from knowing that.
If it's temp, it's more likely not less likely that we need some sort of
hack because vacuum/analyze won't come along to help.
        regards, tom lane