On Wed, Feb 21, 2018 at 07:20:00AM +0000, Tsunakawa, Takayuki wrote:
> Right. Then I thought of doing the following to avoid making a new
> function only for RemoveNonParentXlogFiles() which is similar to
> RemoveXlogFile().
>
> * Add an argument "bool durable" to RemoveXlogFile(). Based on its
> value, RemoveXlogFile() calls either durable_xx() or xx().
>
> * Likewise, add an argument "bool durable" to InstallXLogFileSegment()
> and do the same.
>
> * RemoveNonParentXlogFiles() calls RemoveXlogFile() with
> durable=false. At the end of the function, sync the pg_wal directory
> with fsync_fname().
>
> * RemoveOldXlogFiles() does the same thing. One difference is that it
> passes false to RemoveXlogFile() during recovery (InRecovery == true)
> and true otherwise.
It seems to me that you would reintroduce partially the problems that
1d4a0ab1 has fixed. In short, if a crash happens in the code paths
calling RemoveXlogFile with durable = false before fsync'ing pg_wal,
then a rename has no guarantee to be durable, so you could finish again
with a file that as an old name, but new contents. A crucial thing
which matters for a rename to be durable is that the old file is sync'ed
as well.
--
Michael