Re: [HACKERS] Need a builtin way to run all tests faster manner

[Tom Lane]

Peter Eisentraut  writes:
> make check-world -O -j6 PROVE_FLAGS=-j6
> 2 min 34 seconds
> Nice!

One thing I've noticed with parallel check-world is that it's possible
for a sub-job to fail and for the resulting complaint from make to scroll
offscreen before everything stops, leaving all the visible output being
from another sub-job that completed just fine.  If you don't look VERY
closely, or check $?, you can easily be misled into thinking the tests
all passed.

I think it'd be a good idea to fix check-world so that there's a positive
printout at the end of a successful run, along the lines of the

+@echo "All of PostgreSQL successfully made. Ready to install."

we have for the "all" target.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Peter Eisentraut]

make check-world -O -j6 PROVE_FLAGS=-j6

2 min 34 seconds

Nice!

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Andres Freund]

On 2017-03-13 00:35:06 -0400, Tom Lane wrote:
> Andres Freund  writes:
> > On 2017-03-11 22:14:07 -0500, Tom Lane wrote:
> >> This looks generally sane to me, although I'm not very happy about folding
> >> the "$(MKDIR_P) output_iso" call into pg_isolation_regress_check --- that
> >> seems weird and unlike the way it's done for the regular regression test
> >> case.
> 
> > Yea, not super happy about that either - alternatively we could fold it
> > into pg_regress.
> 
> Yeah, teaching pg_regress to auto-create the --temp-instance directory
> seems perfectly sane from here.

I was thinking about outputdir, not temp-instance.  The latter is
already created:

/* make the temp instance top directory */
make_directory(temp_instance);

Attached is an updated patch that creates outputdir if necessary.  This
is possibly going to trigger a time-to-check-time-to-use coverity
warning, but the rest of pg_regress does if(!exists) mkdir() type logic,
so I did the same.

Besides the pg_regress change, the only thing I've changed is to remove
the in-line "$(MKDIR_P) output_iso && \".

- Andres
>From a63c61ca828d186a374d05db264f95b5b07187f9 Mon Sep 17 00:00:00 2001
From: Andres Freund 
Date: Sat, 11 Mar 2017 15:03:18 -0800
Subject: [PATCH] Improve isolation regression tests infrastructure.

Previously if a directory had both isolationtester and plain
regression tests, they couldn't be run in parallel, because they'd
access the same files/directories.  That, so far, only affected
contrib/test_decoding.

Rather than fix that locally in contrib/test_decoding improve
pg_regress_isolation_[install]check to use separate resources from
plain regression tests.

That requires a minor change in pg_regress, namely that the
--outputdir is created if not already existing, that seems like good
idea anyway.

Use the improved helpers even where previously not used.

Author: Tom Lane and Andres Freund
Discussion: https://postgr.es/m/20170311194831.vm5ikpczq52c2...@alap3.anarazel.de
---
 contrib/test_decoding/.gitignore |  5 +++--
 contrib/test_decoding/Makefile   |  7 +--
 src/Makefile.global.in   | 29 ++--
 src/test/isolation/.gitignore|  5 ++---
 src/test/isolation/Makefile  |  8 
 src/test/modules/snapshot_too_old/.gitignore |  2 +-
 src/test/modules/snapshot_too_old/Makefile   |  6 +++---
 src/test/regress/pg_regress.c|  6 +-
 8 files changed, 42 insertions(+), 26 deletions(-)

diff --git a/contrib/test_decoding/.gitignore b/contrib/test_decoding/.gitignore
index 1f95503494..b4903eba65 100644
--- a/contrib/test_decoding/.gitignore
+++ b/contrib/test_decoding/.gitignore
@@ -1,5 +1,6 @@
 # Generated subdirectories
 /log/
-/isolation_output/
-/regression_output/
+/results/
+/output_iso/
 /tmp_check/
+/tmp_check_iso/
diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile
index d2bc8b8350..6c18189d9d 100644
--- a/contrib/test_decoding/Makefile
+++ b/contrib/test_decoding/Makefile
@@ -5,7 +5,7 @@ PGFILEDESC = "test_decoding - example of a logical decoding output plugin"
 
 # Note: because we don't tell the Makefile there are any regression tests,
 # we have to clean those result files explicitly
-EXTRA_CLEAN = $(pg_regress_clean_files) ./regression_output ./isolation_output
+EXTRA_CLEAN = $(pg_regress_clean_files)
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
@@ -42,11 +42,8 @@ REGRESSCHECKS=ddl xact rewrite toast permissions decoding_in_xact \
 	spill slot
 
 regresscheck: | submake-regress submake-test_decoding temp-install
-	$(MKDIR_P) regression_output
 	$(pg_regress_check) \
 	--temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \
-	--temp-instance=./tmp_check \
-	--outputdir=./regression_output \
 	$(REGRESSCHECKS)
 
 regresscheck-install-force: | submake-regress submake-test_decoding temp-install
@@ -56,10 +53,8 @@ regresscheck-install-force: | submake-regress submake-test_decoding temp-install
 ISOLATIONCHECKS=mxact delayed_startup ondisk_startup concurrent_ddl_dml
 
 isolationcheck: | submake-isolation submake-test_decoding temp-install
-	$(MKDIR_P) isolation_output
 	$(pg_isolation_regress_check) \
 	--temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \
-	--outputdir=./isolation_output \
 	$(ISOLATIONCHECKS)
 
 isolationcheck-install-force: all | submake-isolation submake-test_decoding temp-install
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 831d39a9d1..e7862016aa 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -545,14 +545,31 @@ TEMP_CONF += --temp-config=$(TEMP_CONFIG)
 endif
 
 pg_regress_locale_flags = $(if $(ENCODING),--encoding=$(ENCODING)) $(NOLOCALE)
+pg_regress_clean_files = results/ regression.diffs regression.out tmp_check/ tmp_check_iso/ log/ output_iso/
 
-pg_regress_check = $(with_temp_install) 

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Andrew Dunstan]

On 03/13/2017 07:21 PM, Andrew Dunstan wrote:
>
> On 03/13/2017 12:35 AM, Tom Lane wrote:
>> Andres Freund  writes:
>>> On 2017-03-11 22:14:07 -0500, Tom Lane wrote:
 This looks generally sane to me, although I'm not very happy about folding
 the "$(MKDIR_P) output_iso" call into pg_isolation_regress_check --- that
 seems weird and unlike the way it's done for the regular regression test
 case.
>>> Yea, not super happy about that either - alternatively we could fold it
>>> into pg_regress.
>> Yeah, teaching pg_regress to auto-create the --temp-instance directory
>> seems perfectly sane from here.
>
> w.r.t. $subject, I thought it might be useful to get some recent stats
> from the buildfarm. Results are below. The bin checks dwarf everything
> else. Upgrade checks and isolation check are other items of significant
> cost. Upgrade checks could be significantly shortened if we could avoid
> rerunning the regression tests.


Sorry for line wrapping. Better formatted here:


cheers

andrew

-- 
Andrew Dunstanhttps://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Andrew Dunstan]

