Re: [GENERAL] pgstattuple triggered checkpoint failure and database outage?

[Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>]

Tom Lane wrote:
> Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
> > Tom Lane wrote:
> > > A quick look at contrib/pgstattuple shows that it makes no effort
> > > whatsoever to avoid reading temp tables belonging to other sessions.
>
> > contrib/pageinspect has the same bug. Not surprising as it was largely 
> > inspired by pgstattuple.
>
> Given the seriousness of the consequences (forced database shutdown is
> no fun), I wonder whether we should install some low-level defense
> against this type of problem; ie teach ReadBuffer to throw error if
> asked to read a block from someone else's temp table.

That would be nice.

> This isn't entirely trivial because it's presently expensive to
> determine whether a table is someone else's temp table: it takes a
> system catalog lookup.   I'm not even sure that it'd be safe to have
> the relcache do it and cache the result --- it could lead to infinite
> recursion.  (At the very least this would promote pg_namespace into
> the set of critical relcache entries.)

You could hard code that PG_CATALOG_NAMESPACE is not a temp namespace. I 
believe that would stop the recursion. Would that avoid promoting 
pg_namespace to critical status, too?

> The solution that seems most practical to me is to add a bool column
> to pg_class indicating "this is a temp table".  Then, if that flag
> is set but it's not our own temp table (which we can tell easily),
> refuse to read.  However, a patch of that size would take a little
> while to develop, and I'm not entirely sure it's worth the trouble.
> I can't remember having seen bugs of this type before.

In addition to the one Alvaro mentioned, I recall having problems with 
this when working on the patch to allow temporary file access with two 
phase commit in autumn.

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com