Re: Test to dump and restore objects left behind by regression
Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
From: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Andres Freund <andres@anarazel.de>, Daniel Gustafsson <daniel@yesql.se>,
Tom Lane <tgl@sss.pgh.pa.us>, Michael Paquier <michael@paquier.xyz>, vignesh C <vignesh21@gmail.com>, Peter Eisentraut <peter@eisentraut.org>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-04-04T11:11:51Z
Lists: pgsql-hackers
Attachments
- 0001-Reduce-time-taken-by-002_pg_upgrade-test-to-20250404.patch (text/x-patch) patch 0001
- 0002-Turn-off-log_statement-to-save-CPU-cycles-20250404.patch (text/x-patch) patch 0002
On Thu, Apr 3, 2025 at 10:44 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote: > > On 2025-Apr-03, Andres Freund wrote: > > > I've increased the timeout even further, but I can't say that I am happy about > > the slowest test getting even slower. Adding test time in the serially slowest > > test is way worse than adding the same time in a concurrent test. > > Yeah. We discussed strategies to shorten the runtime, but the agreement > upthread was that we'd look for more elaborate ways to do that > afterwards. As I mentioned, I can see adding something like > PG_TEST_EXCLUDE that we could use to suppress this test on slow hosts. > Would that work for you? > > (We also discussed the fact that this was part of 002_pg_upgrade.pl > instead of being elsewhere. The reason is that this depends on the > regression tests having run, and this is the only TAP test that does > that. Well, this one and 027_stream_regress.pl which is even slower.) > > > I suspect that the test will go a bit faster if log_statement weren't forced > > on, printing that many log lines, with context, does make valgrind slower, > > IME. But Cluster.pm forces it to on, and I suspect that putting a global > > log_statement=false into TEMP_CONFIG would have it's own disadvantages. > > I'm sure we can make this change as well somehow, overridding the > setting just 002_pg_upgrade.pl, as attached. I don't think it's > relevant for this particular test. The log files go from 21 MB to > 2.4 MB. It's not nothing ... It doesn't show any time improvement on my laptop, but it may improve valgrind timing. My valgrind setup is broken, trying to fix it and run it. I have included this as 0002 in the attached patchset. 0001 is an attempt to reduce runtime of the test by not setting up a cluster for restoring the database. Instead the test uses the upgraded node as the target. This works well since we expect the old node and new node to be running the same version and default install. The only unpleasantness is 1. dump and restore phases are spatially and temporally separated 2. The upgraded regression database needs to be renamed to save its state for diagnosis, if required. But as a result this saves 3 seconds on my laptop. Earlier we saw that the test added 9 seconds on my laptop and we gained back 3 seconds; doesn't seem bad. It will show a significant difference in valgrind run. -- Best Wishes, Ashutosh Bapat
Commits
-
Hide expensive pg_upgrade test behind PG_TEST_EXTRA
- d185161e4739 18.0 landed
- 37fc1803cc12 19 (unreleased) landed
-
Set log_statement=none in t/002_pg_upgrade.pl
- 64fba9c61787 18.0 landed
-
002_pg_upgrade.pl: Move pg_dump test code for better stability
- 8806e4e8deb1 18.0 landed
-
002_pg_upgrade.pl: rename some variables for clarity
- abe56227b2e2 18.0 landed
-
Verify roundtrip dump/restore of regression database
- 172259afb563 18.0 landed
-
Refactor TAP test code for file comparisons into new routine in Utils.pm
- 169208092f5c 18.0 landed
-
Virtual generated columns
- 83ea6c54025b 18.0 cited
-
Put generated_stored test objects in a schema
- 894be11adfa6 18.0 cited
-
Introduce "builtin" collation provider.
- 2d819a08a1cb 17.0 cited
-
Revert "Improve compression and storage support with inheritance"
- 74563f6b9021 17.0 cited