On 03/13/2017 12:35 AM, Tom Lane wrote:
> Andres Freund  writes:
>> On 2017-03-11 22:14:07 -0500, Tom Lane wrote:
>>> This looks generally sane to me, although I'm not very happy about folding
>>> the "$(MKDIR_P) output_iso" call into pg_isolation_regress_check --- that
>>> seems weird and unlike the way it's done for the regular regression test
>>> case.
>> Yea, not super happy about that either - alternatively we could fold it
>> into pg_regress.
> Yeah, teaching pg_regress to auto-create the --temp-instance directory
> seems perfectly sane from here.


w.r.t. $subject, I thought it might be useful to get some recent stats
from the buildfarm. Results are below. The bin checks dwarf everything
else. Upgrade checks and isolation check are other items of significant
cost. Upgrade checks could be significantly shortened if we could avoid
rerunning the regression tests.

cheers

andrew


pgbfprod=> select s.branch, s.log_stage, count(*), avg(extract(epoch
from stage_duration)::numeric(15,2))::numeric(15,1),
stddev(extract(epoch from stage_duration)::numeric(15,2))::numeric(15,1)
from public.build_status_log s where sysname <> 'crake' and snapshot >
now() - interval '10 days'  and log_stage !~ 'start|stop' group by
s.branch, s.log_stage having count(*) > 20 and avg(extract(epoch from
stage_duration)::numeric(15,2)) > 20 order by log_stage, branch <>
'HEAD', branch desc
;
branch |   log_stage   | count
|  avg   | stddev
---+---+---++
 HEAD  | bin-check.log |   388 |
1739.0 | 1758.5
 REL9_6_STABLE | bin-check.log |91 |
1430.5 | 1287.9
 REL9_5_STABLE | bin-check.log |87 |
1140.0 |  994.1
 REL9_4_STABLE | bin-check.log |72
|  751.0 |  666.8
 HEAD  | check.log |  2305
|  263.1 | 1197.0
 REL9_6_STABLE | check.log |   610
|  294.7 | 1369.6
 REL9_5_STABLE | check.log |   627
|  170.1 |  819.6
 REL9_4_STABLE | check.log |   512
|  140.4 |  535.3
 REL9_3_STABLE | check.log |   449
|  112.0 |  446.0
 REL9_2_STABLE | check.log |   406
|  109.2 |  380.9
 HEAD  | check-pg_upgrade.log  |  1785
|  319.4 | 1310.5
 REL9_6_STABLE | check-pg_upgrade.log  |   482
|  571.3 | 2811.0
 REL9_5_STABLE | check-pg_upgrade.log  |   484
|  350.5 | 2160.3
 REL9_4_STABLE | check-pg_upgrade.log  |   385
|  240.8 | 1278.9
 REL9_3_STABLE | check-pg_upgrade.log  |   353
|  214.0 | 1188.3
 REL9_2_STABLE | check-pg_upgrade.log  |   314
|  195.6 | 1016.6
 HEAD  | config.log|  2216
|   84.5 |  101.5
 REL9_6_STABLE | config.log|   576
|   90.0 |   90.8
 REL9_5_STABLE | config.log|   584
|  114.0 |  358.8
 REL9_4_STABLE | config.log|   495
|   84.5 |   85.3
 REL9_3_STABLE | config.log|   431
|   97.9 |  100.7
 REL9_2_STABLE | config.log|   391
|   93.1 |   94.6
 HEAD  | contrib-install-check-C.log   |  2250
|  122.9 |  474.5
 REL9_6_STABLE | contrib-install-check-C.log   |   606
|  124.6 |  410.3
 REL9_5_STABLE | contrib-install-check-C.log   |   622
|   84.7 |  348.8
 REL9_4_STABLE | contrib-install-check-C.log   |   508
|  105.9 |  434.1
 REL9_3_STABLE | contrib-install-check-C.log   |   445
|   61.8 |  273.4
 REL9_2_STABLE | contrib-install-check-C.log   |   403
|   54.3 |  205.7
 HEAD  | contrib-install-check-cs_CZ.UTF-8.log |   184
|   25.7 |   11.8
 REL9_6_STABLE | contrib-install-check-cs_CZ.UTF-8.log |46
|   25.7 |   14.0
 REL9_5_STABLE | contrib-install-check-cs_CZ.UTF-8.log |42
|   23.8 |   16.6
 REL9_4_STABLE | contrib-install-check-cs_CZ.UTF-8.log |36
|   22.9 |   12.4
 HEAD  | contrib-install-check-en_US.8859-15.log   |37
|  173.9 |   32.3
 HEAD  | contrib-install-check-en_US.ISO8859-1.log |33
|  244.7 |   35.7
 HEAD  | contrib-install-check-en_US.log   |   171
|   65.9 |  101.6
 REL9_6_STABLE | contrib-install-check-en_US.log   |42
|   61.7 |   89.3
 REL9_5_STABLE | contrib-install-check-en_US.log   |37
|   54.3 |   79.3
 REL9_4_STABLE | contrib-install-check-en_US.log   |33
|   53.7 |   71.5
 REL9_3_STABLE | 

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Tom Lane]

Andres Freund  writes:
> On 2017-03-11 22:14:07 -0500, Tom Lane wrote:
>> This looks generally sane to me, although I'm not very happy about folding
>> the "$(MKDIR_P) output_iso" call into pg_isolation_regress_check --- that
>> seems weird and unlike the way it's done for the regular regression test
>> case.

> Yea, not super happy about that either - alternatively we could fold it
> into pg_regress.

Yeah, teaching pg_regress to auto-create the --temp-instance directory
seems perfectly sane from here.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Andres Freund]

0;115;0c
On 2017-03-11 22:14:07 -0500, Tom Lane wrote:
> Andres Freund  writes:
> > On 2017-03-11 11:48:31 -0800, Andres Freund wrote:
> >> I think that'd be a good plan.  We probably should also keep --outputdir
> >> seperate (which test_decoding/Makefile does, but
> >> pg_isolation_regress_check doesn't)?
> 
> > Here's a patch doing that (based on yours).  I'd be kind of inclined to
> > set --outputdir for !isolation tests too; possibly even move tmp_check
> > below output_iso/ output_regress/ or such - but that seems like it
> > potentially could cause some disagreement...
> 
> This looks generally sane to me, although I'm not very happy about folding
> the "$(MKDIR_P) output_iso" call into pg_isolation_regress_check --- that
> seems weird and unlike the way it's done for the regular regression test
> case.

Yea, not super happy about that either - alternatively we could fold it
into pg_regress.  But given the way that prove_checks works, I thought
it'd not be too ugly comparatively.

- Andres


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Tom Lane]

Andres Freund  writes:
> On 2017-03-11 11:48:31 -0800, Andres Freund wrote:
>> I think that'd be a good plan.  We probably should also keep --outputdir
>> seperate (which test_decoding/Makefile does, but
>> pg_isolation_regress_check doesn't)?

> Here's a patch doing that (based on yours).  I'd be kind of inclined to
> set --outputdir for !isolation tests too; possibly even move tmp_check
> below output_iso/ output_regress/ or such - but that seems like it
> potentially could cause some disagreement...

This looks generally sane to me, although I'm not very happy about folding
the "$(MKDIR_P) output_iso" call into pg_isolation_regress_check --- that
seems weird and unlike the way it's done for the regular regression test
case.  But probably Peter has a better-informed opinion.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Andres Freund]

