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.
Features:
* Block incremental backup. (Reviewed by John Morris, Stephen Frost, Stefan Fercot.)
* SFTP support for repository storage. (Contributed by Reid Thompson. Reviewed by Stephen Frost, David Steele.)
* PostgreSQL 16 support. (Reviewed by Stefan Fercot.)
Improvements:
* Allow page header checks to be skipped. (Reviewed by David Christensen. Suggested by David Christensen.)
* Avoid chown() on recovery files during restore. (Reviewed by Stefan Fercot, Marcelo Henrique Neppel. Suggested by Marcelo Henrique Neppel.)
* Add error retry detail for HTTP retries.
Documentation Improvements:
* Add warning about using recovery type=none. (Reviewed by Stefan Fercot.)
* Add note about running stanza-create on already-created repositories.
Double spaces have fallen out of favor in recent years because they no longer contribute to readability.
We have been using single spaces and editing related paragraphs for some time, but now it seems best to update the remaining instances to avoid churn in unrelated commits and to make it clearer what spacing contributors should use.
Remove beta status and update documentation to remove beta references and warnings.
The repo-block-* sub-options have been marked internal. Most users will be best off with the default behavior and we may still decide to change these options for remove them in the future.
These were intended to allow the block list to be scanned without reading the map but were never utilized. They were left in "just in case" and because they did not seem to be doing any harm.
In fact, it is better not to have the block numbers because this allows us set the block size at a future time as long as it is a factor of the super block size. One way this could be useful is to store older files without super blocks or a map in the full backup and then build a map for them if the file gets modified in a diff/incr backup. This would require reading the file from the full backup to build the map but it would be more space efficient and we could make more intelligent decisions about block size. It would also be possible to change the block size even if one had already been selected in a prior backup.
Omitting the block numbers makes the chunking unnecessary since there is now no way to make sense of the block list without the map. Also, we might want to build maps for unchunked block lists, i.e. files that were copied normally.
The chown() was already skipped on the files restored from the repository but the same logic was not applied to the generated recovery files, probably because chown'ing a few recovery files does not have performance implications. Use the same logic for recovery files to determined if they need to be chown'd.
Ultimately this behavior is pretty hard to test, so add a fail safe into the Posix driver that will skip chown if the permissions are already as required.
These checks cause false negatives for page checksum verification when the page is encrypted because pd_upper might end up as 0 in the encrypted data. This issue is rare but reproducible given a large enough cluster.
Make these checks optional, but leave them enabled by default.
Ubuntu 18.04 will be EOL before the next release, so update to the oldest available Debian version.
Also fix one incorrect return value type, a test cast, and adjust some test timeouts.
Eliminate the boilerplate of declaring this and assigning memory to it, which is the same for the vast majority of object creations.
Keep the old version of the macro as OBJ_NEW_BASE_BEGIN() for a few exceptions in the core code and (mostly) in the tests.
Each call to lockAcquireP() passed enough information to initialize the lock system. This was somewhat inefficient and as locks become more complicated it will lead to more code duplication. Since a process can only take one type of lock it makes sense to do most of the initialization up front.
Also reduce the log level of lockRelease() since it is only called at exit and the lock will be released in any case.
Bug Fixes:
* Skip writing recovery.signal by default for restores of offline backups. (Reviewed by Stefan Fercot. Reported by Marcel Borger.)
Features:
* Block incremental backup (BETA). (Reviewed by John Morris, Stephen Frost, Stefan Fercot.)
Improvements:
* Keep only one all-default group index. (Reviewed by Stefan Fercot.)
Documentation Improvements:
* Add explicit instructions for upgrading between 2.x versions. (Contributed by Christophe Courtois. Reviewed by David Steele.)
* Remove references to SSH made obsolete when TLS was introduced.
Bug Fixes:
* Remove the distinction between maps where super block size is equal to block size and maps where they are not. In practice, maps with equal blocks are now rare and most of the optimizations can be applied directly to super blocks where the blocks are equal. This fixes a bug where a map that was created with equal size blocks and then converted to differing block sizes would generate an invalid map.
* Free reads during restore to avoid running out of file handles.
Improvements:
* Store super block sizes in the block map. This allows the final block size to be removed from the block list and provides a more optimal restore and better potential for analysis.
* Always round the super block size up to the next block size. This makes the number of blocks per super block more predictable.
* Allow super block sizes to be changed at will in the map. The first case for this is to store the reduced super block size required when the last super block is short but it could be used to dynamically change the super block size to optimize compression.
* Store a block count rather than a list of blocks in a super block. Blocks must always be sequential, though there may be an offset to the first block in a super block. This saves 11-14% on space for checksum sizes 6-7.
* In the case that all the blocks for a super block are present, and there is no offset, the block size is omitted.
This allows options to be marked as beta, which will require that the --beta option be supplied to prevent accidental usage of a beta feature.
The online and command-line documentation also show warnings when options are beta.
Block sizes are incremented when the size of the map becomes as large as a single block. This is arbitrary but it appears to give a good balance of block size vs map size.
The full backup super block size is set to minimize loss of compression efficiency since most blocks in the database will likely never be modified. For diff/incr backup super blocks, a smaller size is allowable since only modified blocks are stored. The overall savings of not storing unmodified blocks offsets the small loss in compression efficiency due to the smaller super block and allows more granular fetches during restore.
As calculated this size is not correct since it does not include the parts of prior block incrementals that are required to make the current block incremental valid. At best this could be approximated and the resulting values might be very confusing.
For now, at least, exclude this metric for block incremental backups.
xxHash is significantly faster than SHA-1 so this helps reduce the overhead of the feature.
A variable number of bytes are used from the xxHash depending on the block size with a minimum of six bytes for the smallest block size. This keeps the maps smaller while still providing enough bits to detect block changes.
Small blocks sizes can lead to reduced compression efficiency, so allow multiple blocks to be compressed together in a super block. The disadvantage is that the super block must be read sequentially to retrieve blocks. However, different super block sizes can be used for different backup types, so the full backup super block sizes are large for compression efficiency and diff/incr are smaller for retrieval efficiency.
Forks may update pg_control version or WAL magic without affecting the structures that pgBackRest depends on.
This option forces pgBackRest to treat a cluster as the specified version when it cannot be automatically identified.
When restoring an offline backup on PostgreSQL >= 12, skip writing recovery.signal by default since this will error if the backup was made with wal_level=minimal. If the user explicitly sets the type option to something other than none, then write recovery.signal as usual since it is possible to do Point-In-Time-Recovery from an offline backup as long as wal_level was not minimal.
Raw encryption was already being used for block incremental. This commit adds raw compression to block incremental where possible (see da918587).
Raw compression/encryption is also added to bundling for a backup set when block incremental is enabled on the full backup. This prevents a break in backward compatibility since block incremental is not backward compatible.
Raw format saves 12 bytes of header for gzip and 4 bytes of checksum for lz4 (plus CPU overhead). This may not seem like much, but over millions of small files or incremental blocks can really add up. Even though it may be a relatively small percentage of the overall backup size it is still objectively a large amount of data.
Use raw format for protocol compression to exercise the feature.
Raw compression format will be added to bundling and block incremental in a followup commit.
It is possible for a group index to be created for an option that is later found to not meet dependencies. In this case all values would be default leading to a phantom group, which can be quite confusing.
Remove group indexes that are all default (except the final one) and make sure the key for the final all default group index is 1.
Add an explicit statement that there is nothing special to do when upgrading between 2.x versions.
Leave the previous paragraph about the default location that changed between 2.00 and 2.02, as it is more a matter of transitioning from 1.x to 2.x.
The code is not completely reflowed yet so there are some cases that uncrustify will not catch. The formatting will be improved over time.
Some block of code require special formatting so have been surrounded with the {uncrustify-off}/{uncrustify-on} markers. These exceptions should be kept to a minimum.
Add --code-format (to reformat code) and --code-format-check (to check formatting) to test.pl.
Add a CI test that will check code formatting. Code must be correctly formatted before it can be merge to integration.
Add documentation to the coding standards for code formatting.
Improvements:
* Remove support for PostgreSQL 9.0/9.1/9.2. (Reviewed by Stefan Fercot.)
* Restore errors when no backup matches the current version of PostgreSQL. (Contributed by Stefan Fercot. Reviewed by David Steele. Suggested by Soulou.)
* Add compress-level range checking for each compress-type. (Reviewed by Stefan Fercot. Suggested by gkleen, ViperRu.)
Documentation Improvements:
* Add warning about enabling "hierarchical namespace" on Azure storage. (Reviewed by Stefan Fercot. Suggested by Vojtech Galda, Pluggi, asjonos.)
* Add replacement for linefeeds in monitoring example. (Reviewed by Stefan Fercot. Suggested by rudonx, gmustdie, Ivan Shelestov.)
* Clarify target-action behavior on various PostgreSQL versions. (Contributed by Chris Bandy. Reviewed by David Steele, Anton Kurochkin, Stefan Fercot. Suggested by Anton Kurochkin, Chris Bandy.)
* Updates and clarifications to index page. (Reviewed by Stefan Fercot.)
* Add dark mode to the website. (Suggested by Stephen Frost.)
If this feature is enabled expire will fail since directories need to be deleted separately.
Ideally we would add support for this feature but for now we'll just document the issue.
The primary goal of the block incremental backup is to save space in the repository by only storing changed parts of a file rather than the entire file. This implementation is focused on restore performance more than saving space in the repository, though there may be substantial savings depending on the workload.
The repo-block option enables the feature (when repo-bundle is already enabled). The block size is determined based on the file size and age. Very old or very small files will not use block incremental.
The callbacks in iniLoad() made the downstream code more complicated than it needed to be so use an iterator model instead.
Combine the two functions that were used to load the ini data to remove code duplication. In theory it would be nice to use iniValueNext() in the config/parse module rather than loading a KeyValue store but this would mean a big change to the parser, which does not seem worthwhile at this time.
Allocating memory made these functions simpler but it meant that memory was leaking into the calling context when logging was enabled. It is not clear that this was an issue but it seems that trace level logging could result it a lot of memory usage depending on the use case.
This also makes it possible to audit allocations returned to the calling context, which will be done in a followup commit.
Also rename objToLog() to objNameToLog() since it seemed logical to name the new function objToLog().
This should make it a little clearer what the variable (VR) macros are doing since the declaration/definition cannot both be set to extern (but functions can).
Splitting the variable macros out also allows them to be changed in the future with little churn, while changing the function macro creates a large amount of churn.
This is immediately useful because it will detect any extern'd functions or variables that are not being used. It also detects functions or variables that are declared but not defined.
If a FV/VR_EXTERN macro is missing it will be detected either because of a mismatch in the declaration/definition or because a new defined symbol will appear in the nm test.
Eventually the unity build will be used to create a more optimized pgbackrest binary but that will need to wait.
It is probably not a good idea to restore the latest backup when it was not made from the current PostgreSQL version. If there is no backup after a stanza-upgrade then replicas might be built with a prior version leading to failures.
Add an error in this case if the latest backup would be used, i.e. --set or --type=time/lsn is not specified.
In particular the section about other backup solutions not supporting parallel processing was no longer accurate, so reword it.
Also update some other sections that used older nomenclature, had awkward wording, or needed clarification.
The behavior of pause depends on the hot_standby parameter and the PostgreSQL version so mention both.
This behavior has been verified on PostgreSQL 9.6–15. PostgreSQL 12 is an inflection point because the behavior of an unset recovery_target_action with hot_standby=off changed in https://git.postgresql.org/gitweb/?p=postgresql.git;h=2dedf4d9a899b36d1a8ed29be5efbd1b31a8fe85.
The copy command was converting \n to a linefeed, which the json conversion did not like. In a healthy repository there won't be any linefeeds but certain errors can contain them.
Fix by loading into a text field and then replacing the linefeed when converting to jsonb.
The prior range checking was done based on the valid values for gz. While this worked it was a subset of what is available for lz4 and zst.
Allow the range to be specified for each compress-type. Adding this functionality to the parse module would be a better solution but that is a bigger project than this fix deserves, at least for now.
Calculate a checksum of the data stored in the repository when a file is transformed (e.g. compressed). This allows resume and verify to operate without needing to decompress/decrypt the data.
This can also be used to verify more complex formats such as block incremental and allow backups from the repository without needing to decompress the data to verify the checksum.
Add some basic encrypted tests to maintain coverage. These will be expanded in a future commit.
Manifest checksums were stored as hex-encoded strings due to legacy compatibility with Perl. Storing the checksums as binary in memory uses half the space and avoids many conversions.
There is no change to the on-disk manifest format which stores the checksum as a hex-encoded string.
Our new policy is to support ten versions of PostgreSQL, the five supported releases and the last five EOL releases. As of PostgreSQL 15, that means 9.0/9.1/9.2 are no longer supported by pgBackRest.
Remove all logic associated with 9.0/9.1/9.2 and update the tests.
Document the new support policy.
Update InfoPg to read/write control versions for the history in backup.info, since we can no longer rely on the mappings being available. In theory this could have been an issue after removing 8.3/8.4 if anybody was using a version that old.
Bug Fixes:
* Fix missing reference in diff/incr backup. (Reviewed by Stefan Fercot. Reported by Marcel Borger, ulfedf, jaymefSO.)
Improvements:
* Add hint when an option is specified without an index. (Reviewed by Stefan Fercot.)
When loading prior manifests without the new reference list, the code failed to add the current backup to the reference list. Since the current backup is never explicitly referenced, building references from the file list was not sufficient to generate a complete list.
The main problem here was a bad test, fixed in 28f6604. This masked the issue and prevented it from being found. Now it is clear in the test that the current label is missing from the reference list.
Fix by adding the current label to the reference list if a reference list is not stored in the manifest.
Hopefully this will make it a little clearer to the user what is wrong when they specify an indexed option without an index.
Also fix an ambiguous use of cfgParseOptionP(). The prior code worked in that it set prefixMatch = true but it was not very readable.
