On Tue, Nov 4, 2014 at 11:45 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I wrote:
>> However: at no point in this sequence did we flush the original catalog
>> blocks out of shared buffers. Therefore, a read of the database's
>> pg_class or pg_type at this point is likely to find the previous states of
>> those pages (pre-composite-type-drop) as valid, matching entries, so that
>> we skip reading the up-to-date page contents back from disk.
>
>> A quick fix for this would be to issue DropDatabaseBuffers during
>> movedb()
>
> BTW, I wondered whether the exact same hazard didn't exist for ALTER TABLE
> SET TABLESPACE. At first glance it looks bad, because ATExecSetTableSpace
> doesn't forcibly drop old shared buffers for the moved table either.
> Closer examination says we're safe though:
>
> * The buffers will be dropped during smgrdounlink at end of transaction,
> so you could only be at risk if you moved a table, modified it, and moved
> it back in a single transaction.
>
> * A new relfilenode will be assigned during each movement.
>
> * When assigning a relfilenode during the move-back, we'd be certain to
> choose a relfilenode different from the original one, because the old
> relation still exists on-disk at this point.
>
> So it's safe as things stand; but this seems a bit, um, rickety. Should
> we insert a DropRelFileNodesAllBuffers call into ATExecSetTableSpace to
> make it safer? It's kind of annoying to have to scan the buffer pool
> twice, but I'm afraid that sometime in the future somebody will try to get
> clever about optimizing away the end-of-transaction buffer pool scan, and
> break this case. Or maybe I'm just worrying too much.
I think you're worrying too much. Adding a comment to the code that
drives the end-of-transaction buffer pool scan to warn future
optimizers not to be too clever might be warranted; adding run-time
overhead to avoid a bug we don't have seems excessive.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