On 2017-03-11 11:48:31 -0800, Andres Freund wrote:
> On 2017-03-11 12:05:23 -0500, Tom Lane wrote:
> > I wrote:
> > > I believe the core problem is that contrib/test_decoding's regresscheck
> > > and isolationcheck targets each want to use ./tmp_check as their
> > > --temp-instance.  make has no reason to believe it shouldn't run those
> > > two sub-jobs in parallel, but when it does, we get two postmasters trying
> > > to share the same directory.  This looks reasonably straightforward to
> > > solve, but I'm not entirely familiar with the code here, and am not
> > > sure what is the least ugly way to fix it.
> > 
> > Enlarging on that: if I cd into contrib/test_decoding and do
> > "make check -j4" or so, it reliably fails.
> 
> Yep, can reproduce here as well. Interesting that, with -j16, I could
> survive several dozen runs from the toplevel locally.
> 
> 
> > This is a localized patch that only fixes things for
> > contrib/test_decoding; what I'm wondering is if it would be better to
> > establish a more widespread convention that
> > $(pg_isolation_regress_check) should use a different --temp-instance
> > directory than $(pg_regress_check) does.
> 
> I think that'd be a good plan.  We probably should also keep --outputdir
> seperate (which test_decoding/Makefile does, but
> pg_isolation_regress_check doesn't)?

Here's a patch doing that (based on yours).  I'd be kind of inclined to
set --outputdir for !isolation tests too; possibly even move tmp_check
below output_iso/ output_regress/ or such - but that seems like it
potentially could cause some disagreement...

- Andres
>From fb89f431d120e9123dca367d23f330b7d31b3f01 Mon Sep 17 00:00:00 2001
From: Andres Freund 
Date: Sat, 11 Mar 2017 15:03:18 -0800
Subject: [PATCH] Improve isolation regression tests infrastructure.

Previously if a directory had both isolationtester and plain
regression tests they couldn't be run in parallel, because they'd
access the same files/directories.  That, so far, only affected
contrib/test_decoding.

Rather than fix that locally in contrib/test_decoding improve
pg_regress_isolation_[install]check to use separate resources from
plain regression tests.

Use the improved helpers everywhere, even where previously not used.

Author: Tom Lane and Andres Freund
Discussion: https://postgr.es/m/20170311194831.vm5ikpczq52c2...@alap3.anarazel.de
---
 contrib/test_decoding/.gitignore |  5 +++--
 contrib/test_decoding/Makefile   |  7 +--
 src/Makefile.global.in   | 29 ++--
 src/test/isolation/.gitignore|  5 ++---
 src/test/isolation/Makefile  |  8 
 src/test/modules/snapshot_too_old/.gitignore |  2 +-
 src/test/modules/snapshot_too_old/Makefile   |  6 +++---
 7 files changed, 37 insertions(+), 25 deletions(-)

diff --git a/contrib/test_decoding/.gitignore b/contrib/test_decoding/.gitignore
index 1f95503494..b4903eba65 100644
--- a/contrib/test_decoding/.gitignore
+++ b/contrib/test_decoding/.gitignore
@@ -1,5 +1,6 @@
 # Generated subdirectories
 /log/
-/isolation_output/
-/regression_output/
+/results/
+/output_iso/
 /tmp_check/
+/tmp_check_iso/
diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile
index d2bc8b8350..6c18189d9d 100644
--- a/contrib/test_decoding/Makefile
+++ b/contrib/test_decoding/Makefile
@@ -5,7 +5,7 @@ PGFILEDESC = "test_decoding - example of a logical decoding output plugin"
 
 # Note: because we don't tell the Makefile there are any regression tests,
 # we have to clean those result files explicitly
-EXTRA_CLEAN = $(pg_regress_clean_files) ./regression_output ./isolation_output
+EXTRA_CLEAN = $(pg_regress_clean_files)
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
@@ -42,11 +42,8 @@ REGRESSCHECKS=ddl xact rewrite toast permissions decoding_in_xact \
 	spill slot
 
 regresscheck: | submake-regress submake-test_decoding temp-install
-	$(MKDIR_P) regression_output
 	$(pg_regress_check) \
 	--temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \
-	--temp-instance=./tmp_check \
-	--outputdir=./regression_output \
 	$(REGRESSCHECKS)
 
 regresscheck-install-force: | submake-regress submake-test_decoding temp-install
@@ -56,10 +53,8 @@ regresscheck-install-force: | submake-regress submake-test_decoding temp-install
 ISOLATIONCHECKS=mxact delayed_startup ondisk_startup concurrent_ddl_dml
 
 isolationcheck: | submake-isolation submake-test_decoding temp-install
-	$(MKDIR_P) isolation_output
 	$(pg_isolation_regress_check) \
 	--temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \
-	--outputdir=./isolation_output \
 	$(ISOLATIONCHECKS)
 
 isolationcheck-install-force: all | submake-isolation submake-test_decoding temp-install
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 831d39a9d1..9796ffd753 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -545,14 +545,31 @@ TEMP_CONF += 

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Andres Freund]

On 2017-03-11 14:17:42 -0800, Andres Freund wrote:
> On 2017-03-07 09:36:51 -0300, Alvaro Herrera wrote:
> > FWIW, +1 on improving matters here.
> >
> > Andres Freund wrote:
> >
> > > The best I can come up so far is a toplevel target that creates the temp
> > > install, starts a cluster and then runs the 'installcheck-or-check'
> > > target on all the subdirectories via recursion. Individual makefiles can
> > > either use the pre-existing cluster (most of of contrib for example), or
> > > use the temporary install and run their pre-existing check target using
> > > that (the tap tests, test_decoding, ...).
> >
> > I think a toplevel installcheck-or-check target is a good first step
> > (though definitely lets find a better name).  Just being able to run all
> > tests without the need for 95% of pointless initdb's would be helpful
> > enough.
> 
> Here's a hacky proof-of-concept patch that implements a contrib/
> 'fastcheck' target.
> timing of:
> 
> make -s -j01 -Otarget check   2m49.286s
> make -s -j16 -Otarget check   0m44.120s
> make -s -j01 -Otarget fastcheck   1m1.533s
> make -s -j16 -Otarget fastcheck   0m24.385s
> make -s -j01 -Otarget USE_MODULE_DB=1 \
>   installcheck check-test_decoding-recurse0m56.016s
> make -s -j16 -Otarget USE_MODULE_DB=1 \
>   installcheck check-test_decoding-recurse0m23.608s

Ooops - all these timings are from a coverage enabled build - the times
are overall a bit smaller without that.

- Andres


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Andres Freund]

On 2017-03-07 09:36:51 -0300, Alvaro Herrera wrote:
> FWIW, +1 on improving matters here.
>
> Andres Freund wrote:
>
> > The best I can come up so far is a toplevel target that creates the temp
> > install, starts a cluster and then runs the 'installcheck-or-check'
> > target on all the subdirectories via recursion. Individual makefiles can
> > either use the pre-existing cluster (most of of contrib for example), or
> > use the temporary install and run their pre-existing check target using
> > that (the tap tests, test_decoding, ...).
>
> I think a toplevel installcheck-or-check target is a good first step
> (though definitely lets find a better name).  Just being able to run all
> tests without the need for 95% of pointless initdb's would be helpful
> enough.

Here's a hacky proof-of-concept patch that implements a contrib/
'fastcheck' target.
timing of:

