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

On Thu, Oct 9, 2014 at 10:52 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> 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.

+1 for applying this change.

- (flags & CHECKPOINT_CAUSE_TIME) ? " time" : "");
+ (flags & CHECKPOINT_CAUSE_TIME) ? " time" : "",
+ (flags & CHECKPOINT_FLUSH_ALL) ? " flush-all" :"");

ISTM that you forgot to add the following change.

- msg = "restartpoint starting:%s%s%s%s%s%s%s";
+ msg = "restartpoint starting:%s%s%s%s%s%s%s%s";
else
- msg = "checkpoint starting:%s%s%s%s%s%s%s";
+ msg = "checkpoint starting:%s%s%s%s%s%s%s%s";

Regards,

--
Fujii Masao