Обсуждение: pg_basebackup test coverage
$SUBJECT is not great. The options to pg_basebackup that are not tested by any part of the regression test suite include the single-letter options rlzZdUwWvP as well as --no-estimate-size. It would probably be good to fix as much of this as we can, but there are a couple of cases I think would be particularly good to cover. One is 'pg_basebackup -Ft -Xnone -D -', which tries to write the output as a single tar file on standard output, injecting the backup_manifest file into the tar file instead of writing it out separately as we normally would. This case requires special handling in a few places and it would be good to check that it actually works. The other is the -z or -Z option, which produces a compressed tar file. Now, there's nothing to prevent us from running commands like this from the pg_basebackup tests, but it doesn't seem like we could really check anything meaningful. Perhaps we'd notice if the command exited non-zero or didn't produce any output, but it would be nice to verify that the resulting backups are actually correct. The way to do that would seem to be to extract the tar file (and decompress it first, in the -z/-Z case) and then run pg_verifybackup on the result and check that it passes. However, as far as I can tell there's no guarantee that the user has 'tar' or 'gunzip' installed on their system, so I don't see a clean way to do this. A short (but perhaps incomplete) search didn't turn up similar precedents in the existing tests. Any ideas? -- Robert Haas EDB: http://www.enterprisedb.com
On Thu, Dec 10, 2020 at 12:32:52PM -0500, Robert Haas wrote: > It would probably be good to fix as much of this as we can, but there > are a couple of cases I think would be particularly good to cover. One > is 'pg_basebackup -Ft -Xnone -D -', which tries to write the output as > a single tar file on standard output, injecting the backup_manifest > file into the tar file instead of writing it out separately as we > normally would. This case requires special handling in a few places > and it would be good to check that it actually works. The other is the > -z or -Z option, which produces a compressed tar file. > > Now, there's nothing to prevent us from running commands like this > from the pg_basebackup tests, but it doesn't seem like we could really > check anything meaningful. Perhaps we'd notice if the command exited > non-zero or didn't produce any output, but it would be nice to verify > that the resulting backups are actually correct. The way to do that > would seem to be to extract the tar file (and decompress it first, in > the -z/-Z case) and then run pg_verifybackup on the result and check > that it passes. However, as far as I can tell there's no guarantee > that the user has 'tar' or 'gunzip' installed on their system, so I > don't see a clean way to do this. A short (but perhaps incomplete) > search didn't turn up similar precedents in the existing tests. > > Any ideas? I would probe for the commands with "tar -cf- anyfile | tar -xf-" and "echo foo | gzip | gunzip". If those fail, skip any test that relies on an unavailable command.
On Thu, Dec 10, 2020 at 10:53:51PM -0800, Noah Misch wrote: > On Thu, Dec 10, 2020 at 12:32:52PM -0500, Robert Haas wrote: >> Now, there's nothing to prevent us from running commands like this >> from the pg_basebackup tests, but it doesn't seem like we could really >> check anything meaningful. Perhaps we'd notice if the command exited >> non-zero or didn't produce any output, but it would be nice to verify >> that the resulting backups are actually correct. The way to do that >> would seem to be to extract the tar file (and decompress it first, in >> the -z/-Z case) and then run pg_verifybackup on the result and check >> that it passes. However, as far as I can tell there's no guarantee >> that the user has 'tar' or 'gunzip' installed on their system, so I >> don't see a clean way to do this. A short (but perhaps incomplete) >> search didn't turn up similar precedents in the existing tests. >> >> Any ideas? > > I would probe for the commands with "tar -cf- anyfile | tar -xf-" and "echo > foo | gzip | gunzip". If those fail, skip any test that relies on an > unavailable command. Why don't you just use Archive::Tar [1] instead of looking for some system commands? Combining list_files() with extract(), it is possible to extract an archive, with or without compression, without hoping for an equivalent to exist on Windows. That would not be the first time either that there is a TAP test that skips some tests if a module does not exist. See for example what psql does with IO::Pty. [1]: https://metacpan.org/pod/Archive::Tar -- Michael
Вложения
On Fri, Dec 11, 2020 at 3:04 AM Michael Paquier <michael@paquier.xyz> wrote: > Why don't you just use Archive::Tar [1] instead of looking for some system > commands? Combining list_files() with extract(), it is possible to > extract an archive, with or without compression, without hoping for an > equivalent to exist on Windows. That would not be the first time > either that there is a TAP test that skips some tests if a module does > not exist. See for example what psql does with IO::Pty. Well, either this or Noah's method has the disadvantage that not everyone will get the benefit of the tests, and that those who wish to get that benefit must install more stuff. But, all three of the computers I have within arm's reach (yeah, I might have a problem) have Archive::Tar installed, so maybe it's not a big concern in practice. I am slightly inclined to think that the perl package approach might be better than shell commands because perhaps it is more likely to work on Windows, but I'm not positive. -- Robert Haas EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes:
> Well, either this or Noah's method has the disadvantage that not
> everyone will get the benefit of the tests, and that those who wish to
> get that benefit must install more stuff. But, all three of the
> computers I have within arm's reach (yeah, I might have a problem)
> have Archive::Tar installed, so maybe it's not a big concern in
> practice.
FWIW, it looks to me like Archive::Tar is part of the standard Perl
installation on both RHEL and macOS, so it's probably pretty common.
> I am slightly inclined to think that the perl package
> approach might be better than shell commands because perhaps it is
> more likely to work on Windows, but I'm not positive.
Yeah, that makes sense to me too.
regards, tom lane
On Fri, Dec 11, 2020 at 12:23:10PM -0500, Robert Haas wrote: > On Fri, Dec 11, 2020 at 3:04 AM Michael Paquier <michael@paquier.xyz> wrote: > > Why don't you just use Archive::Tar [1] instead of looking for some system > > commands? Combining list_files() with extract(), it is possible to > > extract an archive, with or without compression, without hoping for an > > equivalent to exist on Windows. That would not be the first time > > either that there is a TAP test that skips some tests if a module does > > not exist. See for example what psql does with IO::Pty. > > Well, either this or Noah's method has the disadvantage that not > everyone will get the benefit of the tests, and that those who wish to > get that benefit must install more stuff. But, all three of the > computers I have within arm's reach (yeah, I might have a problem) > have Archive::Tar installed, so maybe it's not a big concern in > practice. I am slightly inclined to think that the perl package > approach might be better than shell commands because perhaps it is > more likely to work on Windows, but I'm not positive. Outside Windows, Archive::Tar is less portable. For example, in the forty-two systems of the GCC Compile Farm, five lack Archive::Tar. (Each of those five is a CentOS 7 system. Every system does have tar, gzip and gunzip.) Either way is fine with me. Favoring Archive::Tar, a Windows-specific bug is more likely than a CentOS/RHEL-specific bug. Favoring shell commands, they can catch PostgreSQL writing a tar file that the system's tar can't expand.