Horton

Re: [patch] BUG #15005: ANALYZE can make pg_class.reltuples inaccurate.

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Jeff Janes <jeff.janes@gmail.com>
Cc: David Gould <daveg@sonic.net>, Alvaro Herrera <alvherre@2ndquadrant.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Alina Alexeeva <alexeeva@adobe.com>, Ullas Lakkur Raghavendra <lakkurra@adobe.com>
Date: 2018-03-12T22:05:01Z
Lists: pgsql-hackers

Attachments

I wrote:
> Re-reading that thread, it seems like we should have applied Jeff's
> initial trivial patch[1] (to not hold AutovacuumScheduleLock across
> table_recheck_autovac) rather than waiting around for a super duper
> improvement to get agreed on.  I'm a bit tempted to go do that;
> if nothing else, it seems simple enough to back-patch, unlike most
> of the rest of what was discussed.

Jeff mentioned that that patch wasn't entirely trivial to rebase over
15739393e, and I now see why: in the code structure as it stands,
we don't know soon enough whether the table is shared.  In the
attached rebase, I solved that with the brute-force method of just
reading the pg_class tuple an extra time.  I think this is probably
good enough, really.  I thought about having do_autovacuum pass down
the tuple to table_recheck_autovac so as to not increase the net
number of syscache fetches, but I'm slightly worried about whether
we could be passing a stale pg_class tuple to table_recheck_autovac
if we do it like that.  OTOH, that just raises the question of why
we are doing any of this while holding no lock whatever on the target
table :-(.  I'm content to leave that question to the major redesign
that was speculated about in the bug #13750 thread.

This also corrects the inconsistency that at the bottom, do_autovacuum
clears wi_tableoid while taking AutovacuumLock, not AutovacuumScheduleLock
as is the documented lock for that field.  There's no actual bug there,
but cleaning this up might provide a slight improvement in concurrency
for operations that need AutovacuumLock but aren't looking at other
processes' wi_tableoid.  (Alternatively, we could decide that there's
no real need anymore for the separate AutovacuumScheduleLock, but that's
more churn than I wanted to deal with here.)

I think this is OK to commit/backpatch --- any objections?

			regards, tom lane

Commits

  1. Sync up our various ways of estimating pg_class.reltuples.

  2. Fix tuple counting in SP-GiST index build.

  3. Fix errors in contrib/bloom index build.

  4. When updating reltuples after ANALYZE, just extrapolate from our sample.

  5. Avoid holding AutovacuumScheduleLock while rechecking table statistics.