| From: | Noah Misch <noah(at)leadboat(dot)com> |
|---|---|
| To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
| Cc: | Stephen Frost <sfrost(at)snowman(dot)net>, Jeff Davis <pgsql(at)j-davis(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: Commits 8de72b and 5457a1 (COPY FREEZE) |
| Date: | 2012-12-11 03:01:08 |
| Message-ID: | 20121211030108.GB32120@tornado.leadboat.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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
| Attachment | Content-Type | Size |
|---|---|---|
| sinval-newrelfilenode-v1.patch | text/plain | 1011 bytes |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Karl O. Pinc | 2012-12-11 03:23:03 | Re: Multiple --table options for other commands |
| Previous Message | Karl O. Pinc | 2012-12-11 02:48:46 | Re: Doc patch, further describe and-mask nature of the permission system |