check with serial

Started by Andrew Dunstanover 8 years ago6 messages
#1Andrew Dunstan
andrew.dunstan@2ndquadrant.com

The other day I wanted to run "make check" but with the serial schedule.
This wasn't as easy as it should have been. Although we now have
installcheck-parallel we don't have check-serial. Should we have that?
Alternatively, should we allow a SCHEDULE=foo argument for the "check"
target which defaults to parallel?

cheers

andrew

--
Andrew Dunstan https://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

#2Andrew Dunstan
andrew.dunstan@2ndquadrant.com
In reply to: Andrew Dunstan (#1)
1 attachment(s)
Re: check with serial

On 05/01/2017 09:39 AM, Andrew Dunstan wrote:

The other day I wanted to run "make check" but with the serial schedule.
This wasn't as easy as it should have been. Although we now have
installcheck-parallel we don't have check-serial. Should we have that?
Alternatively, should we allow a SCHEDULE=foo argument for the "check"
target which defaults to parallel?

Here's a simple patch that does what I had in mind. It allows providing
for an arbitrary schedule file in both the check and installcheck
recipes. The historic behaviour is preserved.

cheers

andrew

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

Attachments:

check-schedule-override.patchtext/x-patch; name=check-schedule-override.patchDownload
diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile
index b923ea1..7e100cb 100644
--- a/src/test/regress/GNUmakefile
+++ b/src/test/regress/GNUmakefile
@@ -127,13 +127,13 @@ tablespace-setup:
 REGRESS_OPTS = --dlpath=. $(EXTRA_REGRESS_OPTS)
 
 check: all tablespace-setup
-	$(pg_regress_check) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(EXTRA_TESTS)
+	$(pg_regress_check) $(REGRESS_OPTS) --schedule=$(srcdir)/$(if $(SCHEDULE),$(SCHEDULE),parallel)_schedule $(MAXCONNOPT) $(EXTRA_TESTS)
 
 check-tests: all tablespace-setup
 	$(pg_regress_check) $(REGRESS_OPTS) $(MAXCONNOPT) $(TESTS) $(EXTRA_TESTS)
 
 installcheck: all tablespace-setup
-	$(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/serial_schedule $(EXTRA_TESTS)
+	$(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/$(if $(SCHEDULE),$(SCHEDULE),serial)_schedule $(EXTRA_TESTS)
 
 installcheck-parallel: all tablespace-setup
 	$(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(EXTRA_TESTS)
#3Vaishnavi Prabakaran
vaishnaviprabakaran@gmail.com
In reply to: Andrew Dunstan (#2)
Re: check with serial

On Tue, May 2, 2017 at 11:30 PM, Andrew Dunstan <
andrew.dunstan@2ndquadrant.com> wrote:

Here's a simple patch that does what I had in mind. It allows providing
for an arbitrary schedule file in both the check and installcheck
recipes. The historic behaviour is preserved.

Hmm, installcheck command with SCHEDULE set as "Parallel" does not honor
"MAXCONNOPT" settings in the attached patch.

And, now after your patch, do we still need "installcheck-parallel"
command? It is redundant IMO, just give a thought.

Documentation changes("Running the Tests") are also required as the
behavior documented is now changed in this patch.

Best Regards,
Vaishnavi,
Fujitsu Australia.

#4Andrew Dunstan
andrew.dunstan@2ndquadrant.com
In reply to: Vaishnavi Prabakaran (#3)
Re: check with serial

On 05/02/2017 10:13 PM, Vaishnavi Prabakaran wrote:

On Tue, May 2, 2017 at 11:30 PM, Andrew Dunstan
<andrew.dunstan@2ndquadrant.com
<mailto:andrew.dunstan@2ndquadrant.com>> wrote:

Here's a simple patch that does what I had in mind. It allows
providing
for an arbitrary schedule file in both the check and installcheck
recipes. The historic behaviour is preserved.

Hmm, installcheck command with SCHEDULE set as "Parallel" does not
honor "MAXCONNOPT" settings in the attached patch.

good point.

And, now after your patch, do we still need "installcheck-parallel"
command? It is redundant IMO, just give a thought.

I'd be quite happy to remove the target in favor of this more general
solution.

Thoughts?

Documentation changes("Running the Tests") are also required as the
behavior documented is now changed in this patch.

Yes, agreed. Thanks for your comments.

cheers

andrew

--
Andrew Dunstan https://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

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#4)
Re: check with serial

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:

On 05/02/2017 10:13 PM, Vaishnavi Prabakaran wrote:

And, now after your patch, do we still need "installcheck-parallel"
command? It is redundant IMO, just give a thought.

I'd be quite happy to remove the target in favor of this more general
solution.

-1 --- that will break a lot of existing habits to no purpose, not to
mention creating complications for scripts used to test multiple branches.
We have intentionally maintained backwards compatibility in these testing
targets for a very long time, even though that's left us with
inconsistencies like installcheck defaulting to serial while check
defaults to parallel. Adding a new capability is not an excuse for
breaking the historical test targets.

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

#6Andrew Dunstan
andrew.dunstan@2ndquadrant.com
In reply to: Tom Lane (#5)
Re: check with serial

On 05/03/2017 10:12 AM, Tom Lane wrote:

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:

On 05/02/2017 10:13 PM, Vaishnavi Prabakaran wrote:

And, now after your patch, do we still need "installcheck-parallel"
command? It is redundant IMO, just give a thought.

I'd be quite happy to remove the target in favor of this more general
solution.

-1 --- that will break a lot of existing habits to no purpose, not to
mention creating complications for scripts used to test multiple branches.
We have intentionally maintained backwards compatibility in these testing
targets for a very long time, even though that's left us with
inconsistencies like installcheck defaulting to serial while check
defaults to parallel. Adding a new capability is not an excuse for
breaking the historical test targets.

OK

cheers

andrew

--
Andrew Dunstan https://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