Since there were some issues found with the meson install (7877983a, 7b95fd3b) it makes sense for any packagers who have not made the migration to hold off until the next release.
Move the note to the next release where hopefully all issues have been addressed.
This feature (enabled with --repo-s3-sse-customer-key) provides an encryption key to encrypt the data after it has been transmitted to the server.
While not as secure as encrypting data before transmission (--repo-cipher-type), this may be useful in certain configurations.
On some platforms, e.g. FreeBSD, there is a requirement to allow the user to disable support for features even when the required library is present.
Introduce tri-state options for the optional features: auto mimics the current behavior and is the default, enable requires libraries for the feature to be present, and disable disables the feature without checking the libraries.
NOTE TO PACKAGERS: The build system for pgBackRest is now meson. The autoconf/make build will not receive any new features and will be removed after a few releases.
Bug Fixes:
* Skip zero-length files for block incremental delta restore. (Reviewed by Sebastian Krause, René Højbjerg Larsen. Reported by Sebastian Krause.)
* Fix performance regression in storage list. (Reviewed by Stephen Frost. Reported by Maksym Boguk.)
* Fix progress logging when file size changes during backup. (Reviewed by Stephen Frost. Reported by samkingno.)
Improvements:
* Improved support for dual stack connections. (Reviewed by Stephen Frost. Suggested by Timothée Peignier.)
* Make meson the primary build system. (Reviewed by Stephen Frost.)
* Detect files that have not changed during non-delta incremental backup. (Reviewed by Stephen Frost.)
* Prevent invalid recovery when backup_label removed. (Reviewed by Stephen Frost.)
* Improve archive-push WAL segment queue handling. (Reviewed by Stephen Frost.)
* Limit resume functionality to full backups. (Reviewed by Stephen Frost, Stefan Fercot.)
* Update resume functionality for block incremental. (Reviewed by Stephen Frost.)
* Allow --version and --help for version and help. (Reviewed by Greg Sabino Mullane. Suggested by Greg Sabino Mullane.)
* Add detailed backtrace to autoconf/make build. (Reviewed by Stephen Frost.)
Documentation Improvements:
* Update references to recovery.conf. (Reviewed by Stefan Fercot. Suggested by Stephen Frost.)
If the file size changed during backup then the progress percentage in the log would not be accurate.
Fix this by using the original size to increment the progress since progress total was calculated from original file sizes.
Resume was not updated for block incremental so block incremental files were always removed during a resume. Resume worked but was very inefficient with block incremental enabled.
Update resume to preserve block incremental files and add tests.
If backup_label is removed from a restored backup then PostgreSQL will instead use checkpoint information from pg_control to attempt (what is thinks is) crash recovery. This will nearly always result in a corrupt cluster because the checkpoint will not be from the beginning of the backup, and even if it is, the end point will not be specified, which could lead to recovery stopping too early.
To prevent this, invalidate the checkpoint LSN in pg_control on restore. If backup_label is removed then recovery will still fail because PostgreSQL will not be able to find the invalid checkpoint. The LSN of the checkpoint is not logged but it will be visible in pg_controldata output as 0/DEAD. This value is invalid because PostgreSQL always skips the first WAL segment when initializing a cluster.
Connections are established using the "happy eyeballs" approach from RFC 8305, i.e. new addresses (if available) are tried if the prior address has already had a reasonable time to connect. This prevents waiting too long on a failed connection but does not try all the addresses at once. Prior connections that are still waiting are rechecked periodically if no subsequent connection is successful.
This improves substantially on 39bb8a0, which failed to take into account connection attempts that do not fail (but never connect) and use up all the available time.
This saves about 16KiB in the binary and reduces exported symbols by about 75%. All variables are still exported and any functions that are referenced by their pointers or extern'd but never used outside the module where they are defined.
In addition to modest space savings, this should also increase performance a bit since the compiler can simplify calls to these functions and load the binary should also be a little faster.
The GCC documentation does not make it clear that visibility can be used with variables, but it certainly makes a difference in the binary size, so something is happening. Other sources on the internet suggest that visibility can be used with variables. Clearly exports are not affected, but there may be some other optimization happening.
Meson has a lot of advantages over autoconf/make, primarily in ease-of-use and performance. Make meson the only build system used for testing and building the Debian documentation, but leave the RHEL documentation using autoconf/make for now so it gets some testing.
Resume does not work correctly with delta diff/incr backups because the presence of a reference causes it to remove the file with the idea that it can just be referenced again. This is true for timestamp-based backups but for deltas all existing files need to be rechecked (which requires a reference).
This is fixable but not without significant effort and new tests and it calls into question the usefulness of non-full resumes. For diff/incr, if the file was changed since the prior backup there is a good chance it will be modified again before the resume occurs.
In order to keep this feature as useful as possible for the most valuable case, limit resumes to full backups.
02eea55 added code to load a buffer of data from a file being backup up to detect files that have been truncated to zero after manifest generation. This mechanism can also be used to detect files that have not changed since the prior backup.
If the result of the file copy fits into a single buffer, then the size and checksum can be compared to the prior file before anything gets stored. If the file matches then it is referenced to the file in to prior backup.
The size that can be compared for normal copies is limited by the buffer size but for block incremental it works with any size file since there is no output from block incremental when the file is identical.
Infer the size of all WAL segments from the size of the first segment rather than getting info for all segments (up to queue size). If the segments are not the same size then there are larger issues than the WAL queue.
storageListP() returns a list of entries in a path and should not need to stat/head, etc. in order to get more detailed info. This was broken by 75623d4 which failed to set the level correctly.
Set the correct level and update tests.
There's no easy way to directly test for a regression here but the SFTP tests will fail if more detailed info is requested since it would require script changes.
The Perl integration tests were migrated as faithfully as possible, but there was some cruft and a few unit tests that it did not make sense to migrate.
Also remove all Perl code made obsolete by this migration.
All unit, performance, and integration tests are now written in C but significant parts of the test harness remain to be migrated.
a42614e introduced the capability to preserve smaller than expected files for block incremental restore delta, but failed to take into account that zero-length files are both useless and cause the block checksum filter to error.
Fix this by skipping zero-length files during block incremental restore delta.
These tests have not been maintained for several years, i.e. no tests for new features have been added. They are highly duplicative of the unit tests but do have the advantage of mixing in different storage drivers. They were allowed to remain because they were not doing any harm even if they were probably not doing any good.
However, the real integration tests (that run directly against PostgreSQL) also test storage drivers and have been updated with new features over time. The real integration tests are now being migrated to C and as part of that effort the mock integration tests need to be removed or migrated, and they do not provide enough value to migrate.
Remove all mock integration tests and a leftover Perl performance test.
Update references to recovery.conf to include postgresql.auto.conf used in newer versions.
Also update a broken recovery URL and point it to the current version (with a hint to select the proper version of PostgreSQL).
c8264291 added libbacktrace to the meson build (not used in production yet), but held off on adding it to autoconf/make before more performance testing was done.
Performance tests show there is no noticeable cost to adding libbacktrace, so add it to get more detail error stack traces.
It is a bit confusing that --help and --version do not work like most command-line programs. For example, git allows either --help or help.
Make these work by making them shortcuts (not actual options) to the applicable commands.
The user will still need to use help (not --help) to get help on specific commands/options, but at least they can get to the main help (which will tell them this) via --help.
Bug Fixes:
* Fix short read in block incremental restore. (Reviewed by Stephen Frost, Brent Graveland. Reported by Adol Rodriguez, Brent Graveland.)
* Fix overflow suppressing backup progress in info output. (Fixed by Robert Donovan. Reviewed by Joe Wildish.)
Improvements:
* Preserve partial files during block incremental delta restore. (Reviewed by Stephen Frost.)
* Add support for alternate compile-time page sizes. (Contributed by Viktor Kurilko. Reviewed by David Steele.)
* Skip files truncated during backup when bundling. (Contributed by Georgy Shelkovy. Reviewed by David Steele.)
* Improve SFTP storage error messages. (Contributed by Reid Thompson. Reviewed by David Steele.)
Use storageSftpEvalLibSsh2Error() in more locations to provide better error information. Also add storageSftpLibSsh2SessionLastError() for the same reason.
During restore it is possible to read all the blocks out of a compressed super block without reading all the input. This is because the compression format may have some trailing bytes that are not required for decompression but are required to indicate that data has ended. If a buffer aligned with the compressed data in a certain way, these last bytes might not be read.
Explicitly read out any final bytes at the end of each super block to handle this case. This should always result in no additional data out and we check for that, but it does move the read position to the beginning of the next compressed super block so decompression can begin without error.
Refactor 02eea555 to always close the file immediately on EOF and use backupCopyResultCopy to continue processing. Closing the file immediately saves a later EOF check and is friendlier to added logic in this area. Using backupCopyResultCopy to continue is clearer also makes it easier to add new logic.
Also store zero checksum so the bulk of results collection can be moved within the copy block.
This allows less duplication of buffers.
For delta check return file->pgFileSize/file->pgFileChecksum instead of pgTestSize/pgTestChecksum since this saves one buffer duplication and we know these values are equal since we just checked them.
Also add an assert to ensure copyChecksum is valid relative to size.
If a file stored with block incremental shrinks below the prior block size then the map is useless and the entire file needs to be stored again.
In this case use the new block incremental values (even if none) rather than preserving the old ones.
Previously files that were smaller than the expected size were not preserved for block incremental, even though it is possible that block incremental could make use of a partial file.
One example is when a restore encounters an error. On retry the partial file can be used as a starting point rather than copying again from the beginning. Another example is restoring a backup where a file is larger than what already exists in the data directory.
Preserve any size file when block incremental will be used for the delta in order to reuse partial files when possible. If the file is smaller than expected then disable the whole-file checksum to reduce overhead.
This refactor should provide more clarity on what factors affect an incremental, rather that just having one big expression do it all. Overall this may be slightly more efficient since some values are reused that before were recalculated.
No behavioral changes are introduced.
Writing the sz and szCplt parameters in the lock file used jsonWriteUInt64() but reading these parameters used jsonReadUInt(). This caused a silent exception for any backups larger than MAX_UINT and prevented the info command from reporting progress.
Correct this so the reads are symmetric and verified before/after with a test.
In bundle mode pgBackRest skips files of zero size, that is, it does not queue them for copying.
After splitting the files into bundles, pgBackRest launches one or more processes that directly perform the backup, namely, read the files and, if necessary, write them to the bundles.
If during the time between the distribution of all files among bundles and the direct copying of a file to a bundle, this file of non-zero size was truncated to zero size (for example, when the table was truncated), then pgBackRest still unconditionally places such a zero-size file in the bundle, taking up space in it equal to the size of the headings, and additionally writes the original file size to the manifest.
In debug build an assertion was added, that does not allow zero-size files to be written to bundles, which leads to an error.
To solve the problem, this patch, when reading the next file, loads one buffer from the file to detect if it is zero-size. If so it marks the file as truncated and continues on to the next file.
The advantages of the solution are that, firstly, the assert will not fire on debug builds, and secondly, we will not place zero-size files in bundles, which exactly corresponds to the specification.
The patch adds the backupCopyResultTruncate value to the BackupCopyResult enumeration to use it to indicate the result when a non-zero size file is truncated to zero size during the backup process.
Alternate pages sizes can be selected at compile-time, .e.g. 4096. While compile-time settings are generally not well tested by core, some established forks such as Greenplum use them.
Bug Fixes:
* Fix regression in retries. (Reviewed by Stephen Frost. Reported by Norman Adkins, Tanel Suurhans, Jordan English, Timothée Peignier.)
* Fix recursive path remove in SFTP storage driver. (Fixed by Reid Thompson. Reviewed by Stephen Frost. Reported by Luc.)
Improvements:
* Remove support for PostgreSQL 9.3. (Reviewed by Stephen Frost.)
Documentation Features:
* Document maintainer options. (Reviewed by Stefan Fercot.)
* Update point-in-time recovery documentation for PostgreSQL >= 13.
Test Suite Improvements:
* Allow config/load unit test to run without libssh2 installed. (Contributed by Reid Thompson. Reviewed by David Steele. Suggested by Wu Ning.)
These fields were not used because of the noop so it was hard to keep them up to date. Rather than attempt to do so, just remove them and add a comment to explain why they are missing.
storageSftpPathRemove() used LIBSSH2_FX_FAILURE to determine when it was attempting to unlink a directory, but it appears that LIBSSH2_FX_PERMISSION_DENIED is also valid for this case.
Update storageSftpPathRemove() to accept either error and adjust tests.
PITR changed in PostgreSQL 13 to error when the recovery target is not reached. Update the documentation to work with PostgreSQL >= 13 as well as < 13.
Also update the versions built for RHEL and Debian since PostgreSQL 11 is now EOL.
Per our policy to support five EOL versions of PostgreSQL, 9.3 is no longer supported by pgBackRest.
Remove all logic associated with 9.3 and update the tests.
5314dbf aimed to make nested Wait objects more accurate with regard to wait time but it also got rid of the "bonus" retry that was implicit in the prior implementation. This meant that if an operation used up the entire allotted timeout, it would not be retried. Object stores especially are noisy places and some amount of retry should always be attempted. So even though removing the "bonus" retry was intended, it turned out not to be a good idea.
Instead of an implicit retry, formalize two retries in the Wait object even if the wait time has expired. Any number of retries are allowed during the wait period. Also remove waitRemaining() since it is no longer needed.
Adjust tests as needed to account for the extra timeouts.
Note that there may still be an underlying issue here that is simply being masked by retries. That is, the issue expressing was that waiting for a socket to be writable was timing out and without a retry that caused a hard error. This patch does nothing to address the source of the write timeout and perhaps there is nothing we can do about it. It does seem similar to the write issue we had with our blocking TLS implementation, but it was never clear if that was a problem with TLS, the kernel, or a bug in pgBackRest itself. It cropped up after a kernel update and we switched to non-blocking TLS to address the issue (c88684e).
The pq scripts were pretty static which had already led to a lot of code duplication in the backup test harness.
Instead allow the scripts to be built dynamically, which allows for much more flexibility and reduces duplication. For now just make these changes in the backup harness, but they may be useful elsewhere.
While we are making big changes, also update the macro/function names to hew closer to our current harness naming conventions.
Document maintainer options in a separate section with appropriate explanation and caveats.
Also make the pg-version-force option user visible now that maintainer caveats have been documented.
The reference documentation was still using a very old version of rendering from before the user guide was introduced. This was preserved in the initial C migration to reduce the diff between Perl and C for testing purposes. The old version used hard linefeeds to simulate paragraphs and reduce the amount of markup that needed to be used. In retrospect this was not a great idea.
Instead use more natural rendering that does not depend on using hard linefeeds between paragraphs.
For some reason the internal section id was included in the title. This was probably copied from another section title where it made more sense, e.g. including the option name after the title.
Also add release note missed in 1eb01622.
Migrate generation of these files from help.xml to the intermediate documentation format. This allows us to share a lot of code that is already in C and remove duplicated code in Perl. More duplicate code can be removed in Perl once man generation is migrated.
Also update the unit test harness to allow testing of modules in the doc directory.
The help source file had previously been hardcoded and now that is no longer needed.
A future commit will introduce more sources outside of the xml path.
Bug Fixes:
* Fix issue restoring block incremental without a block list. (Reviewed by Stephen Frost, Burak Yurdakul. Reported by Burak Yurdakul.)
Features:
* Add --repo-storage-tag option to create object tags. (Reviewed by Stephen Frost, Stefan Fercot, Timothée Peignier.)
* Add known hosts checking for SFTP storage driver. (Contributed by Reid Thompson. Reviewed by Stephen Frost, David Steele.)
* Support for dual stack connections. (Reviewed by Stephen Frost.)
* Add backup size completed/total to info command JSON output. (Contributed by Stefan Fercot. Reviewed by David Steele.)
Improvements:
* Multi-stanza check command. (Reviewed by Stephen Frost.)
* Retry reads of pg_control until checksum is valid. (Reviewed by Stefan Fercot, Stephen Frost.)
* Optimize WAL segment check after successful backup. (Reviewed by Stephen Frost.)
* Improve GCS multi-part performance. (Reviewed by Reid Thompson.)
* Allow archive-get command to run when stanza is stopped. (Reviewed by Tom Swartz, David Christensen, Reid Thompson.)
* Accept leading tilde in paths for SFTP public/private keys. (Contributed by Reid Thompson. Reviewed by David Steele.)
* Reload GCS credentials before renewing authentication token. (Reviewed by Stephen Frost. Suggested by Daniel Farina.)
Documentation Bug Fixes:
* Fix configuration reference example for the tls-server-address option. (Fixed by Hartmut Goebel. Reviewed by David Steele.)
* Fix command reference example for the filter option.
Test Suite Improvements:
* Allow storage/sftp unit test to run without libssh2 installed. (Contributed by Reid Thompson. Reviewed by David Steele. Suggested by Wu Ning.)
This example was broken by 24f7252. Revert to (almost) the prior code to fix this example until something better can be committed. The something better is in progress but it adds new build requirements so it is too late to include it for the release.
Technically this breaks some other examples, but they are all internal and not visible in the user-facing documentation.
It is currently possible for a block map to be written without any accompanying blocks. This happens when a file timestamp is updated but the file has not changed. On restore, this caused problems when encryption was enabled, the block map was bundled after a file that had been stored without block incremental, and both files were included in a single bundle read. In this case the block map would not be decrypted and the encrypted data was passed to blockMapNewRead() with unpredictable results. In many cases built-in retries would rectify this problem as long as delta was enabled since block maps would move to the beginning of the bundle read and be decrypted properly. If enough files were affected, however, it could overwhelm the retries and throw an error. Subsequent delta restores would eventually be able to produce a valid result.
Fix this by moving block map decryption so it works correctly no matter where the block map is located in the read. This has the additional benefit of limiting how far the block map can read so it will error earlier if corrupt. Though in this case there was no repository corruption involved, it appeared that way to blockMapNewRead() since it was reading encrypted data.
Arguably block maps without blocks should not be written at all, but it would be better to consider that as a separate change. This pattern clearly exists in the wild and needs to be handled, plus the new implementation has other benefits.
By default require a known hosts match as part of the SFTP storage driver's authentication process, i.e. repo-sftp-host-key-check-type=strict. The match is expected to be found in the default list or in a list of known hosts files provided by the user. An exception is made if a fingerprint has been manually configured with repo-sftp-host-fingerprint or repo-sftp-host-key-check-type=accept-new can be used to automatically add new hosts.
Also allow host key verification to be skipped, as before, but require the user to explicitly set this (repo-sftp-host-key-check-type=none) rather than it being the default.
This option is intended to eventually create a comprehensive report about the state of the pgBackRest configuration based on the results of the check command.
Implement a detailed report of the configuration options in the environment and configuration files. This should be useful information when debugging configuration errors, since invalid options and configurations are automatically noted. While custom config locations will not be found automatically, it will at least be clear that the config is not in a standard location.
For now keep this option internal since there is a lot of work to be done, but commit it so that it can be used when needed and tested in various environments.
Note that for now when --report is specified, the check command is not being run at all. Only the config report is generated. This behavior will be improved in the future.
This new option allows tags to be added to objects in S3, GCS, and Azure repositories.
This was fairly straightforward for S3 and Azure, but GCS does not allow tags for a simple upload using the JSON interface. If tags are required then the resumable interface must be used even if the file falls below the limit that usually triggers a resumable upload (i.e. size < repo-storage-upload-chunk-size).
This option is structured so that tags must be specified per-repo rather than globally for all repos. This seems logical since the tag keys and values may vary by service, e.g. S3 vs GCS.
These storage tags are independent of backup annotations since they are likely to be used for different purposes, e.g. billing, while the backup annotations are primarily intended for monitoring.
The prior code would only connect to the first address provided by getaddrinfo().
Instead try each address in the list. If all connections fail then wait and try them all again until timeout.
Currently a round robin approach is used where each connection attempt must fail before the next connection is attempted. This works fine, for example, when an ipv6 address has no route to the host, but will work less well when a host answers but doesn't respond in a timely fashion.
We may consider a Happy Eyeballs approach in the future, but since pgBackRest is primarily a background process, it is not clear that slightly improved response time (in the case of failed connections) is worth the extra complexity.
The prior code did one list command against the storage for each WAL segment. This led to a lot of lists and was especially inefficient when the WAL (or the majority of it) was already present.
Optimize to keep the contents of a WAL directory and use them on a subsequent search. Leave the optimizations for a single WAL segment since other places still use that mode.
On certain file systems (e.g. ext4) pg_control may appear torn if there is a concurrent write while reading the file. To prevent an invalid read, retry until the checksum matches the control data.
Special handling is required for the pg-version-force feature since the offset of the checksum is not known. In this case, scan from the default position to the end of the data looking for a checksum match. This is a bit imprecise, but better than nothing, and the chance of a random collision in the control data seems very remote considering the ratio of data size (< 512 bytes) to checksum size (4 bytes).
This was discovered and a possible solution proposed for PostgreSQL in [1]. The proposed solution may work for backup, but pgBackRest needs to be able to read pg_control reliably outside of backup. So no matter what fix is adopted for PostgreSQL, pgBackRest need retries. Further adjustment may be required as the PostgreSQL fix evolves.
[1] https://www.postgresql.org/message-id/20221123014224.xisi44byq3cf5psi%40awork3.anarazel.de
The prior example (::*) was not valid and would result in the following error:
ERROR: [049]: unable to get address for '::*': [-2] Name or service not known
Correct values are either * for IPv4 or :: for IPv6. The IPv4 value is used as the example since only one example is allowed.
The release.xml file was getting pretty unwieldy so break release notes into separate files. Also break contributors into a separate file.
In theory most of release.xml could now be generated automatically but adding a new release does not represent a serious maintenance burden, so for the time being it does not seem worth it.
The prior code avoided uploading a chunk if it was not clear whether the write was complete or not. This was primarily due to the GCS documentation being very vague on what to do in the case of a zero-size chunk.
Now chunks are uploaded as they are available. This should improve performance and also reduces the diff against a future commit that absolutely requires zero-size chunks.
The restore command can run while the stanza is stopped so it makes sense for the archive-get command to follow the same rule.
The important thing is to ensure that all commands that write to the repository are stopped when the stanza is stopped.
The documentation indicates that leading tilde file paths for public/private keys are valid but the functionality was omitted from the original implementation.
Check command now checks multiple stanzas when the stanza option is omitted.
The stanza list is extracted from the current configuration rather than scanning the repository like the info command. Scanning the repository is a problem because configuration for each stanza may not be present in the current configuration. Since this functionality is new for check there is no regression.
Add a new section to the user guide to cover multi-stanza configuration and provide additional coverage for this feature.
Also fix a small issue in the parser when an indexed option has a dependency on a non-indexed option. There were no examples of this case in the previous configuration.
This allows the service key to be updated while a command is running. The new key should be written atomically and ideally the old key should remain valid for some period of time to avoid a race condition if the old token happens to expire at the same time that the new key is being generated.
Bug Fixes:
* Preserve block incremental info in manifest during delta backup. (Reviewed by Stephen Frost. Reported by Francisco Miguel Biete Banon.)
* Fix block incremental file names in verify command. (Reviewed by Reid Thompson. Reported by Francisco Miguel Biete Banon.)
* Fix spurious automatic delta backup on backup from standby. (Reviewed by Stephen Frost. Reported by krmozejko, Don Seiler.)
* Skip recovery.signal for PostgreSQL >= 12 when recovery type=none. (Reviewed by Stefan Fercot. Reported by T.Anastacio.)
* Fix unique label generation for diff/incr backup. (Fixed by Andrey Sokolov. Reviewed by David Steele.)
* Fix time-based archive expiration when no backups are expired. (Reviewed by Stefan Fercot.)
Improvements:
* Improve performance of SFTP storage driver. (Contributed by Stephen Frost, Reid Thompson. Reviewed by David Steele.)
* Add timezone offset to info command date/time output. (Reviewed by Stefan Fercot, Philip Hurst. Suggested by Philip Hurst.)
* Centralize error handling for unsupported features. (Reviewed by Stefan Fercot.)
Documentation Improvements:
* Clarify preference to install from packages in the user guide. (Reviewed by Stefan Fercot. Suggested by dr-kd.)
When performing backup from standby the file sizes on the standby may not be equal to file sizes on the primary. This is because replication continues during the backup and by the time the file is copied from the standby it may have changed. Since we cap the size of all files copied from the standby this practically applies to truncation and in particular truncation of free space maps (at least, every case we have seen so far is an fsm). Free space maps are especially vulnerable since they are only partially replicated, which amplifies the difference between the primary and standby.
On an incremental backup it may look like the size has changed on the primary (because of the final size recorded by the standby in the prior backup) but the timestamp may not have changed on the primary and this will trigger a checksum delta for safety. While this has no impact on backup integrity, checksum delta incrementals can run much longer than regular incrementals and backup schedules may be impacted.
The solution is to preserve the original size in the manifest and use it to do the time/size check. In the case of backup from standby the original size will always be the size on the primary, which makes comparisons against subsequent file sizes on the primary consistent. Original size is only stored in the manifest when it differs from final size, so there should not be any noticeable manifest bloat.
It was possible for block incremental info to be lost if a file had been modified in such a way that block incremental would be disabled if the file were new, e.g. if the file shrank below the block incremental limit or the file timestamp regressed far enough into the past. In those cases the block incremental info would not be copied in manifestBuildIncr().
Instead always copy the block incremental info in case the file ends up being referenced to a prior backup.
The validation tests were not robust enough to catch this issue so they were improved in 1d42aed.
In the particular case that exposed this bug, a file had a timestamp that was almost four weeks in the past at full backup time. A few days later a fail over occurred and the next incremental ran on the new primary (old standby) in delta mode. The same file had a timestamp just a few hours older than in the full backup, but now four weeks older than the current backup. Block incremental was disabled for the file on initial manifest build because of its age, which meant the block incremental info was not copied into the new manifest. The delta then determined the file had not changed and referenced it to the full backup. On restore, the file appeared to be a normal file stored in a bundle but could not be decompressed because it was in fact a block incremental.
The verify command was not appending the .pgbi extension instead of the compression extension when verifying block incremental files stored outside a bundle.
Originally the idea was that verify would not need any changes (since it just examines repo-size and checksum) but at some point the new extension was added and broke that assumption.
Use backupFileRepoPathP() to generate the correct filename (Just like backup, restore, etc).
Coverity complained about time_t being cast directly to unsigned int, so instead cast the result of the operation.
We are confident in both cases that the time_t values will not be out of unsigned int range but Coverity has no way to know that.
One of these is new (introduced by 9efd5cd0) but the other one (from a9867cb0) remained unnoticed for a while, though it has not caused any production impact.
The initial implementation used simple waits when having to loop due to getting a LIBSSH2_ERROR_EAGAIN, but we don't want to just wait some amount of time, we want to wait until we're able to read or write on the fd that we would have blocked on.
This change removes all of the wait code from the SFTP driver and changes the loops to call the newly introduced storageSftpWaitFd(), which in turn checks with libssh2 to determine the appropriate direction to wait on (read, write, or both) and then calls fdReady() to perform the wait using the provided timeout.
This also removes the need to pass ioSession or timeout down into the SFTP read/write code.
In the case that no backups were expired but time-based retention was met no archive expiration would occur and the following would be logged:
INFO: time-based archive retention not met - archive logs will not be expired
In most cases this was harmless, but when retention was first met or if retention was increased, it would require one additional backup to expire earlier WAL. After that expiration worked as normal.
Even once expiration was working normally the message would continue to be output, which was pretty misleading since retention had been met, even though there was nothing to do.
Bring this code in line with count-based retention, i.e. always log what should be expired at detail level (even if nothing will be expired) and then log info about what was expired (even if nothing is expired). For example:
DETAIL: repo1: 11-1 archive retention on backup 20181119-152138F, start = 000000010000000000000002
INFO: repo1: 11-1 no archive to remove
If there were at least two full backups and the last one was expired, it was impossible to make either a differential or incremental backup without first making a new full backup. The backupLabelCreate() function identified this situation as clock skew because the new backup label was compared with label of the expired full backup.
If the new backup is differential or incremental, then its label is now compared with the labels of differential or incremental backups related to the same full backup.
Also convert a hard-coded date length to a macro.
Deleting a stanza after all the storage driver stanzas were created was causing problems because the SFTP driver is slow and the GCS driver has no server (so it threw errors). This delayed the shutdown of PostgreSQL, which for some reason caused systemctl to hang when the documentation was being built on a RHEL host.
Move the section up and add a comment about why the location is required. Also add a comment to the GCS section about its location.
This does not address the issue of systemctl hanging on RHEL container hosts but it will hopefully make it less common.
Bring PostgreSQL >= 12 behavior in line with other versions when recovery type=none.
We are fairly sure this did not work correctly when PostgreSQL 12 was released, but apparently the issue has been fixed since then. Either way, after testing we have determined that the behavior is now as expected.
Some features are conditionally compiled into pgBackRest (e.g. lz4). Previously checking to see if the feature existed was the responsibility of the feature's module.
Centralize this logic in the config/parse module to make the errors more detailed and consistent.
This also fixes the assert that is thrown when SFTP storage is specified but SFTP support is not compiled into pgBackRest.
This was not tested in 87087fac and the generated config was only valid for pushing from the primary. Also do some general cleanup.
Update the SFTP server user to be "pgbackrest" instead of "postgres".
Even though sftp-all=y now creates a valid configuration, the user guide build still fails because SFTP is too slow and operations time out (particularly starting PostgreSQL). This will need to be addressed in a future commit.
This parameter is now optional and defaults to none so there is no reason to explicitly show it in user-facing documentation.
Also make the vm parameter in ci.pl optional to be consistent with how test.pl behaves.