make -s -j01 -Otarget check 2m49.286s
make -s -j16 -Otarget check 0m44.120s
make -s -j01 -Otarget fastcheck 1m1.533s
make -s -j16 -Otarget fastcheck 0m24.385s
make -s -j01 -Otarget USE_MODULE_DB=1 \
installcheck check-test_decoding-recurse0m56.016s
make -s -j16 -Otarget USE_MODULE_DB=1 \
installcheck check-test_decoding-recurse0m23.608s

All of these should, excepting rerunning initdb/server start, have
equivalent test coverage.

'fastcheck' is obviously, if you look at it, a POC. We'd need:
- abstract this away somehow, it's not nice to copy the logic into a
  makefile
- tie contrib/'fastcheck' into check-world by overriding that target
- use the temp install we've created previously?
- more bulletproof server stop mechanism?

- Andres
>From 77520424a7b8a45e629eada864baac49db7fceb1 Mon Sep 17 00:00:00 2001
From: Andres Freund 
Date: Sat, 11 Mar 2017 14:03:26 -0800
Subject: [PATCH] WIP; 'fastcheck' target in contrib

---
 contrib/Makefile | 17 +
 1 file changed, 17 insertions(+)

diff --git a/contrib/Makefile b/contrib/Makefile
index e84eb67008..ca08fc9b74 100644
--- a/contrib/Makefile
+++ b/contrib/Makefile
@@ -90,6 +90,23 @@ endif
 # Missing:
 #		start-scripts	\ (does not have a makefile)
 
+NONSHARED_DATADIR_TARGETS=test_decoding
+SHARED_DATADIR_TARGETS=$(filter-out $(NONSHARED_DATADIR_TARGETS), $(SUBDIRS))
+SERVOPTIONS="-c log_line_prefix='%m [%p] %q%a ' -c unix_socket_directories=$(shell pwd)/tmp_fastcheck/ -clisten_addresses= -c port=5432"
+REGRESSPORT=5432
+REGRESSHOST=$(shell pwd)/tmp_fastcheck/
+
+fastcheck:
+	@mkdir -p tmp_fastcheck
+	@rm -rf tmp_fastcheck/data
+	$(bindir)/initdb --no-clean --no-sync -D tmp_fastcheck/data > tmp_fastcheck/initdb.log
+	PGPORT=$(REGRESSPORT) PGHOST=$(REGRESSHOST) \
+		$(bindir)/pg_ctl -D tmp_fastcheck/data -o$(SERVOPTIONS) --log tmp_fastcheck/postmaster.log start
+	$(MAKE) PGPORT=$(REGRESSPORT) PGHOST=$(REGRESSHOST) USE_MODULE_DB=1 -Otarget \
+		$(foreach t,$(SHARED_DATADIR_TARGETS),installcheck-$(t)-recurse) \
+		$(foreach t,$(NONSHARED_DATADIR_TARGETS),check-$(t)-recurse) || \
+		$(bindir)/pg_ctl -D tmp_fastcheck/data stop
+	$(bindir)/pg_ctl -D tmp_fastcheck/data stop
 
 $(recurse)
 $(recurse_always)
-- 
2.11.0.22.g8d7a455.dirty


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Jim Nasby]

On 3/11/17 2:06 PM, Tom Lane wrote:

Jim Nasby  writes:

It's actually a lot harder to mess up providing a git repo link than
manually submitting patches to the mailing list.

Yeah, we've heard that proposal before.  We're still not doing it though.
Insisting on patches being actually submitted to the mailing list is
important for archival and possibly legal reasons.  If someone sends
in a link to $random-repo, once that site goes away there's no way to
determine exactly what was submitted.


The full proposal was that the commitfest app have the ability to 
generate and post the patch for you, assuming that the smoke-test passes.

--
Jim Nasby, Chief Data Architect, OpenSCG
http://OpenSCG.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Tom Lane]

Jim Nasby  writes:
> It's actually a lot harder to mess up providing a git repo link than 
> manually submitting patches to the mailing list.

Yeah, we've heard that proposal before.  We're still not doing it though.
Insisting on patches being actually submitted to the mailing list is
important for archival and possibly legal reasons.  If someone sends
in a link to $random-repo, once that site goes away there's no way to
determine exactly what was submitted.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Andres Freund]

On 2017-03-11 12:05:23 -0500, Tom Lane wrote:
> I wrote:
> > I believe the core problem is that contrib/test_decoding's regresscheck
> > and isolationcheck targets each want to use ./tmp_check as their
> > --temp-instance.  make has no reason to believe it shouldn't run those
> > two sub-jobs in parallel, but when it does, we get two postmasters trying
> > to share the same directory.  This looks reasonably straightforward to
> > solve, but I'm not entirely familiar with the code here, and am not
> > sure what is the least ugly way to fix it.
> 
> Enlarging on that: if I cd into contrib/test_decoding and do
> "make check -j4" or so, it reliably fails.

Yep, can reproduce here as well. Interesting that, with -j16, I could
survive several dozen runs from the toplevel locally.


> This is a localized patch that only fixes things for
> contrib/test_decoding; what I'm wondering is if it would be better to
> establish a more widespread convention that
> $(pg_isolation_regress_check) should use a different --temp-instance
> directory than $(pg_regress_check) does.

I think that'd be a good plan.  We probably should also keep --outputdir
seperate (which test_decoding/Makefile does, but
pg_isolation_regress_check doesn't)?

- Andres


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Jim Nasby]

On 3/10/17 6:06 PM, Peter Eisentraut wrote:

On 3/10/17 19:00, Jim Nasby wrote:

Maybe instead of having the commitfest app try and divine patches from
the list it should be able to send patches to the list from a specified
git repo/branch. Anyone that provides that info would have tests run
automagically, patches sent, etc. Anyone who doesn't can just keep using
the old process.


Those people who know what they're doing will presumably run all those
checks before they submit a patch.  It's those people who send in
patches that don't apply cleanly or fail the tests that would benefit
from this system.  But if they're that careless, then they also won't
take care to use this particular system correctly.


It's actually a lot harder to mess up providing a git repo link than 
manually submitting patches to the mailing list. For most patches, it's 
also a hell of a lot faster to just submit a repo URL rather than 
dealing with patch files. Having this also means that reviewers can 
focus more on what the patch is actually doing instead of mechanical 
crap best left to a machine.


Of course, *you* work on changes that are far more complex than any 
newbie will, and it wouldn't surprise me if such a feature wouldn't help 
you or other senior hackers at all. But AFAICT it wouldn't get in your 
way either. It would remove yet another burden for new hackers.


Anyway, this is well off topic for the original thread...
--
Jim Nasby, Chief Data Architect, OpenSCG
http://OpenSCG.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Tom Lane]

I wrote:
> I believe the core problem is that contrib/test_decoding's regresscheck
> and isolationcheck targets each want to use ./tmp_check as their
> --temp-instance.  make has no reason to believe it shouldn't run those
> two sub-jobs in parallel, but when it does, we get two postmasters trying
> to share the same directory.  This looks reasonably straightforward to
> solve, but I'm not entirely familiar with the code here, and am not
> sure what is the least ugly way to fix it.

