Thread

  1. Re: Make relation_openrv atomic wrt DDL

    Robert Haas <robertmhaas@gmail.com> — 2011-07-07T15:43:30Z

    On Wed, Jul 6, 2011 at 10:44 PM, Noah Misch <noah@2ndquadrant.com> wrote:
    > Attached.  I made the counter 64 bits wide, handled the nothing-found case per
    > your idea, and improved a few comments cosmetically.  I have not attempted to
    > improve the search_path interposition case.  We can recommend the workaround
    > above, and doing better looks like an excursion much larger than the one
    > represented by this patch.
    
    I looked at this some more and started to get uncomfortable with the
    whole idea of having RangeVarLockRelid() be a wrapper around
    RangeVarGetRelid().  This hazard exists everywhere the latter function
    gets called, not just in relation_open().  So it doesn't seem right to
    fix the problem only in those places.
    
    So I went through and incorporated the logic proposed for
    RangeVarLockRelid() into RangeVarGetRelid() itself, and then went
    through and examined all the callers of RangeVarGetRelid().  There are
    some, such as has_table_privilege(), where it's really impractical to
    take any lock, first because we might have no privileges at all on
    that table and second because that could easily lead to a massive
    amount of locking for no particular good reason.  I believe Tom
    suggested that the right fix for these functions is to have them
    index-scan the system catalogs using the caller's MVCC snapshot, which
    would be right at least for pg_dump.  And there are other callers that
    cannot acquire the lock as part of RangeVarGetRelid() for a variety of
    other reasons.  However, having said that, there do appear to be a
    number of cases that are can be fixed fairly easily.
    
    So here's a (heavily) updated patch that tries to do that, along with
    adding comments to the places where things still need more fixing.  In
    addition to the problems corrected by your last version, this fixes
    LOCK TABLE, ALTER SEQUENCE, ALTER TABLE .. RENAME, the whole-table
    variant of REINDEX, CREATE CONSTRAINT TRIGGER (which is flat-out wrong
    as it stands, since it acquires *no lock at all* on the table
    specified in the FROM clause, never mind the question of doing so
    atomically), CREATE RULE, and (partially) DROP TRIGGER and DROP RULE.
    
    Regardless of exactly how we decide to proceed here, it strikes me
    that there is a heck of a lot more work that could stand to be done in
    this area...  :-(
    
    -- 
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company