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.
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.
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.
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.
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).
8d6bceb5 refactored version/help to operate more like regular commands in part to simplify the implementation of --version and --help. Unfortunately this had the side effect of these commands also loading pgbackrest.conf which would lead to an error if the file could not be read or parsed.
Add a filter to prevent version or help from loading pgbackrest.conf. Also prevent reads from the env to stop any warnings or errors from that source.
0c32757f made lz4 required in the meson build but conditional compilation was left in to make reverting easy for packagers just in case.
Since a few releases have gone by without any complaints, remove conditional compilation for lz4.
Warn if anything is declared more than once in the same scope, even when the extra declaration is valid and changes nothing. This is primarily useful for catching missing header ifdef barriers.
Move the environ variable into config/parse.h since it must be declared by us and we use it multiple times.
Warn if a global function is defined without a previous prototype declaration. This helps detect when a function that should be static is accidentally declared extern.
Most of the changes are to add missing header files so functions can see their declarations.
In a some cases functions that should have been static were marked as such. There were only five of these in the core but every little bit counts.
Lastly, it was necessary to suppress the warning in the postgres test modules where the function declarations are not available. This is fixable by aligning the module with the auto-generated code in core, but is not a priority.
Warn whenever a pointer is cast so as to remove a type qualifier from the target type. For example, warn if a const char * is cast to an ordinary char *.
Most of the changes for this are fairly rote: just add a const qualifier where needed. In some cases functions needed to be reworked to return non-const where before they had returned const and then cast it back to non-const. None of these patterns appeared to be bugs, but they were certainly misleading.
Some cases (especially excvp() and calls to bz2) could not be fixed because of how functions out of our control are defined. In those cases the warnings have been suppressed and a comment added to detail the exception. This was also done a few places in the tests.
These three objects can be created as constants at compile time using specialized macros. Unfortunately since the values assigned are also const, cast-qual complained about the cost qualifier being lost.
Fix this by creating new structures to be used just for creating these constants. This is not ideal due to the need to keep the duplicated structures in sync, but in practice these structures are almost never modified. Testing should catch any out of sync structures and this feature is valuable enough to keep even though in theory there could be memory safety issues. In practice the APIs prevent const objects from being used in an unsafe way and testing provides a fair assurance of safety. Writing to these consts would be a fatal error even if it did not cause a segfault.
Ideally, we would be able to use warning suppression in these macros to avoid the extra struct, but due to the way they are used it is not possible to add the required pragmas (even using _Pragma).
Finally this construction makes it obvious that something special is being done, rather than it being under the covers.
Per our policy to support five EOL versions of PostgreSQL, 9.4 is no longer supported by pgBackRest. Remove all logic associated with 9.4 and update the tests.
This includes a small fix in infoPg.c to allow backup.info files with old versions to be saved. This allows expire to function when old versions are present. Even though those older versions cannot be used, they can be expired.
Tests for 9.4 are left in the expire/info tests to demonstrate that these commands work with old versions present.
NOTE TO PACKAGERS: This is last feature release to support the autoconf/make build. Please migrate to meson if you have not already done so. 2.54.X patch releases (if any) will continue to support autoconf/make.
Bug Fixes:
* Fix PostgreSQL query performance for large datasets. (Fixed by Thibault Vincent, David Steele. Reviewed by David Christensen, Antoine Millet. Reported by Antoine Millet.)
Features:
* Allow repositories on versioned storage to be read at a target time. (Reviewed by Stefan Fercot, David Christensen.)
* Allow requested standby backup to proceed with no standby. (Reviewed by Stefan Fercot.)
Improvements:
* Summarize backup reference list for info command text output. (Contributed by Stefan Fercot. Reviewed by David Steele.)
* Refresh web-id token for each S3 authentication. (Contributed by Brent Graveland. Reviewed by David Steele.)
* Correctly display current values for indexed options in help. (Reviewed by David Christensen.)
* Save backup.info only when contents have changed. (Reviewed by Stefan Fercot.)
* Remove limitation on reading files in parallel during restore. (Reviewed by David Christensen.)
* Improve SFTP error messages. (Contributed by Reid Thompson. Reviewed by David Steele.)
Documentation Features:
* Add performance tuning section to user guide. (Reviewed by Stefan Fercot.)
Documentation Improvements:
* Clarify source for data_directory. (Contributed by Stefan Fercot. Reviewed by David Steele. Suggested by Matthias.)
* Better logic for deciding when a summary should be lower-cased. (Suggested by Daniel Westermann.)
There have been occasional SFTP authentication failures on 32-bit. We are planning to drop 32-bit support so it does not seem worth chasing these errors down and they are likely timing issues anyway.
In the case of a rapid restart it is possible that the socket may not be immediately available, so retry until it becomes available.
This is particularly useful for testing where sockets are bound and released very rapidly.
Option help summaries do not have initial capitals (except in special cases) and final periods so it makes sense to render the command summaries the same way.
Use the same function for both so they are consistent.
The asynchronous logic used to implement the query timeout was misusing PQisBusy(), which caused the wait handler to throttle the consumption of command results. It could introduce a large delay on a query up to `db-timeout` because of the back-off sequence.
Following the recommendation of libpq, fix by polling the client socket for data availability and then continue consuming results and checking for command busyness.
This is useful for code that has its own wait mechanism, e.g. poll(), but still needs a way to track overall time elapsed.
To keep it simple waitRemains() is called by waitMore().
Ubuntu 20.04 has been having consistent errors starting PostgreSQL 10 so move 9.5 to this container instead. An older version makes sense with an older distro.
Also move PostgreSQL 12 from RHEL 8 since this version will be EOL soon.
The current value for an indexed option was always for the first index, e.g. pg1-path. This is likely legacy from before indexing was added (and faithfully copied over from Perl, apparently).
Fix this by enumerating the current values in the option help and displaying <multi> in the option list when more than one value exists.