Enlarging on that: if I cd into contrib/test_decoding and do
"make check -j4" or so, it reliably fails.  With the attached patch,
it passes.  This is a localized patch that only fixes things for
contrib/test_decoding; what I'm wondering is if it would be better to
establish a more widespread convention that $(pg_isolation_regress_check)
should use a different --temp-instance directory than $(pg_regress_check)
does.

regards, tom lane

diff --git a/contrib/test_decoding/.gitignore b/contrib/test_decoding/.gitignore
index 1f95503..09d60bf 100644
*** a/contrib/test_decoding/.gitignore
--- b/contrib/test_decoding/.gitignore
***
*** 3,5 
--- 3,6 
  /isolation_output/
  /regression_output/
  /tmp_check/
+ /tmp_check_iso/
diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile
index d2bc8b8..ea31d88 100644
*** a/contrib/test_decoding/Makefile
--- b/contrib/test_decoding/Makefile
*** PGFILEDESC = "test_decoding - example of
*** 5,11 
  
  # Note: because we don't tell the Makefile there are any regression tests,
  # we have to clean those result files explicitly
! EXTRA_CLEAN = $(pg_regress_clean_files) ./regression_output ./isolation_output
  
  ifdef USE_PGXS
  PG_CONFIG = pg_config
--- 5,12 
  
  # Note: because we don't tell the Makefile there are any regression tests,
  # we have to clean those result files explicitly
! EXTRA_CLEAN = $(pg_regress_clean_files) ./regression_output \
! 	./isolation_output ./tmp_check_iso
  
  ifdef USE_PGXS
  PG_CONFIG = pg_config
*** isolationcheck: | submake-isolation subm
*** 59,64 
--- 60,66 
  	$(MKDIR_P) isolation_output
  	$(pg_isolation_regress_check) \
  	--temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \
+ 	--temp-instance=./tmp_check_iso \
  	--outputdir=./isolation_output \
  	$(ISOLATIONCHECKS)
  

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Tom Lane]

Peter Eisentraut  writes:
 make check-world -j2 seems to run fine for me.

> I have been pounding it a bit, and every so often the test_decoding
> tests fail in mysterious ways, but otherwise it seems to work fine.  I'm
> curious what you are seeing.

For me, it's quite unreliable at make parallelism above -j2 or so.

I believe the core problem is that contrib/test_decoding's regresscheck
and isolationcheck targets each want to use ./tmp_check as their
--temp-instance.  make has no reason to believe it shouldn't run those
two sub-jobs in parallel, but when it does, we get two postmasters trying
to share the same directory.  This looks reasonably straightforward to
solve, but I'm not entirely familiar with the code here, and am not
sure what is the least ugly way to fix it.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Peter Eisentraut]

On 3/10/17 19:00, Jim Nasby wrote:
> Maybe instead of having the commitfest app try and divine patches from 
> the list it should be able to send patches to the list from a specified 
> git repo/branch. Anyone that provides that info would have tests run 
> automagically, patches sent, etc. Anyone who doesn't can just keep using 
> the old process.

Those people who know what they're doing will presumably run all those
checks before they submit a patch.  It's those people who send in
patches that don't apply cleanly or fail the tests that would benefit
from this system.  But if they're that careless, then they also won't
take care to use this particular system correctly.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Peter Eisentraut]

On 3/10/17 14:43, Andres Freund wrote:
> I do get regular issues, although the happen to not end up in visible
> failures.  All the tests output their regression.diffs into the same
> place - which means there'll every now be a failure to remove the file,
> and if there were an actual failure it'd possibly end up being
> attributed to the wrong test.  So I think we need to relocate that file
> to be relative to the sql/ | specs/ | expected/ directories?

I'm confused here.  Every test suite runs in a separate directory, and
for the tests in the same suite pg_regress will take care of running the
tests in parallel and producing the diffs.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Jim Nasby]

On 3/10/17 5:57 PM, Peter Eisentraut wrote:

On 3/10/17 14:53, Jim Nasby wrote:

The biggest win we'd get from something like Travis would be if the
commitfest monitored for new patch files coming in for monitored threads
and it created a new branch, applied the patches, and if they applied
without error commit the branch and push to let Travis do it's thing. We
wouldn't want that running in the main git repo, but it should be fine
in a fork that's dedicated to that purpose.


This has been discussed several times before, e.g.,

https://www.postgresql.org/message-id/54dd2413.8030...@gmx.net


Maybe instead of having the commitfest app try and divine patches from 
the list it should be able to send patches to the list from a specified 
git repo/branch. Anyone that provides that info would have tests run 
automagically, patches sent, etc. Anyone who doesn't can just keep using 
the old process.

--
Jim Nasby, Chief Data Architect, OpenSCG
http://OpenSCG.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Peter Eisentraut]

On 3/10/17 14:53, Jim Nasby wrote:
> The biggest win we'd get from something like Travis would be if the 
> commitfest monitored for new patch files coming in for monitored threads 
> and it created a new branch, applied the patches, and if they applied 
> without error commit the branch and push to let Travis do it's thing. We 
> wouldn't want that running in the main git repo, but it should be fine 
> in a fork that's dedicated to that purpose.

This has been discussed several times before, e.g.,

https://www.postgresql.org/message-id/54dd2413.8030...@gmx.net

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Jim Nasby]

On 3/10/17 2:18 PM, Magnus Hagander wrote:

But if you can put together something that picks up the individual
patches out of the mail threads in the CF app and keeps branch-tips in a
git repo up-to-date with those, including feeding the results back into
the app, then go for it :)


Seems like an ideal project for someone not on -hackers... do we have a 
list of "How you can help Postgres besides hacking database code" anywhere?

--
Jim Nasby, Chief Data Architect, OpenSCG
http://OpenSCG.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Magnus Hagander]

On Fri, Mar 10, 2017 at 3:11 PM, Jim Nasby  wrote:

> On 3/10/17 2:05 PM, Magnus Hagander wrote:
>
>>
>> Travis specifically would not help us with this, due to the dependency
>> on gifhub, but something that knows how to run "patch ... && configure
>> && make && make check" in a container would.
>>
>
> Who's updating https://github.com/postgres/postgres/ right now?
> Presumably that script would be the basis for this...


Yes. It's a pure mirror script.

I'm pretty sure we *don't* want to push a branch for every single patch
that goes in a CF all into our master repository...

We could use a completely separate repo on github for it if we want, yes.
Then we just need to figure out how to get the patches there..


>
> I'm unsure what would be easiest -- have something drive a "throwaway
>> github repo" off the data in the CF app and try to pull things from
>> there, or to just spawn containers and run it directly without travis.
>>
>
> I'd be a bit nervous about creating our own container solution and opening
> that to automatically deploying patches. Travis (and other tools) already
> have that problem solved (or at least if they get hacked it's on them to
> clean up and not us :)
>
> Plus it'd be a heck of a lot more work on our side to set all that stuff
> up.


This is what I'm not so sure about. Setting up an empty container provided
we only ever need to test the one thing and only ever need the one platform
is not particularly complicated.

But if you can put together something that picks up the individual patches
out of the mail threads in the CF app and keeps branch-tips in a git repo
up-to-date with those, including feeding the results back into the app,
then go for it :)




