Re: Test to dump and restore objects left behind by regression

On Fri, Apr 4, 2025 at 4:41 PM Ashutosh Bapat
<ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
>
> On Thu, Apr 3, 2025 at 10:44 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)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