PBM-1329 Write to temporary file name and sync before renaming

[DanielOliverRJ]

Whilst trying to understand historic cases where we have seen BSON deserialisation errors, I realised it was possible for filesystem writes to be incomplete, but be unable to determine the fact. In particular, once an oplog file exists on the filesystem, it is assumed to be complete even though the write may have been interrupted.

This change performs all filesystem writes to a temporary file in the same directory and does a rename once the sync has returned.

In the event of a failure, calling Close() on the same file handle does not cause an error because the deferred call discards the error.

I do not believe non-filesystem storage are affected by this problem, as the providers have other checks in place to ensure the upload completed.

[it-percona-cla]

All committers have signed the CLA.

[defbin]

hi @DanielOliverRJ. thank you for your contributions. Could you please try to review/sign the CLA again so we can proceed?

[DanielOliverRJ]

Sorry, I had to get the CLA text approved before I could sign. This is now completed.

[defbin]

Hi @DanielOliverRJ

Thank you for your contribution.

I see one problem here:

percona-backup-mongodb/pbm/storage/fs/fs.go

Lines 70 to 101 in fd2c57a

	err := os.MkdirAll(path.Dir(filepath), os.ModeDir|0o755)
	if err != nil {
	return errors.Wrapf(err, "create path %s", path.Dir(filepath))
	}
	fw, err := os.Create(filepath)
	if err != nil {
	return errors.Wrapf(err, "create destination file <%s>", filepath)
	}
	defer fw.Close()
	err = os.Chmod(filepath, 0o644)
	if err != nil {
	return errors.Wrapf(err, "change permissions for file <%s>", filepath)
	}
	_, err = io.Copy(fw, data)
	if err != nil {
	return errors.Wrapf(err, "copy file <%s>", filepath)
	}
	err = fw.Sync()
	if err != nil {
	return errors.Wrapf(err, "sync file <%s>", filepath)
	}
	err = fw.Close()
	if err != nil {
	return errors.Wrapf(err, "close file <%s>", filepath)
	}
	err = os.Rename(filepath, finalpath)

Please ensure that the temporary file is deleted in case of an error before the function exists. Thus, the caller code may remain unaware of the implementation details specific to file system storage.

[DanielOliverRJ]

I've re-factored slightly to move write to a common function and attempt to remove the temporary file on any error.

[defbin]

tested by debugging