Re: ALTER TABLE lock strength reduction patch is unsafe

From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Simon Riggs <simon(at)2ndquadrant(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Noah Misch <noah(at)leadboat(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: ALTER TABLE lock strength reduction patch is unsafe
Date: 2013-07-15 14:32:10
Message-ID: 20130715143210.GA31361@awork2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2013-07-15 10:06:31 -0400, Robert Haas wrote:
> Noah discovered an interesting one recently: apparently, the relcache
> machinery has some logic in it that depends on the use of
> AccessExclusiveLock in subtle ways. I'm going to attempt to explain
> it, and maybe he can jump in and explain it better. Essentially, the
> problem is that when a relcache reload occurs, certain data structures
> (like the tuple descriptor, but there are others) are compared against
> the old version of the same data structure. If there are no changes,
> we do nothing; else, we free the old one and install the new one. The
> reason why we don't free the old one and install the new one
> unconditionally is because other parts of the backend might have
> pointers to the old data structure, so just replacing it all the time
> would result in crashes. Since DDL requires AccessExclusiveLock +
> CheckTableNotInUse(), any actual change to the data structure will
> happen when we haven't got any pointers anyway.

Aren't we swapping in the new data on a data level for that reason? See
RelationClearRelation().

> A second thing I'm concerned about is that, although MVCC catalog
> scans guarantee that we won't miss a concurrently-updated row
> entirely, or see a duplicate, they don't guarantee that the rows we
> see during a scan of catalog A will be consistent with the rows we see
> during a scan of catalog B moments later. For example, hilarity will
> ensue if relnatts doesn't match what we see in pg_attribute. Now I
> don't think this particular example matters very much because I think
> there are probably lots of other things that would also break if we
> try to add a column without a full table lock, so we're probably
> doomed there anyway. But there might be other instances of this
> problem that are more subtle.

Hm. Other transactions basically should be protected against this
because they can't se uncommitted data anyway, right? ISTM that our own
session basically already has be safe against hilarity of this kind,
right?

I am concerned about stuff like violating constraints because we haven't
yet seen the new constraint definition and the like... Or generating
wrong plans because we haven't seen that somebody has dropped a
constraint and inserted data violating the old one.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2013-07-15 14:53:02 Re: vacuumlo - use a cursor
Previous Message Robins Tharakan 2013-07-15 14:23:28 Re: Add regression tests for ROLE (USER)