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.
There seems to be a shortage of arm64 hosts because queue times have been steadily increasing over the last few weeks. It can now take several hours to get an arm64 test queued, which makes it difficult to get development done.
Disable for the time being and hope the resource issue gets resolved in the future.
The leak kind is usually definite but sometimes flaps to possible. For stability purposes accept any leak kind.
Note that this is a leak in a specific version of libssh2 and not a bug in pgBackRest.
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.
It is possible for the server to receive notifications for children that were not spawned by it, e.g. when running as the root process of a container.
Only assert when the child process is found in our process list. Otherwise, the code may be something unexpected.
Since this is an assert there should not be any real world consequences either way, but the issue has caused problems in testing.
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.
The core Exec object is efficient but geared toward the specific needs of core and not ease-of-use as required for build, documentation, and testing.
execOne() works similarly to system() except that it automatically redirects stderr to stdout and captures the output.
This has never been a problem for performance tests since they do not call functions that log at info level or above, but the upcoming integration tests may do so. In any case it is better to disable this functionality outside of unit tests.
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).
The output combined a representation of the files/paths/links in the manifest along with output of what was validated on disk. This was redundant and made maintenance of the tests difficult, especially with all the quoting in the manifest output (which also made it hard to search for output).
Instead use primarily the output created during validation and add fields from the manifest that were missing. Exclude paths from the output when there are files in the path since the path is then implied.
One major change here is that checksums are no longer output. This makes it easier to write tests that work on multiple architectures and all checksums are already verified during validation.
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.
'pgbackrest help help' just displayed the help overview, rather than display help for the help command.
Fix this by making sure the command is set and routing correctly in the help command.
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.)
Tablespaces were enabled for tests that did not have tablespaces, resulting in tablespace_map being present in backups even when it was not needed.
Instead of specifying if tablespaces are present, automatically detect tablespaces in hrnBackupPqScript().
Long scripts followed by a number of tests are really challenging to debug and update.
Instead, break up the scripts to be inline with the tests that they drive. This should make maintenance of the tests much simpler.
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.
This warning has had a note since the C migration that it should be moved below the backup file log message, so do that.
Also update the warning message a bit to correct for tense. This message was likely in a different place originally.
Having two enum values for file copy makes things a bit more complicated than they need to be (especially in an upcoming commit).
Instead add a flag to indicate that the repository file was invalid since the only purpose is to trigger a warning message.
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.
c9703b35 added logging for file size changes during backup. Later 5ed6f8df added the sizeOriginal member to ManifestFile, which arguably is better to use for logging rather than size before backup since it will always contain the original size. Size could in theory be modified for deduplication purposes.
Update logging to use sizeOriginal.
If delta is not enabled, then the timestamp is used to determine if a file has changed. If the timestamp changes but the file is the same then the prior map will be stored unchanged in the new backup. This is not quite as bad as storing the entire file but it is obviously not ideal.
This will be fixed in a future commit, but add the test now to show the current behavior.
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.
