This restriction prevented multiple files being read from a remote simultaneously, which was not supported by the protocol. Although the limitation only applied to remotes, it was applied in all cases for testing purposes and because we planned to fix it.
Protocol command multiplexing added in df8cbc91 allows files to be read simultaneously from a remote so this restriction is no longer required.
Note that there is a test for this condition since the prior code had coverage. It might be tricky to ensure that test doesn't go away, but in general we should have enough tests in place to ensure simultaneous reads function as expected.
Add a "prefer" value to the backup-standby option to allow the backup to proceed when no standby is found. Note that this will not help if the standby is responding but fails to sync with the primary after the backup has started.
Introduce a new option modifier, bool-like, that allows a boolean option to be converted to a string or string-id option while still allowing the option to act like a boolean on the command-line, e.g. --no-backup-standby.
This prevents backup.info from being saved again when expire does not make any changes.
More importantly, as we look to support versioning on object stores, it will be much easier to determine a good point-in-time to use for restore if there are no extraneous saves of backup.info.
In these functions infoBackup was marked as const even though it was modified in the function. This was allowed by the compiler because the infoBackup struct was not being directly modified but it still goes against our coding conventions.
The token file pointed to by the AWS_WEB_IDENTITY_TOKEN_FILE environment variable was read once at startup, but for long operations the token might expire before completion.
Instead read the token on each S3 authentication so the current token is always used.
Move tests that use functions that are not valid for non-Posix repositories to pg storage.
This allows for a test storage driver that only implements functions required for repositories.
Add getters where needed and remove the unused driver member from the StorageRead/Write objects. The new getters are only required for testing but they don't compromise the core code.
The rm would miss files beginning with a dot which are being used going forward for the versioning test driver.
Instead use find since it will delete all files.
This function will be needed elsewhere going forward and in any case it makes sense to move this out of the restore module.
Refactor to avoid dependency on a regular expression.
Connection errors could cause a segfault if the error was delayed enough to pass the initial call to sckClientOpenWait() and the error was instead thrown by a subsequent call to sckClientOpenWait(), which was not correctly initializing a variable required for error handling.
While this can be produced fairly easily in a test environment, I was unable to craft a unit test to hit this exact condition, probably due to timing. The new code still has full coverage and I added several comments to help prevent regressions.
When restore was run as the root user the pg_control file would end up with root permissions. This bug was introduced in e634fd8. Fix this by directly overwriting the pg_control temp file rather than doing an atomic write that updates permissions. Also update other parameters to more closely match similar calls.
There was also an adjacent error where restore as the root user would fail if the base path did not exist. Fix this by ignoring the missing path since it will be created later and this logic is just trying to find an alternate user for permissions if the user in the manifest does not exist.
The backup reference list can be very long so it seems better to summarize the list by default for text output and keep the full list when --set is specified.
The none command was a bit confusing since it was only valid when parsing failed but still needed to be added to various switches and logic. Replace with cfgInited() which should make it clearer what state configuration is in.
Make the default command help and convert --version and --help to real options.
Combine version and help output into a single function to simplify processing in main.
Additional reformatting and a bit of refactoring.
This refactor simplifies the main() functions and puts the more commonly run commands first.
For core main() also remove code duplication in local/remote role handling.
Previously requesting help for roles other than main would result in a segfault since help.auto.c.inc does not include such help.
Prevent this by erroring when a user requests help for roles other than main.
This is a bug but it is unlikely to have ever been seen in the field and in any case it has a very low severity since it only impacts a corner-case for the help command.
This command was used by the Perl integration tests to create buckets for storage types that required it. Now that the integration tests are written in C they can simply use the same code to create buckets.
The command was also used in the documentation but there it seems more appropriate to use the corresponding vendor CLI.
Previously it was not possible to read or write two files at the same time on the same remote because the protocol was entirely taken over by the read or write command. Multiple reads are required to make restores efficient when a list of bundled files is being read but blocks need to be retrieved from a separate file or a different part of the same file.
Improve that situation with sessions that allow related commands to be run with shared state. Also break read/write into separate requests (rather than pushing all data at once) so they can be multiplexed.
The disadvantage for read/write is that they now require more back and forth to transfer a file. This is mitigated by sending asynchronous read/write requests to keep both server and client as busy as possible. Reads that can fit into a single buffer are optimized to transfer in a single command. Reads that transfer the entire file can also skip the close command since it is implicit on end-of-file.
These changes allow the protocol to be simplified to provide one response per request, which makes the data end message obsolete. Any data sent for the request is now added to the parameters so no data needs to be sent separately to the server outside the request parameters.
Also update the Db protocol to use the new sessions. Previously this code had tracked its own sessions.
IMPORTANT NOTE: The log-level-stderr option default has been changed from warn to off. This makes it easier to capture errors when only redirecting stdout. To preserve the prior behavior set log-level-stderr=warn.
NOTE TO PACKAGERS: The lz4 library is now required by the meson build.
NOTE TO PACKAGERS: Compiler support for __builtin_clzl() and __builtin_bswap64() is now required by the meson build.
Bug Fixes:
* Fix SFTP renaming failure when file already exists. (Fixed by Reid Thompson. Reviewed by David Steele. Reported by ahmed112212.)
Features:
* Allow backups to run concurrently on different repositories. (Reviewed by Reid Thompson, Stefan Fercot.)
* Support IP-based SANs for TLS certificate validation. (Contributed by David Christensen. Reviewed by David Steele.)
Improvements:
* Default log-level-stderr option to off. (Reviewed by Greg Sabino Mullane, Stefan Fercot.)
* Allow alternative WAL segment sizes for PostgreSQL ≤ 10. (Contributed by Viktor Kurilko. Reviewed by David Steele.)
* Add hint to check SFTP authorization log. (Contributed by Vitalii Zurian. Reviewed by Reid Thompson, David Steele.)
Documentation Improvements:
* Clarify archive-push multi-repo behavior. (Reviewed by Stefan Fercot.)
Since 1141dc20 it has been possible to request that cfgParse() skip loading the config file. Use this logic to replace the code used to ignore config files in doc/test config load.
Typically we use the oldest Debian/Ubuntu to run 32-bit unit and integration tests. However, 32-bit is no longer fully supported by Ubuntu (multiple packages we need are missing) and apt.postgresql.org no longer packages for any 32-bit version.
To address these changes, do 64-bit integration testing on the oldest Debian/Ubuntu (currently Ubuntu 20.04) and 32-bit unit/integration testing on the oldest Debian (currently 11) using the included version for integration testing.
Mock integration tests were removed in d41b21c8 but CI was still trying to run them, so remove from CI.
Also rename mock to integration in test unit tests to avoid confusion in the future.
The prior SAN code only recognized DNS-based SANs, which meant that it would not properly validate if using an IP-based SAN.
Add support for IPv4 and IPv6 SANs with exact matching only.
This simplifies testing when certificate generation tools have trouble generating a DNS:1.2.3.4-style SAN, preferring to include the SAN as IP:1.2.3.4.
Reduce redundancy by improving cfgParseCommandId() to work when a command role is present. This way the function does not need to be called twice.
Also, remove the use of StringList in cfgParse() since checking for a colon is faster and saves memory allocations. Modify cfgParseCommandRoleEnum() to accept char * since a String is no longer produced for the role name.
Errors in code generation can cause the test binary build to fail and then it is not possible to see the generated code.
Instead, generate code first so any errors can be seen and analyzed.
Create mappings between integer, size, time, and stringid option values and their string equivalents. This allows for better error messages and means that the mappings do not need to be stored with defaults, allow lists, etc.
Update error handling for libssh2_sftp_rename_ex() in storageWriteSftpClose() when a file already exists.
The SFTP servers used during development and testing never returned LIBSSH2_FX_FILE_ALREADY_EXISTS, rather they returned LIBSSH2_FX_FAILURE when a file already existed. However, it is clear that some SFTP servers use LIBSSH2_FX_FILE_ALREADY_EXISTS so add support.
The prior locking only allowed one backup per stanza, which was required by PostgreSQL <= 9.5 and didn't present a problem when only one stanza could be created.
Now that multiple stanzas are allowed relax this restriction so that backups can run concurrently for PostgreSQL > 9.5. To do this, update the locking to be per stanza and repo rather than per stanza. Remotes are not aware of the repos that require locking so send an explicit list of files to be locked to the remote. Also remove the advisory lock for PostgreSQL > 9.5.
For info output the running backups are combined for progress output in order to avoid changing the JSON format. It definitely makes sense to have per repo progress as well but that will be left for a future commit.
Similar to size options in 038abaa7, time option defaults and allowed values were displayed in seconds, which could be confusing when the values were large.
The time options were not updated in 038abaa7 because it required removing the ability to do fractional seconds, e.g. 0.5 seconds. In theory this could cause breakage for users but it seems really unlikely. Fractional seconds are used in tests, however, so the tests have been changed to use milliseconds where required, e.g. 500ms.
This default was being mistakenly inherited from job-retry and would set job-retry-interval to 1 second for archive get and push. In practice this did not matter since archive get and push default to one retry and the first retry interval is always 0, but it still makes sense to fix it to correct the behavior when the value of job-retry is changed.
