Обсуждение: [COMMITTERS] pgsql: Avoid having vacuum set reltuples to 0 on non-empty relationsin

Avoid having vacuum set reltuples to 0 on non-empty relations in the
presence of page pins, which leads to serious estimation errors in the
planner.  This particularly affects small heavily-accessed tables,
especially where locking (e.g. from FK constraints) forces frequent
vacuums for mxid cleanup.

Fix by keeping separate track of pages whose live tuples were actually
counted vs. pages that were only scanned for freezing purposes.  Thus,
reltuples can only be set to 0 if all pages of the relation were
actually counted.

Backpatch to all supported versions.

Per bug #14057 from Nicolas Baccelli, analyzed by me.

Discussion: https://postgr.es/m/20160331103739.8956.94469@wrigleys.postgresql.org

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/1914c5ea7daaaaba4420f65c991256af5d4a9813

Modified Files
--------------
src/backend/commands/vacuumlazy.c                | 15 ++++--
src/test/isolation/expected/vacuum-reltuples.out | 62 ++++++++++++++++++++++++
src/test/isolation/isolation_schedule            |  1 +
src/test/isolation/specs/vacuum-reltuples.spec   | 45 +++++++++++++++++
4 files changed, 119 insertions(+), 4 deletions(-)

Andrew Gierth <rhodiumtoad@postgresql.org> writes:
> Avoid having vacuum set reltuples to 0 on non-empty relations in the
> presence of page pins, which leads to serious estimation errors in the
> planner.

Hm, buildfarm results suggest this test is not entirely stable:

https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2017-03-17%2000%3A47%3A57

I have not looked very closely, but I'm suspicious that the test case
depends on no autovacuum transactions running concurrently with it.
Disabling autovac on the table itself is not enough to control whether
global xmin is being held back by some other autovac transaction
somewhere.

            regards, tom lane

On 2017-03-16 23:37:06 -0400, Tom Lane wrote:
> Andrew Gierth <rhodiumtoad@postgresql.org> writes:
> > Avoid having vacuum set reltuples to 0 on non-empty relations in the
> > presence of page pins, which leads to serious estimation errors in the
> > planner.
>
> Hm, buildfarm results suggest this test is not entirely stable:
>
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2017-03-17%2000%3A47%3A57
>
> I have not looked very closely, but I'm suspicious that the test case
> depends on no autovacuum transactions running concurrently with it.
> Disabling autovac on the table itself is not enough to control whether
> global xmin is being held back by some other autovac transaction
> somewhere.

Yea, brought it up on IRC too - he's gone for the night (Europe).  I
suspect changing things > might do the trick.

- Andres

>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:

 Tom> Hm, buildfarm results suggest this test is not entirely stable:

I see it and will work on it.

 Tom> I have not looked very closely, but I'm suspicious that the test
 Tom> case depends on no autovacuum transactions running concurrently
 Tom> with it.  Disabling autovac on the table itself is not enough to
 Tom> control whether global xmin is being held back by some other
 Tom> autovac transaction somewhere.

Aren't vacuum transactions ignored for that? vacuum_set_xid_limits is
certainly passing ignoreVacuum=true to GetOldestXmin.

--
Andrew.

>>>>> "Andrew" == Andrew Gierth <andrew@tao11.riddles.org.uk> writes:

 Tom> I have not looked very closely, but I'm suspicious that the test
 Tom> case depends on no autovacuum transactions running concurrently
 Tom> with it.  Disabling autovac on the table itself is not enough to
 Tom> control whether global xmin is being held back by some other
 Tom> autovac transaction somewhere.

 Andrew> Aren't vacuum transactions ignored for that?
 Andrew> vacuum_set_xid_limits is certainly passing ignoreVacuum=true to
 Andrew> GetOldestXmin.

Answering my own question: vacuum transactions are ignored but analyze
transactions aren't, and autovacuum will happily kick off an autoanalyze
of pg_attribute during the test.

Simplest fix seems to just be to remove the delete statement; I've
verified that the problem can be reproduced without it, and that should
make the result independent of OldestXmin. I'll commit that shortly
unless I hear to the contrary.

--
Andrew.