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.
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.
Older versions of ninja may fail to rebuild correctly when changes are made to the configuration. In this case there is an automatic retry but the unexpected log output would cause the test to fail.
For tests that are expected to succeed, check that the log is empty but also accept a retry message as long as the test does eventually succeed.
Add a new harness function, harnessLogResultEmptyOrContains(), to make this work and also clean up some adjacent code.
If the same port is reused too quickly bind may fail with this error:
FileOpenError: unable to bind socket: [98] Address already in use
We specify SO_REUSEADDR when creating the socket but apparently this is not always enough if the port is reused very rapidly.
Fix this (hopefully) by using a unique port for each test that needs one. This does in theory limit the number of tests that can use ports, but we allow 768 per test, whereas the test that uses the most ports is common/io-tls with 4.
This allows analysis of coverage failures that only happen in CI. It is not ideal since the report needs to be copied from the log output into an HTML file where it can be viewed, but better than nothing.
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.)
The backupFile() tests were written before the bulk of the backup command had been migrated to C. Some of them have been migrated to the complete backup tests, but others were left because there was no way to make changes to files during a backup.
Now that we have the backup script harness introduced in 337da35a it is now possible to migrate all the tests. The new tests are better because they not only test backupFile() but all the functions upstream and downstream of it.
This behavior violates an assertion but is completely possible with the current implementation. This behavior will be fixed in a future commit, but for now at least test how it works correctly and remove the assertion so the test runs without error.
Also add a new harness that allows changes during the backup to be scripted.
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.
All storage interface read methods should return actual read bytes. This patch refactors storageReadRemote() to eliminate duplicated code and return actual read bytes. The return value is calculated as the number of bytes written to the passed buffer.
This is technically a bug but does not express as an issue currently because this return value is not being used. It will be used in the future, though, so it needs to be fixed.
David Steele
Reid Thompson
Reid Thompson
Robert Donovan
Georgy Shelkovy
Viktor Kurilko
Georgy Shelkovy