> The bigger issue with those is the usual -- how do you handle patches
>> that have dependencies on each other,because they're always going to
>> show up as broken individually. I guess we could tell people doing those
>> to just push a git branch on github and register that one in the CF app
>> (which does have some very basic support for tracking that, but I doubt
>> anybody uses it today).
>>
>
> If people use git format-patch it should JustWork(tm). Specifying a
> specific repo is another option.
>

Right. But people don't use that all the time, and it's not currently a
requirement on patch submitters. and we've traditionally been of the
opinion that we don't want to put too much requirements on such things for
submitters.



> Even if we can't make it work for really complicated patches it might
> still be a win.


Definitely as long as we can keep some manual process for thos epatches.



> If the travis build failed, commitfest could notify the author.
>>
>> It could also rebase master into each branch on a daily basis so
>> authors would know very quickly if something got committed that
>> broke their patch.
>>
>>
>> It could at least verify that the patch still applies, yes.
>>
>
> If the rebase was pushed to github and travis was setup, travis would then
> test the changes as well.
>
>
Right.

//Magnus

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Jim Nasby]

On 3/10/17 2:05 PM, Magnus Hagander wrote:


Travis specifically would not help us with this, due to the dependency
on gifhub, but something that knows how to run "patch ... && configure
&& make && make check" in a container would.


Who's updating https://github.com/postgres/postgres/ right now? 
Presumably that script would be the basis for this...



I'm unsure what would be easiest -- have something drive a "throwaway
github repo" off the data in the CF app and try to pull things from
there, or to just spawn containers and run it directly without travis.


I'd be a bit nervous about creating our own container solution and 
opening that to automatically deploying patches. Travis (and other 
tools) already have that problem solved (or at least if they get hacked 
it's on them to clean up and not us :)


Plus it'd be a heck of a lot more work on our side to set all that stuff up.


The bigger issue with those is the usual -- how do you handle patches
that have dependencies on each other,because they're always going to
show up as broken individually. I guess we could tell people doing those
to just push a git branch on github and register that one in the CF app
(which does have some very basic support for tracking that, but I doubt
anybody uses it today).


If people use git format-patch it should JustWork(tm). Specifying a 
specific repo is another option.


Even if we can't make it work for really complicated patches it might 
still be a win.



If the travis build failed, commitfest could notify the author.

It could also rebase master into each branch on a daily basis so
authors would know very quickly if something got committed that
broke their patch.


It could at least verify that the patch still applies, yes.


If the rebase was pushed to github and travis was setup, travis would 
then test the changes as well.

--
Jim Nasby, Chief Data Architect, OpenSCG
http://OpenSCG.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Magnus Hagander]

On Fri, Mar 10, 2017 at 2:53 PM, Jim Nasby  wrote:

> On 3/10/17 1:09 PM, Peter Eisentraut wrote:
>
>> On 3/10/17 03:27, Jim Nasby wrote:
>>
>>> Perhaps https://travis-ci.org/ or something similar could be used for
>>> this. That avoids any issues about random code.
>>>
>>
>> That doesn't achieve any platform coverage, which is the main point here.
>>
>
> I don't think platform coverage is the first thing to worry about with
> patches, nor with ongoing development.
>

I think we are talking about solving two different problems...


>
> The biggest win we'd get from something like Travis would be if the
> commitfest monitored for new patch files coming in for monitored threads
> and it created a new branch, applied the patches, and if they applied
> without error commit the branch and push to let Travis do it's thing. We
> wouldn't want that running in the main git repo, but it should be fine in a
> fork that's dedicated to that purpose.
>

Travis specifically would not help us with this, due to the dependency on
gifhub, but something that knows how to run "patch ... && configure && make
&& make check" in a container would.

I'm unsure what would be easiest -- have something drive a "throwaway
github repo" off the data in the CF app and try to pull things from there,
or to just spawn containers and run it directly without travis.

The bigger issue with those is the usual -- how do you handle patches that
have dependencies on each other,because they're always going to show up as
broken individually. I guess we could tell people doing those to just push
a git branch on github and register that one in the CF app (which does have
some very basic support for tracking that, but I doubt anybody uses it
today).



> If the travis build failed, commitfest could notify the author.
>
> It could also rebase master into each branch on a daily basis so authors
> would know very quickly if something got committed that broke their patch.
>

It could at least verify that the patch still applies, yes.



> Obviously that doesn't remove the need for manual testing or the
> buildfarm, but it would at least let everyone know that the patch passed a
> smoke test


It's definitely something that would be useful, but as you say, it solves a
different problem.

//Magnus

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Jim Nasby]

On 3/10/17 1:09 PM, Peter Eisentraut wrote:

On 3/10/17 03:27, Jim Nasby wrote:

Perhaps https://travis-ci.org/ or something similar could be used for
this. That avoids any issues about random code.


That doesn't achieve any platform coverage, which is the main point here.


I don't think platform coverage is the first thing to worry about with 
patches, nor with ongoing development.


The biggest win we'd get from something like Travis would be if the 
commitfest monitored for new patch files coming in for monitored threads 
and it created a new branch, applied the patches, and if they applied 
without error commit the branch and push to let Travis do it's thing. We 
wouldn't want that running in the main git repo, but it should be fine 
in a fork that's dedicated to that purpose.


If the travis build failed, commitfest could notify the author.

It could also rebase master into each branch on a daily basis so authors 
would know very quickly if something got committed that broke their patch.


Obviously that doesn't remove the need for manual testing or the 
buildfarm, but it would at least let everyone know that the patch passed 
a smoke test.

--
Jim Nasby, Chief Data Architect, OpenSCG
http://OpenSCG.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Andres Freund]

On 2017-03-10 01:59:53 -0500, Peter Eisentraut wrote:
> On 3/8/17 16:49, Andres Freund wrote:
> >> make check-world -j2 seems to run fine for me.
> > 
> > Hm, I at least used to get a lot of spurious failures with this. I
> > e.g. don't think the free port selection is race free.
> 
> I was also not sure about that, but as Michael has pointed out, that
> doesn't matter anymore, because it now uses a private socket directory.

Yea, I had forgotten about that bit.


> I have been pounding it a bit, and every so often the test_decoding
> tests fail in mysterious ways, but otherwise it seems to work fine.  I'm
> curious what you are seeing.

I do get regular issues, although the happen to not end up in visible
failures.  All the tests output their regression.diffs into the same
place - which means there'll every now be a failure to remove the file,
and if there were an actual failure it'd possibly end up being
attributed to the wrong test.  So I think we need to relocate that file
to be relative to the sql/ | specs/ | expected/ directories?

Curious about the test decoding thing - hadn't seen that here.

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Peter Eisentraut]

On 3/10/17 03:27, Jim Nasby wrote:
> Perhaps https://travis-ci.org/ or something similar could be used for 
> this. That avoids any issues about random code.

That doesn't achieve any platform coverage, which is the main point here.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Peter Eisentraut]

On 3/10/17 13:02, Magnus Hagander wrote:
> for their largest feature development. Could be a good idea to for
> example ave an example yml file somewhere on the wiki that people could
> put into their branches. 

https://github.com/petere/postgresql/blob/travis/.travis.yml

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Magnus Hagander]

On Fri, Mar 10, 2017 at 11:29 AM, Dagfinn Ilmari Mannsåker <
ilm...@ilmari.org> wrote:

