If the user picks an invalid timeline (or the default is invalid) they will not discover it until after the restore is complete and recovery starts. In that case they'll receive a message like this:
FATAL: requested timeline 2 is not a child of this server's history
DETAIL: Latest checkpoint is at 0/7000028 on timeline 1, but in the history of the requested timeline, the server forked off from that timeline at 0/600AA20.
This message generally causes confusion unless one is familiar with it. In this case 1) a standby was promoted creating a new timeline 2) a new backup was made from the primary 3) the new backup was restored but could not follow the new timeline because the backup was made after the new timeline forked off. Since PostgreSQL 12 following the latest timeline has been the default so this error has become common in split brain situations.
Improve pgBackRest to read the history files and provide better error messages. Now this error is thrown before the restore starts:
ERROR: [058]: target timeline 2 forked from backup timeline 1 at 0/600aa20 which is before backup lsn of 0/7000028
HINT: was the target timeline created by accidentally promoting a standby?
HINT: was the target timeline created by testing a restore without --archive-mode=off?
HINT: was the backup made after the target timeline was created?
This saves time since it happens before the restore and gives more information about what has gone wrong.
If the backup timeline is not an ancestor of the target timeline the error message is:
ERROR: [058]: backup timeline 6, lsn 0/4ffffff is not in the history of target timeline B
HINT: was the target timeline created by promoting from a timeline < latest?
This situation should be rare but can happen during complex recovery scenarios where the user is explicitly setting the target time.
Coverity complained about a possible overflow of result in the prior implementation.
It appears that Coverity was not able to follow the logic through the try block, but refactor and add an assertion to silence the complaint.
Coverity complained that decrementing targetIdx would result in it equaling UINT_MAX. While this is true it had no impact overall (at it least in the current code) since targetIdx was immediately incremented in the loop.
However, Coverity's suggestion is better and safer for future code updates so it makes sense to change it.
Coverity had this complaint:
assert_side_effect: Argument openData of ASSERT() has a side effect because the variable is volatile. The containing function might work differently in a non-debug build.
It appears this can also be fixed by assigning the volatile variable to an automatic but the cast seems to work just as well.
If a query that expected no results returned an error then it would incorrectly report that no results were expected because the error was interpreted as a result.
Switch the order of the checks so that an error is reported instead and add a test to prevent regression.
These functions get called very frequently even though they generally result in a noop at the protocol level.
Lower the log level to reduce noise in the log at debug level.
If a tablespace is created after the backup starts then it cannot be mapped using --tablespace-map-all since there is no record of it in the manifest.
This would be extremely complex to fix but it can be documented.
Verify that all StringIds in the project have been generated correctly.
This also makes it easy to generate new StringIds by copying an existing StringId and modifying the string. The error message will provide the required value.
PostgreSQL < 12 defaults recovery_target_timeline to current but if current is explicitly set it behaves as if latest was set. Since current is not handled in the PostgreSQL code it looks as if there should be an error during the integer conversion but that doesn't happen due to incorrect strtoul() usage (not checking endptr).
Handle this by omitting recovery_target_timeline from recovery.conf when it is explicitly set by the user to current.
The backup command has always been limited to working only when the repository is local. This was due to some limitations in storage (addressed in 01b81f9) and the protocol helper (addressed in 4a94b6be).
Now that there a no limitations preventing this feature it makes sense to enable it. This allows for more flexibility in where backups are run.
This was intended to separate the code changes from documentation and test suite changes but it arguably does not add any clarity.
Since documentation and test suite changes are explicitly marked as such that should be clear enough.
Previously, hex encode looked up each nibble of the input separately. Instead use a larger lookup table containing the two-byte encoding of every possible input byte, resulting in a 1/3 reduction in encoding time.
Inspired by and mostly cribbed from PostgreSQL commit e24d7708.
Simplify and improve data structures that track protocol client connections. The prior code could not store pg or repo clients but not both. We don't have a need for that yet, but tracking clients only by hostIdx was not flexible for some upcoming improvements. It is important to be able to identify and free clients very precisely.
In general this code should be easier to understand and removes duplicated code for local/remote clients.
When bundling and block incremental are both enabled the bundleRaw flag is set to indicate that headers are omitted (whenever possible) for encryption and compression. This is intended to save space, especially when there are very large numbers of small files.
If bundling is disabled this flag needs to be preserved so that existing bundles from prior backups are read correctly. However, the prior code was only saving the flag when bundling was enabled, which caused prior backups to be unreadable if bundling was disabled.
Fix so that the flag is preserved and backups are not broken.
Due to this bug the compression type in integration tests was always set to none. There are sufficient other tests for compression that this was not masking any bugs, but it was obviously not ideal.
Previously setting compress-level-network=0 would disable compression. This worked because gzip disables compression at this level but still transmits the data in gz format.
lz4 does not provide similar functionality so we would need to disable the compression filter entirely. This does not seem worth it however since lz4 compression is very efficient and 0 is the default fast mode.
This option was useful for the Perl code generation and autoconf generation, which were both slow. These are both gone now and the C code generation is fast enough that there is no need to exclude it.
--dry-run will still prevent certain code generation from running. This may not be necessary any more but removing it should be the subject of a separate commit.
This backup method does a preliminary copy of all files that were last modified prior to a defined interval before calling pg_backup_start(). Then the backup is started as usual and the remainder of the files are copied. The advantage is that generally a smaller set of WAL will be required to make the backup consistent, provided there are some files that have not been recently modified.
The length of the prior full backup is used to determine the interval used for the preliminary copy since any files modified within this interval will likely be modified again during the backup. If no prior full backup exists then the interval is set to one day.
This feature is being committed as internal-only for the time being.
Warn if a global variable is defined without a previous declaration. Use this option to detect global variables that do not have a matching extern declaration in a header file.
This code was duplicated in each driver so this means less duplication.
In addition, some drivers were not creating a parameter list for decompression which meant they could not be used remotely. This is not a currently a bug since none of them were being used remotely, but it was a blocker for using lz4 for protocol compression.
The integration tests could fail if:
1. After restoring the PostgreSQL instance the recovery process starts, which calls asynchronous archive-get.
2. After archive-get checks the existence of the queue directory, but before writing the WAL file, there are restores when the next test is begun, which leads to the deletion of the queue directory.
3. Since the directory no longer exists, writing the WAL file will fail, and archive-get will write the error file to the queue.
4. A new PostgreSQL instance will start and the recovery process will begin, which requests the WAL file.
5. The new archive-get looks into the queue directory, finds the error file, and throws out the error, after which the PostgreSQL recovery fails because the previous archive-get background process has not finished yet.
This patch fixes the problem by using a separate spool directory for each test.
An in 355e27d6, it makes sense to exclude FUNCTION_(LOG|TEST)_RETURN_VOID() macros when then they are on the last line of a function because in this case they are a noop (but are still used for debugging).
