Re: BUG #10675: alter database set tablespace and unlogged table

Hi,

On 2014-07-02 17:28:25 +0530, Pavan Deolasee wrote:
> flag to tell checkpointer to flush all pages to the disk. Tom (and even I)
> have reservations about the approach, but I would nevertheless leave it to
> the committer to decide. IMV we must fix this bug one way or the other.
> Otherwise users face risk of failing to do clean shutdown.

I'm looking into this again now. We haven't really come to a agreement
about which approach to take. So unless there's some progress on that
front I'll push a variant of this patch. That seems better than leaving
the issue open for another round of releases.

Issues I've found with the patch so far:
* The RequestCheckpoint() in createdb() also needs CHECKPOINT_FLUSH_ALL.
* I don't think it's wise to renumber the CHECKPOINT_* flags in a commit
that's supposed to be backpatched. That'll cause interesting issues
if there's a extension out there containing a RequestCheckpoint()
call.
* I've also done some minor code and comment changes.

Attached is my new version. I've confirmed that I could reproduce
various broken behaviour before (wrong query results, ERRORs in further
queries trying to write out buffers, shutdown failures), but not after.

I'll note that I think there's arguably another bug that participated in
the shutdown/checkpoint error you reported in
http://archives.postgresql.org/message-id/CABOikdMxX0VdJEKSBd62sX_XAwa_%3DMAFqdAXXbU5V%2BBHZOxrng%40mail.gmail.com
When moving a database into a different tablespace we never invalidate it's
buffers even though they don't correspond to any existing files. The
only reason that currently works is because they're not dirty and thus
never written back or anything.

I think movedb() should also grow a DropDatabaseBuffers() call to fix
that.

Greetings,

Andres Freund

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