> Magnus Hagander  writes:
>
> > AFAIK travis-ci would require us to use github as our hoster for all
> those
> > things, and embrace that workflow, they don't support anything else.
> >
> > There might be others that do, just not travis.
>
> It merely requires the repository to exist on GitHub, and postgresql.git
> is already mirrored to https://github.com/postgres/postgres.  If there
> was a .travis.yml in the repo, people who fork it could easily enable
> Travis-CI for their fork, even the official repo isn't hooked up.
>

That's true. It would require a bunch of additional branches to make it
useful in core though, but it śeems like it could be a useful thing for
people to enable in their own personal forks/branches if they use github
for their largest feature development. Could be a good idea to for example
ave an example yml file somewhere on the wiki that people could put into
their branches.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Dagfinn Ilmari Mannsåker]

Magnus Hagander  writes:

> AFAIK travis-ci would require us to use github as our hoster for all those
> things, and embrace that workflow, they don't support anything else.
>
> There might be others that do, just not travis.

It merely requires the repository to exist on GitHub, and postgresql.git
is already mirrored to https://github.com/postgres/postgres.  If there
was a .travis.yml in the repo, people who fork it could easily enable
Travis-CI for their fork, even the official repo isn't hooked up.

- ilmari

-- 
- Twitter seems more influential [than blogs] in the 'gets reported in
  the mainstream press' sense at least.   - Matt McLeod
- That'd be because the content of a tweet is easier to condense down
  to a mainstream media article.  - Calle Dybedahl


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Magnus Hagander]

On Fri, Mar 10, 2017 at 3:27 AM, Jim Nasby  wrote:

> On 3/7/17 9:52 PM, Magnus Hagander wrote:
>
>> There have also on and off been discussions about building arbitrary
>> patches as they are sent to the mailinglists. Doing that without any
>> committer (or other trusted party) as a filter is a completely different
>> challenge of course, given that it basically amounts to downloading and
>> running random code off the internet.
>>
>
> Perhaps https://travis-ci.org/ or something similar could be used for
> this. That avoids any issues about random code.


AFAIK travis-ci would require us to use github as our hoster for all those
things, and embrace that workflow, they don't support anything else.

There might be others that do, just not travis.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Jim Nasby]

On 3/7/17 9:52 PM, Magnus Hagander wrote:

There have also on and off been discussions about building arbitrary
patches as they are sent to the mailinglists. Doing that without any
committer (or other trusted party) as a filter is a completely different
challenge of course, given that it basically amounts to downloading and
running random code off the internet.


Perhaps https://travis-ci.org/ or something similar could be used for 
this. That avoids any issues about random code.

--
Jim Nasby, Chief Data Architect, OpenSCG
http://OpenSCG.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Peter Eisentraut]

On 3/8/17 16:49, Andres Freund wrote:
>> make check-world -j2 seems to run fine for me.
> 
> Hm, I at least used to get a lot of spurious failures with this. I
> e.g. don't think the free port selection is race free.

I was also not sure about that, but as Michael has pointed out, that
doesn't matter anymore, because it now uses a private socket directory.

I have been pounding it a bit, and every so often the test_decoding
tests fail in mysterious ways, but otherwise it seems to work fine.  I'm
curious what you are seeing.

Combining make -j10 and prove -j4, I get the run time down to 2 minutes
and a bit, from 20+ minutes.  Using the -O option if you have GNU make
>=4 is also useful to get some more sane output.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Michael Paquier]

On Thu, Mar 9, 2017 at 6:49 AM, Andres Freund  wrote:
> On 2017-03-08 16:32:49 -0500, Peter Eisentraut wrote:
>> On 3/6/17 19:53, Andres Freund wrote:
>> > I'm just not quite sure what the best way is to make it easier to run
>> > tests in parallel within the tree.
>>
>> make check-world -j2 seems to run fine for me.
>
> Hm, I at least used to get a lot of spurious failures with this. I
> e.g. don't think the free port selection is race free.

pg_regress is running each Postgres instance in a separate Unix socket
directory, so the port selection is not problem. PostgresNode.pm is
also careful about that.

> Also that
> doesn't solve the issue of the time spent in repetitive initdbs - but
> that's mainly in contrib, and we probably solve that there by having a
> special target in contrib/ building a cluster once.

Yeah, you can count the growing src/test/modules in that as well.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Andres Freund]

On 2017-03-08 16:32:49 -0500, Peter Eisentraut wrote:
> On 3/6/17 19:53, Andres Freund wrote:
> > I'm just not quite sure what the best way is to make it easier to run
> > tests in parallel within the tree.
> 
> make check-world -j2 seems to run fine for me.

Hm, I at least used to get a lot of spurious failures with this. I
e.g. don't think the free port selection is race free.  Also that
doesn't solve the issue of the time spent in repetitive initdbs - but
that's mainly in contrib, and we probably solve that there by having a
special target in contrib/ building a cluster once.


> You can also run prove with a -j option.

Ah, interesting.


> (The problem is that parallel make and parallel tests together might
> explode a bit, so we might want some way to control which aspect we
> parallelize.)

Yea, I indeed see that. Probably could do a top-level prerequisite for
check on a check-world subset that includes docs?

- Andres


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Peter Eisentraut]

On 3/6/17 19:53, Andres Freund wrote:
> I'm just not quite sure what the best way is to make it easier to run
> tests in parallel within the tree.

make check-world -j2 seems to run fine for me.

With higher -j I appear to be running out of memory or disks space, so I
haven't checked that any further, but it seems possible.

You can also run prove with a -j option.

And we could parallelize some of the contrib/pl tests, e.g., plpython.

(The problem is that parallel make and parallel tests together might
explode a bit, so we might want some way to control which aspect we
parallelize.)

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Robert Haas]

On Tue, Mar 7, 2017 at 11:23 PM, Andres Freund  wrote:
> Personally that's not addressing my main concern, which is that the
> latency of getting done with some patch/topic takes a long while. If I
> have to wait for the buildfarm to check some preliminary patch, I still
> have to afterwards work on pushing it to master.  And very likely my
> local check would finish a lot faster than a bunch of buildfarm animals
> - I have after all a plenty powerfull machine, lots of cores, fast ssd,
> lots of memory, ...
>
> So I really want faster end-to-end test, not less cpu time spent on my
> own machine.

Yeah.  I think the buildfarm-for-test-commits or maybe
buildfarm-for-approved-branches-belonging-to-people-we-basically-trust
idea isn't a bad one, but it's not a substitute for $SUBJECT.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Andres Freund]

On 2017-03-07 20:58:35 +0800, Simon Riggs wrote:
> On 7 March 2017 at 20:36, Alvaro Herrera  wrote:
> 
> > FWIW, +1 on improving matters here.
> 
> +1 also.
> 
> I don't see what's wrong with relying on buildfarm though; testing is
> exactly what its there for.
> 
> If we had a two-stage process, where committers can issue "trial
> commits" as a way of seeing if the build farm likes things. If they
> do, we can push to the main repo.

Personally that's not addressing my main concern, which is that the
latency of getting done with some patch/topic takes a long while. If I
have to wait for the buildfarm to check some preliminary patch, I still
have to afterwards work on pushing it to master.  And very likely my
local check would finish a lot faster than a bunch of buildfarm animals
- I have after all a plenty powerfull machine, lots of cores, fast ssd,
lots of memory, ...

