On Tue, Sep 13, 2022 at 05:38:47PM -0400, Tom Lane wrote: > is something I tried along the way to diagnosing the problem, and > it turns out to have exactly zero effect. The $tempdir is some > temporary subdirectory of tmp_check that will get nuked at the end > of the TAP test no matter what. So these rmtrees are merely making > the evidence disappear a bit faster; it will anyway.
FWIW, I just stick a die() in the middle of the code path when I want to look at specific results. Similar method, same result. > # Test background stream process terminating before the basebackup has > > which is not real clean, since then the files get left behind even > on success, which I doubt we want either. Another thing that could be done here is to use the same base location as the cluster nodes aka $PostgreSQL::Test::Utils::tmp_check. That would mean storing in a repo more data associated to the base backups after a fresh run, though. I am not sure that small machine would like this accumulation in a single run even if disk space is cheap these days. > Anyway, I have no objection to dropping the rmtrees, since they're > pretty useless as the code stands. Just wanted to mention this > issue for the archives. I see more ways to change the existing behavior, so for now I have left that untouched. And so, I have spent a couple of hours torturing the patch, applying it after a few tweaks and CI runs: - --without-zlib was causing a failure in the pg_basebackup tests as we have a few tests that parse and validate a set of invalid specs for the client-side and server-side compression. With zlib around, the tests and their expected results are unchanged, that's just a consequence of moving the assignment of a default level much earlier. - pg_basebackup was triggering an assertion when using client-lz4 or client-zstd as we use the directory method of walmethods.c. In this case, we just support zlib as compression and enforce no compression when we are under lz4 or zstd. This came from an over-simplification of the code. There is a gap in the testing of pg_basebackup, actually, because we have zero tests for LZ4 and zstd there. - The documentation of the replication protocol needed some adjustments for the default comporession levels. The buildfarm is green so I think that we are good. I have closed the open item. You have mentioned upthread an extra thing about the fact that we pass down 0 to deflateParams(). This is indeed wrong and we are lucky that Z_DEFAULT_STRATEGY maps to 0. Better to fix and backpatch this one down to where gzip compression has been added to walmethods.c.. I'll just do that in a bit after double-checking the area and the other routines. -- Michael
signature.asc
Description: PGP signature
