On Fri, Apr 4, 2025 at 4:41 PM Ashutosh Bapat <ashutosh.bapat....@gmail.com> wrote: > > On Thu, Apr 3, 2025 at 10:44 PM Alvaro Herrera <alvhe...@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.
Forgot to mention that I made sure that the test is still doing its work correct by reverting fd41ba93e463 and checking that it brings back the failure. Also tested export'ing LC_MONETARY to make sure that the locales in original and restored database remain the same. -- Best Wishes, Ashutosh Bapat