So I really want faster end-to-end test, not less cpu time spent on my
own machine.

- Andres


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Magnus Hagander]

On Tue, Mar 7, 2017 at 7:24 AM, Andrew Dunstan <
andrew.duns...@2ndquadrant.com> wrote:

>
>
> On 03/07/2017 07:58 AM, Simon Riggs wrote:
> > On 7 March 2017 at 20:36, Alvaro Herrera 
> wrote:
> >
> >> FWIW, +1 on improving matters here.
> > +1 also.
> >
> > I don't see what's wrong with relying on buildfarm though; testing is
> > exactly what its there for.
> >
> > If we had a two-stage process, where committers can issue "trial
> > commits" as a way of seeing if the build farm likes things. If they
> > do, we can push to the main repo.
> >
>
>
> I'm happy to work on this.  Not quite sure how it would work, but I'm
> open to any suggestions.
>

Assuming the intention is a service for *committers only*, I suggest
setting up a separate (closed) git repository that committers can push to
and a separate set of BF animals could work from. We could just have it do
all branches in it, no need to filter that way as long as we keep it a
separate repo.

There have also on and off been discussions about building arbitrary
patches as they are sent to the mailinglists. Doing that without any
committer (or other trusted party) as a filter is a completely different
challenge of course, given that it basically amounts to downloading and
running random code off the internet.

But doing just the first would make it a lot easier, and probably still be
of good value.

An in-between could be to hook something off the CF app, but one important
question is how important covering many platforms is. Since we already have
good functionality for doing that in the buildfarm, it makes sense to
utilize that if we can.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Andrew Dunstan]

On 03/07/2017 07:58 AM, Simon Riggs wrote:
> On 7 March 2017 at 20:36, Alvaro Herrera  wrote:
>
>> FWIW, +1 on improving matters here.
> +1 also.
>
> I don't see what's wrong with relying on buildfarm though; testing is
> exactly what its there for.
>
> If we had a two-stage process, where committers can issue "trial
> commits" as a way of seeing if the build farm likes things. If they
> do, we can push to the main repo.
>


I'm happy to work on this.  Not quite sure how it would work, but I'm
open to any suggestions.

cheers

andrew

-- 
Andrew Dunstanhttps://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Dagfinn Ilmari Mannsåker]

Simon Riggs  writes:

> On 7 March 2017 at 20:36, Alvaro Herrera  wrote:
>
>> FWIW, +1 on improving matters here.
>
> +1 also.
>
> I don't see what's wrong with relying on buildfarm though; testing is
> exactly what its there for.
>
> If we had a two-stage process, where committers can issue "trial
> commits" as a way of seeing if the build farm likes things. If they
> do, we can push to the main repo.

In perl we do this by having the smoke testers (build farm) pick up
branches with a specific prefix (smoke-me/ in our case, typically
smoke-me//), in addition to the blead (master) and
maint-x.y (release) branches.

-- 
"I use RMS as a guide in the same way that a boat captain would use
 a lighthouse.  It's good to know where it is, but you generally
 don't want to find yourself in the same spot." - Tollef Fog Heen


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Simon Riggs]

On 7 March 2017 at 20:36, Alvaro Herrera  wrote:

> FWIW, +1 on improving matters here.

+1 also.

I don't see what's wrong with relying on buildfarm though; testing is
exactly what its there for.

If we had a two-stage process, where committers can issue "trial
commits" as a way of seeing if the build farm likes things. If they
do, we can push to the main repo.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Alvaro Herrera]

FWIW, +1 on improving matters here.

Andres Freund wrote:

> The best I can come up so far is a toplevel target that creates the temp
> install, starts a cluster and then runs the 'installcheck-or-check'
> target on all the subdirectories via recursion. Individual makefiles can
> either use the pre-existing cluster (most of of contrib for example), or
> use the temporary install and run their pre-existing check target using
> that (the tap tests, test_decoding, ...).

I think a toplevel installcheck-or-check target is a good first step
(though definitely lets find a better name).  Just being able to run all
tests without the need for 95% of pointless initdb's would be helpful
enough.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Andres Freund]

On 2017-03-06 19:45:27 -0500, Tom Lane wrote:
> Stephen Frost  writes:
> > * Andres Freund (and...@anarazel.de) wrote:
> >> I'm not quite sure what the best way to attack this is, but I think we
> >> need to do something.
>
> > I tend to agree with this, though I haven't got any great answers,
> > unfortunately.
>
> I don't want to reduce test coverage either.  I think the most painless
> way to improve matters would just be to work harder on running tests in
> parallel.  I think most devs these days do most of their work on 4- or
> 8-core machines, yet almost everything except the core regression tests
> is depressingly serial.  I think we could likely get a 2x or better
> reduction in total runtime without too much work just by attacking that.

A lot more probably, based on my preliminary tests / my local test
script.

I'm just not quite sure what the best way is to make it easier to run
tests in parallel within the tree.

The best I can come up so far is a toplevel target that creates the temp
install, starts a cluster and then runs the 'installcheck-or-check'
target on all the subdirectories via recursion. Individual makefiles can
either use the pre-existing cluster (most of of contrib for example), or
use the temporary install and run their pre-existing check target using
that (the tap tests, test_decoding, ...).

Requires editing a bunch of Makefiles to take advantage.  But I don't
really see anything that doesn't require that.

- Andres


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Tom Lane]

Stephen Frost  writes:
> * Andres Freund (and...@anarazel.de) wrote:
>> I'm not quite sure what the best way to attack this is, but I think we
>> need to do something.

> I tend to agree with this, though I haven't got any great answers,
> unfortunately.

I don't want to reduce test coverage either.  I think the most painless
way to improve matters would just be to work harder on running tests in
parallel.  I think most devs these days do most of their work on 4- or
8-core machines, yet almost everything except the core regression tests
is depressingly serial.  I think we could likely get a 2x or better
reduction in total runtime without too much work just by attacking that.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need a builtin way to run all tests faster manner

[Stephen Frost]

Andres,

* Andres Freund (and...@anarazel.de) wrote:
> There's also some tests simply taking way too long, e.g. the pg_dump
> tests just do largely redundant tests for 30s.

About half of the time in the pg_dump case, at least, appears to be in
010_dump_connstr.pl.  I've not looked into it, but based on the test
names it does look like some of those tests might be redundant to what
is already being covered in 002_pg_dump.pl.  Of course, the way the TAP
tests run, they require an initdb and startup of the postmaster, and
that's a somewhat fixed amount of time that any TAP test is going to
take.

I'm all for improving things certainly, though, well, while the pg_dump
tests do a lot, they also try to cover a good bit of the code in
pg_dump.c which is quite large.  I wouldn't want to reduce our code
coverage just to make the regression tests go faster.  The changes to
the pg_dump tests that I'm hoping to push soon will at least reduce the
output some, if not the length of time taken, while providing more code
coverage.

All that said, 30s out of 20m (which seems to more-or-less match what I
get locally too) makes me really wonder if that's the place that we need
to focus.  Is it really the longest running test we have and it's just
that we have tons of them that are doing initdb over and over again?

> I'm not quite sure what the best way to attack this is, but I think we
> need to do something.

I tend to agree with this, though I haven't got any great answers,
unfortunately.

Thanks!

Stephen


signature.asc
Description: Digital signature