In many cases the valid commands are based on the commands valid for roles. In these cases derive the commands from a role list rather than an explicit command list.
Not only is this notation more compact but it helps prevent new commands from being missed.
This exposed a few issues:
1) The cmd option should only be valid when a command supports the local role since it is used to execute the local process. A number of commands were included before that did not have the local role.
2) cmd-ssh should be valid for any command that allows remotes. The annotate command was missing from this list.
3) compress-level-network should be valid for any command that allows remotes. The repo-rm command was missing from this list.
Restoring to a remote pg-host is not supported but the options were a bit untidy. Many options were marked as internal but should be invalid. repo-host-type and repo-host are required to let restore know if a pg-host is configured and remain internal but the rest of the pg-host-* options are now invalid for restore.
The same applies to the archive-get and archive-push commands although these were less likely to cause confusion.
Also reverse the dependency of pg-host and pg-host-type, i.e. make pg-host-type depend on pg-host, and alter pg-host-cmd and pg-host-user to depend on pg-host-type=ssh.
931435c0 added the ability to backup to a remote repo but did not quite get the option updates right. It worked, but a number of options were marked as internal so would not be visible to the user in command-line help.
Also reverse the dependency of repo-host and repo-host-type, i.e. make repo-host-type depend on repo-host, and alter repo-host-cmd and repo-host-user to depend on repo-host-type=ssh.
Previously internal state was not included because it does not affect how commands/options are used -- only whether they are shown in help. However, this makes it hard to know when the internal state changes because help is generated at build time and in any case is just a binary blob.
Internal state is not stored in the config structures since the macros resolve to nothing but it is handy for debugging to see when internal state has changed.
The Perl processing of config.yaml put hard limits on the format of that file. To allow flexibility in the file format remove all Perl processing on config.yaml.
This is just the beginning of migrating the preprocessor to C but even this small bit allows the removal of a lot of Perl code.
55e9969 updated the meson version but only reverted one of the changes implemented in 0eccbc8 where the version was lowered to >= 0.45.
Use get_option() as allowed by the updated version for clearer and more robust build code.
Per our policy to support five EOL versions of PostgreSQL, 9.5 is no longer supported by pgBackRest. Remove all logic associated with 9.5 and update the tests.
An effort was made to advance versions as much as possible in the tests while still providing coverage. Hopefully this will reduce churn when future versions expire, though it has created a bit more here.
Tests for 9.4/9.5 are left in the expire/info tests to demonstrate that these commands work with old versions present.
The 9.6 pg_control struct was being used for 9.5. This was not detected by testing because the new field introduced for 9.6 fit into an alignment hole in the 9.5 struct so the size of the struct and offset of all other members did not change. Since the new member was not used there was no impact on functionality.
9.5 is being removed in the next release so the only reason to fix this is to make the diff for that change more sensible, and to document that this happened.
Some of these types were versioned at one time. Others were not but it seemed better to version all of them for consistency. In fact this just creates churn when PostgreSQL versions are expired.
Also move the uint64 type to version.vendor.h since it is only used by versioned types.
Some of these macros have required different implementations according to PostgreSQL version but none of them do now.
The idea of versioning them even when not required was that it would reduce churn but in fact churn is increased because PostgreSQL versions expire far more often than these macros need version specific implementations.
Bug Fixes:
* Unnest HTTP/TLS/socket timeouts. (Reviewed by David Christensen.)
* Fix possible segfault in page checksum error message. (Fixed by Zsolt Parragi. Reviewed by David Steele.)
Features:
* Add repo-symlink option to suppress creation of repository symlinks. (Reviewed by Douglas J Hunley. Suggested by Ron Johnson.)
Improvements:
* Add HTTP retries for 408 and 429 errors. (Reviewed by David Christensen.)
On recent versions of Docker it is not necessary to specify the architecture of the require image. The --platform option takes care of it.
Rebuild test images that were modified by this change.
This test was removed in c64cd8e0 because it was taking too long to be scheduled, which held up testing.
Now that Github Actions supports aarch64 move the test there.
Similar to adc5e5b23, STRDEF assignment to reason can be out of scope, which may lead to either garbage in the log message or a crash with SIGSEGV.
Fix by using simple char * constants for the reason.
The SFTP storage driver did not set the pathSync method but had logic to see if pathSync was set. This led to some tortured tests since pathSync had to be injected into the interface after the storage object had been created.
Remove the pathSync parameter from storageWriteSftpNew() and also remove related tests.
These timeouts were nested, i.e. an error in the socket connection would also retry in the TLS and HTTP layers. This led to a multiplying effect such that it took nine minutes to fully timeout with the default io-timeout of 60 seconds.
A fix for this was attempted in 5314dbff but it reduced retries by too much and had to be reverted in fa5b2d44.
Instead fix by moving connection attempts to the lower layer (e.g. TLS -> socket) out of the exception block but leave it within the retry loop. So, for example, if the socket connection fails after retries then the error will not be retried by TLS. But if the TLS session fails the socket will be reconnected.
The STRDEF definition of plural is out of scope in the error message just below it, which may lead to either garbage in the log message or a crash with SIGSEGV.
This is particularly visible with pg_tde, which encrypts tables: if pgbackrest is run with checksum enabled, it tries to emit this message for all encrypted files.
Fix by using simple char * constants in the error message.
Rebuild all containers to get the most recent versions of PostgreSQL.
Update the Debian repository install to match current recommendations. This has already been done for the documentation in fcd00a45.
HTTP client errors 408 and 429 were not being retried but there seems to be some benefit to doing so.
408 has only been seen once and in that case the server was returning 400 (with Request Timeout in the response body) but it seems worth doing since this could happen during times of high congestion. Requests are not sent until request content is ready so there is not much else to be done to handle this error.
429 has been seen occasionally in the past but now seems to be common on Cloudflare R2. Ideally we would have a large initial back off here but it is not clear that is worth it at this time. The existing Fibonacci back off should be enough to allow operations to proceed if possible during the configured timeout.
Bug Fixes:
* Fix issue with adhoc expiration when no backups in a repository. (Reviewed by Stefan Fercot. Reported by Anup Gupta.)
Features:
* Add restore progress to info command output. (Contributed by Denis Garsh, Maxim Michkov. Reviewed by David Steele.)
* Add progress-only detail level for info command output. (Contributed by Denis Garsh. Reviewed by David Steele, Stefan Fercot.)
Improvements:
* Retry failed reads on object stores. (Reviewed by David Christensen.)
* Fix defaults in command-line help. (Reviewed by David Christensen, Chris Bandy.)
Documentation Improvements:
* Describe discrete option values in a list where appropriate. (Contributed by Anton Kurochkin. Reviewed by David Steele.)
* Fix "less than" in help output for archive-mode option. (Contributed by Anton Kurochkin. Reviewed by David Steele.)
The prior implementation of the restore command did not provide any progress information. Implement it similarly to the backup command and also update the info command to display restore progress alongside backup progress.
Restore is a read operation and should not block other commands. The only exception is that multiple restores with the same lock and repository path are not allowed, as each restore must write progress to a separate file. Therefore, no lock is needed for restores with a remote role and the restore command should not be terminated when the stop command is executed.
The info command fetches a lot of information from the repository about backups and archives, so this operation can be slow. Because progress data is stored in local lock files, accessing the repository is unnecessary when only progress information is required.
This patch introduces the `--detail-level=[progress|full]` option, with `full` as the default. The `progress` level limits the info command output to progress details without querying the repository. The only remaining operations are scanning the folder structure to list available stanzas and reading lock files.
Note: When `progress` is selected, the info command performs no checks beyond verifying stanza availability.
If there are no backups in one or more repositories then the following error occurs during adhoc expiration:
ASSERT: [025]: cannot get index 0 from list with 0 value(s)
Fix this by skipping the adhoc logic when there are no backups in a repository.
The nonstring variable attribute specifies that an object or member declaration with type array of char, signed char, or unsigned char, or pointer to such a type is intended to store character arrays that do not necessarily contain a terminating NUL.
Newer versions of gcc will warn if this attribute is missing.
Some defaults and allow lists were determined in cfgLoadUpdateOption() because they depended on the values of other options, .e.g. compress-level.
Instead build this functionality into the config parser. Not only does this standardize the defaults and allow lists but it makes it possible to automate the documentation, which is also done in this commit.
It is currently not possible to determine the default of all options knowing just the command. Some defaults are set in cfgLoadUpdateOption() and in an upcoming commit defaults may be based on the value of other options.
It would be possible to update parser to provide this information but that will complicate the parser and since the logic is only used to simplify options passed to remotes it does not seem worth the effort.
For the same reason cfgParseOptionRequired() can also be removed.
David Steele
Zsolt Parragi
Denis Garsh