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

From: "Pavan Deolasee" <pavan(dot)deolasee(at)gmail(dot)com>
>> Anyone else has an opinion on this?
>>
>>
> Since I did not hear anything on this, I created a patch that adds a new
> 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 reviewing this patch.

According to the approach of the patch, CREATE DATABASE also needs the new
flag to sync the unlogged tables before copying them from source database to
the new one. You need to check other places which do checkpointing.

I'm with Tom-san: I think it would be better for online checkpoints to sync
unlogged tables without a new flag. The reasons are:

* There's a greater danger of losing data during operating system restart.
For example, IIRC, Windows gives only 20 seconds to terminate all services
during OS shutdown. If many dirty buffers for unlogged tables linger in the
shared buffers, PostgreSQL service may fail to complete database shutdown.
Even if the online checkpoint writes out all dirty buffers, the possibility
of there being many dirty buffers at shutdown is not zero, but the
probability would be lower.

* As you missed the CREATE DATABASE case, we may fail to specify the new
flag when necessary in the future code.

* Unlogged tables are, as its name suggests, for better performance by not
writing WAL.

* If the online checkpoint leaves dirty buffers, it would be more probable
that the backends have to write them out when evicting the buffers. This
may also indicate some influence on how to interpret pg_stat_bgwriter.

I'm inclined to just make the online checkpoint write out all dirty buffers.
Could you consider this again? Let me mark this as waiting on authoer now.

Regards
MauMau