[wip] Proof-of-concept fix for missing .partial WALs on recovery #629

mikewallace1979 · 2022-07-27T09:38:41Z

Updated with a different approach where .partial files are handled after copying the WALs in the archive.

There are a few upsides to doing this:

We no longer need special cases in the code branch for compressed WALs.
We can use Rsync.from_file_list to copy the .partial files when WALs are uncompressed, since all .partial files now get copied and renamed in a temporary staging directory.

Including the .partial WAL files in the get_required_xlog_files response seems a bit odd since one of the first things we do in _xlog_copy is put them in a separate list so they can be handled after the WALs in the archive are copied, however it feels like the right thing to do given that the .partial file is a required xlog file (unless the requested recovery timeline or time dictate otherwise).

edb-sonar-app · 2022-07-27T09:41:44Z

SonarQube Quality Gate:

0 Bugs
0 Vulnerabilities
0 Security Hotspots
3 Code Smells

35.7% Coverage
0.0% Duplication

mikewallace1979 · 2022-10-05T11:03:03Z

Having talked this through with @amenonsen the assumption about the .partial file being uncompressed is reasonable.

Compression in the .partial file can happen in two places:

PostgreSQL wal_compression - if this is set then pages within the WAL will be compressed, however the file itself is not a compressed file in any sense that external tools need to care.
pg_receivewal compression - this compresses the .partial file but applies a .gz.partial suffix:

	/* File looks like a partial gzip-compressed WAL file */
	if (fname_len == XLOG_FNAME_LEN + strlen(".gz.partial") &&
		strcmp(filename + XLOG_FNAME_LEN, ".gz.partial") == 0)
	{
		*ispartial = true;
		*wal_compression_method = COMPRESSION_GZIP;
		return true;
	}

Given:

Barman runs pg_receivewal itself and we do not support it being run out-of-band writing files into the streaming directory.
Barman does not use pg_receivewal's compression feature.
Barman does not consider a .gz.partial file to be a WAL.

Any .partial file we encounter in Barman at this point is therefore going to be uncompressed (at the file level).

This is a first pass at including .partial files in the streaming directory when copying WALs on recovery. The implementation works by appending any .partial files in the streaming directory during `Server.get_required_xlog_files` and then handling `.partial` files as a special case during `RecoveryExecutor._xlog_copy`. It works, but I don't like it for a number of reasons: 1. The special cases to handle .partial files during _xlog_copy are awkward and does not make for readable code. 2. The assumption that `.partial` files will always have no compression is questionable. Alternatives: 1. Make WalFileInfo handle `.partial` details transparently. 2. Don't use `get_required_xlog_files` at all and instead just have a "and check for .partial files" somewhere in `_xlog_copy`. 3. Flip things around so instead of finding `.partial` files in the `streaming` directory and seeing if they are needed, we figure out from the most recent required xlog what the next xlog will be given the recovery target and use some of the `get-wal` code to look for that specific file. A couple of other things to consider: 1. Should we also check `incoming` for WAL files? If we want parity with `barman get-wal` then we should also check `incoming`. 2. Should we also include non-partial WALs in `streaming` (and `incoming`) in the copy?

…ery" This reverts commit 196c6a9.

Instead of attempting to copy .partial files during the same `Rsync.from_file_list` call, we instead treat it as a separate copy. Offers a number of improvements over the previous attempt: 1. `.partial` files are now also copied when WALs are not compressed. 2. The `.partial` file is not copied if it is not required due to the requested recovery time being met by existing WALs in the archive. On the downside, the sonarqube cognitive complexity metrics are going to hate this. There are a few opportunities for extracting bit of code but both `get_required_xlog_files` and `_xlog_copy` are already quite long and complex functions so it's unlikely we'll be able to make sonarqube completely happy.

…es function

… one This "should never happen" however if `get_required_xlog_files` ever gets run outside of the `barman recover` context it is possible that there are older `.partial` files which haven't been dealt with by the archiver process yet. In such cases we should make the same decision the archiver would and use the latest file.

…an that requested

Updates _xlog_copy so that special handling is only used for `.partial` files in the streaming directory. Any `.partial` files found in the WAL archive are handled the same way Barman would previously handle them, i.e. they are copied across to the recovery WAL destination (decompressing if necessary) and *not* renamed. Because this is a situation which requires some manual intervention a warning message is now logged to tell the user about each `.partial` WAL found in the archive. The user must then determine whether the `.partial` WAL in the archive is the result of a recovery from backup or a standby promotion and, if so, can rename the file to omit its `.partial` extension. There is a clear opportunity for Barman to handle this itself however this is deliberately not tackled by this patch and will instead be added at a later date when we improve the behaviour of Barman in a clustered environment. Relates to #393.