On Mon, Dec 10, 2012 at 08:04:55PM -0500, Robert Haas wrote:
> I think the current behavior, where we treat FREEZE as a hint, is just
> awful.  Regardless of whether the behavior is automatic or manually
> requested, the idea that you might get the optimization or not
> depending on the timing of relcache flushes seems very much
> undesirable.  I mean, if the optimization is actually important for
> performance, then you want to get it when you ask for it.  If it
> isn't, then why bother having it at all?  Let's say that COPY FREEZE
> normally doubles performance on a data load that therefore takes 8
> hours - somebody who suddenly loses that benefit because of a relcache
> flush that they can't prevent or control and ends up with a 16 hour
> data load is going to pop a gasket.

Until these threads, I did not know that a relcache invalidation could trip up
the WAL avoidance optimization, and now this.  I poked at the relevant
relcache.c code, and it already takes pains to preserve the needed facts.  The
header comment of RelationCacheInvalidate() indicates that entries bearing an
rd_newRelfilenodeSubid can safely survive the invalidation, but the code does
not implement that.  I think the comment is right, and this is just an
oversight in the code going back to its beginning (fba8113c).

I doubt the comment at the declaration of rd_createSubid in rel.h, though I
can't presently say what correct comment should replace it.  CLUSTER does
preserve the old value, at least for the main table relation.  CLUSTER
probably should *set* rd_newRelfilenodeSubid.

Thanks,
nm
*** a/src/backend/utils/cache/relcache.c
--- b/src/backend/utils/cache/relcache.c
***************
*** 2149,2156 **** RelationCacheInvalidate(void)
                /* Must close all smgr references to avoid leaving dangling 
ptrs */
                RelationCloseSmgr(relation);
  
!               /* Ignore new relations, since they are never cross-backend 
targets */
!               if (relation->rd_createSubid != InvalidSubTransactionId)
                        continue;
  
                relcacheInvalsReceived++;
--- 2149,2162 ----
                /* Must close all smgr references to avoid leaving dangling 
ptrs */
                RelationCloseSmgr(relation);
  
!               /*
!                * Ignore new relations; no other backend will manipulate them 
before
!                * we commit.  Likewise, before replacing a relation's 
relfilenode, we
!                * shall have acquired AccessExclusiveLock and drained any 
applicable
!                * pending invalidations.
!                */
!               if (relation->rd_createSubid != InvalidSubTransactionId ||
!                       relation->rd_newRelfilenodeSubid != 
InvalidSubTransactionId)
                        continue;
  
                relcacheInvalsReceived++;
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to