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

From: Alvaro Herrera <[email-masked]>
Date: Thu, 3 Apr 2025 10:20:09 +0200

On 2025-Apr-03, Ashutosh Bapat wrote:

> Looks like the problem is in the test itself as pointed out by Jeff in
> [1]. PFA patch fixing the test and enabling statistics back.

Thanks, pushed.

> A note about variable name changes and introduction of new variables.
> We run step 2 between 1 and 3 so that autovacuum gets a chance to run
> on the old cluster and update statistics. Autovacuum run is not
> necessary but useful here. Before these changes all the cluster
> initializations were using the same variables @initdb_params and
> %node_params. However with these changes, we modify the variable in
> step 2 and then again require original values in step 4. So I have
> used two sets of variables prefixed with old_ and new_ for clusters
> created in 1st step and 2nd step respectively. 4th step uses the
> variables with prefix old_. I think this change eliminates confusion
> caused by using same variables with different values.

This was a good change, thanks.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"No es bueno caminar con un hombre muerto"

---

From: Ashutosh Bapat <[email-masked]>
Date: Thu, 3 Apr 2025 14:44:59 +0530

> > [1]. PFA patch fixing the test and enabling statistics back.

Thanks.

-- 
Best Wishes,
Ashutosh Bapat

---

From: Andres Freund <[email-masked]>
Date: Thu, 3 Apr 2025 09:40:49 -0400

Hi,

> > [1]. PFA patch fixing the test and enabling statistics back.

Since then the pg_upgrade tests have been failing on skink/valgrind, due to
exceeding the already substantially increased timeout.

https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=skink&dt=2025-04-03%2007%3A06%3A19&stg=pg_upgrade-check
(note that there are other issues in that run)

284/333 postgresql:pg_upgrade / pg_upgrade/002_pg_upgrade                               TIMEOUT        10000.66s   killed by signal 15 SIGTERM

[10:38:19.815](16.712s) ok 20 - check that locales in new cluster match original cluster
...
# Running: pg_dumpall --no-sync --dbname port=15114 host=/tmp/bh_AdT5uvQ dbname='postgres' --file /home/bf/bf-build/skink-master/HEAD/pgsql.build/testrun/pg_upgrade/002_pg_upgrade/data/tmp_test_gp2G/dump2.sql
death by signal at /home/bf/bf-build/skink-master/HEAD/pgsql/src/test/perl/PostgreSQL/Test/Cluster.pm line 181.
...
[10:44:11.720](351.905s) # Tests were run but no plan was declared and done_testing() was not seen.

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.

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.

/me and checks prices for increasing the size of skink's host.

Greetings,

Andres