Re: [BUG?] lag of minRecoveryPont in archive recovery

On 10.12.2012 03:52, Kyotaro HORIGUCHI wrote:
>> I think that minRecoveryPoint should be updated before the data-file
>> changes, so XLogFlush() should be called before smgrtruncate(). No?
>
> Mmm.. As far as I saw in xact_redo_commit_internal, XLogFlush
> seems should be done AFTER redo substantial operation. Is it
> wrong? If so, I suppose xact_redo_commit_internal could shold be
> fixed in the same way.

So, two options:

1. Redo truncation, then XLogFlush()

There's a window where the original problem could still occur: The file 
is truncated, and you crash before XLogFlush() finishes.

2. XLogFlush(), then redo truncation.

If the truncation fails for some reason (disk failure?) and you have 
already updated minRecoveryPoint, you can not start up until you somehow 
fix the problem so that the truncation can succeed, and then finish 
recovery.

Both options have their merits. The window in 1. is very small, and in 
2., the probability that an unlink() or truncation fails is very small 
as well.

We've chosen 1. in xact_redo_commit_internal(), but I don't think that 
was a carefully made decision, it just imitates what happens in the 
corresponding non-redo commit path. In xact_redo_commit_internal(), it 
makes sense to do XLogFlush() afterwards, for CREATE DATABASE and CREATE 
TABLESPACE. Those create files, and if you e.g run out of disk space, or 
non-existent path, you don't want minRecoveryPoint to be updated yet. 
Otherwise you can no longer recover to the point just before the 
creation of those files. But in case of DROP DATABASE, you have the 
exact same situation as with truncation: if you have already deleted 
some files, you must not be able to stop recovery at a point before that.

So I'd say we should update minRecoveryPoint first, then 
truncate/delete. But we should still keep the XLogFlush() at the end of 
xact_redo_commit_internal(), for the case where files/directories are 
created. Patch attached.

- Heikki