Обсуждение: Let's get rid of serial_schedule

We've several times discussed doing $SUBJECT by replacing the
makefile's use of serial_schedule with calling parallel_schedule
with --max-connections=1.  This'd remove the need to maintain
two lists of regression test scripts.

I got annoyed again just now about how people seem unable to
keep the two lists in the same order, so here is a patch to
get rid of serial_schedule in that way.

(The vcregress.pl changes are untested, but they seem straightforward
enough.  I do wonder though why we spell it --max-concurrent-tests
there when the makefile uses --max-connections.)

It'd perhaps be possible to adjust pg_regress so that when
--max-connections=1 its progress output looks exactly the same
as it did with serial_schedule.  I doubt it's worth the trouble
though, unless anyone really wants that.

Any objections?

            regards, tom lane

diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile
index 95e4bc8228..5dc4bbcb00 100644
--- a/src/test/regress/GNUmakefile
+++ b/src/test/regress/GNUmakefile
@@ -83,7 +83,7 @@ regress_data_files = \
     $(wildcard $(srcdir)/output/*.source) \
     $(filter-out $(addprefix $(srcdir)/,$(input_files)),$(wildcard $(srcdir)/sql/*.sql)) \
     $(wildcard $(srcdir)/data/*.data) \
-    $(srcdir)/parallel_schedule $(srcdir)/serial_schedule $(srcdir)/resultmap
+    $(srcdir)/parallel_schedule $(srcdir)/resultmap

 install-tests: all install install-lib installdirs-tests
     $(MAKE) -C $(top_builddir)/contrib/spi install
@@ -128,7 +128,7 @@ check-tests: all | temp-install
     $(pg_regress_check) $(REGRESS_OPTS) $(MAXCONNOPT) $(TESTS) $(EXTRA_TESTS)

 installcheck: all
-    $(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/serial_schedule $(EXTRA_TESTS)
+    $(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule --max-connections=1
$(EXTRA_TESTS)

 installcheck-parallel: all
     $(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(EXTRA_TESTS)
@@ -146,7 +146,7 @@ runtest: installcheck
 runtest-parallel: installcheck-parallel

 bigtest: all
-    $(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/serial_schedule numeric_big
+    $(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule --max-connections=1 numeric_big

 bigcheck: all | temp-install
     $(pg_regress_check) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) numeric_big
diff --git a/src/test/regress/serial_schedule b/src/test/regress/serial_schedule
deleted file mode 100644
index 6e9cdf92af..0000000000
--- a/src/test/regress/serial_schedule
+++ /dev/null
@@ -1,212 +0,0 @@
-# src/test/regress/serial_schedule
-# This should probably be in an order similar to parallel_schedule.
-test: tablespace
-test: boolean
-test: char
-test: name
-test: varchar
-test: text
-test: int2
-test: int4
-test: int8
-test: oid
-test: float4
-test: float8
-test: bit
-test: numeric
-test: txid
-test: uuid
-test: enum
-test: money
-test: rangetypes
-test: pg_lsn
-test: regproc
-test: strings
-test: numerology
-test: point
-test: lseg
-test: line
-test: box
-test: path
-test: polygon
-test: circle
-test: date
-test: time
-test: timetz
-test: timestamp
-test: timestamptz
-test: interval
-test: inet
-test: macaddr
-test: macaddr8
-test: multirangetypes
-test: create_function_0
-test: geometry
-test: horology
-test: tstypes
-test: regex
-test: type_sanity
-test: opr_sanity
-test: misc_sanity
-test: comments
-test: expressions
-test: unicode
-test: xid
-test: mvcc
-test: create_function_1
-test: create_type
-test: create_table
-test: create_function_2
-test: copy
-test: copyselect
-test: copydml
-test: insert
-test: insert_conflict
-test: create_misc
-test: create_operator
-test: create_procedure
-test: create_index
-test: create_index_spgist
-test: create_view
-test: index_including
-test: index_including_gist
-test: create_aggregate
-test: create_function_3
-test: create_cast
-test: constraints
-test: triggers
-test: select
-test: inherit
-test: typed_table
-test: vacuum
-test: drop_if_exists
-test: updatable_views
-test: roleattributes
-test: create_am
-test: hash_func
-test: errors
-test: infinite_recurse
-test: sanity_check
-test: select_into
-test: select_distinct
-test: select_distinct_on
-test: select_implicit
-test: select_having
-test: subselect
-test: union
-test: case
-test: join
-test: aggregates
-test: transactions
-ignore: random
-test: random
-test: portals
-test: arrays
-test: btree_index
-test: hash_index
-test: update
-test: delete
-test: namespace
-test: prepared_xacts
-test: brin
-test: gin
-test: gist
-test: spgist
-test: privileges
-test: init_privs
-test: security_label
-test: collate
-test: matview
-test: lock
-test: replica_identity
-test: rowsecurity
-test: object_address
-test: tablesample
-test: groupingsets
-test: drop_operator
-test: password
-test: identity
-test: generated
-test: join_hash
-test: brin_bloom
-test: brin_multi
-test: create_table_like
-test: alter_generic
-test: alter_operator
-test: misc
-test: async
-test: dbsize
-test: misc_functions
-test: sysviews
-test: tsrf
-test: tid
-test: tidscan
-test: tidrangescan
-test: collate.icu.utf8
-test: incremental_sort
-test: rules
-test: psql
-test: psql_crosstab
-test: amutils
-test: stats_ext
-test: collate.linux.utf8
-test: select_parallel
-test: write_parallel
-test: publication
-test: subscription
-test: select_views
-test: portals_p2
-test: foreign_key
-test: cluster
-test: dependency
-test: guc
-test: bitmapops
-test: combocid
-test: tsearch
-test: tsdicts
-test: foreign_data
-test: window
-test: xmlmap
-test: functional_deps
-test: advisory_lock
-test: indirect_toast
-test: equivclass
-test: json
-test: jsonb
-test: json_encoding
-test: jsonpath
-test: jsonpath_encoding
-test: jsonb_jsonpath
-test: plancache
-test: limit
-test: plpgsql
-test: copy2
-test: temp
-test: domain
-test: rangefuncs
-test: prepare
-test: conversion
-test: truncate
-test: alter_table
-test: sequence
-test: polymorphism
-test: rowtypes
-test: returning
-test: largeobject
-test: with
-test: xml
-test: partition_join
-test: partition_prune
-test: reloptions
-test: hash_part
-test: indexing
-test: partition_aggregate
-test: partition_info
-test: tuplesort
-test: explain
-test: compression
-test: resultcache
-test: event_trigger
-test: oidjoins
-test: fast_default
-test: stats
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 860899c039..7dd3989b9c 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -106,12 +106,17 @@ exit 0;
 sub installcheck_internal
 {
     my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+    # for backwards compatibility, interpret "serial" as parallel tests
+    my $nctests = 20;
+    $nctests = 1 if $schedule eq 'serial';
+    $schedule = 'parallel' if $schedule eq 'serial';
+
     my @args = (
         "../../../$Config/pg_regress/pg_regress",
         "--dlpath=.",
         "--bindir=../../../$Config/psql",
         "--schedule=${schedule}_schedule",
-        "--max-concurrent-tests=20",
+        "--max-concurrent-tests=$nctests",
         "--encoding=SQL_ASCII",
         "--no-locale");
     push(@args, $maxconn) if $maxconn;
@@ -132,6 +137,11 @@ sub installcheck
 sub check
 {
     my $schedule = shift || 'parallel';
+    # for backwards compatibility, interpret "serial" as parallel tests
+    my $nctests = 20;
+    $nctests = 1 if $schedule eq 'serial';
+    $schedule = 'parallel' if $schedule eq 'serial';
+
     InstallTemp();
     chdir "${topdir}/src/test/regress";
     my @args = (
@@ -139,7 +149,7 @@ sub check
         "--dlpath=.",
         "--bindir=",
         "--schedule=${schedule}_schedule",
-        "--max-concurrent-tests=20",
+        "--max-concurrent-tests=$nctests",
         "--encoding=SQL_ASCII",
         "--no-locale",
         "--temp-instance=./tmp_check");

> On 11 May 2021, at 20:58, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> We've several times discussed doing $SUBJECT by replacing the
> makefile's use of serial_schedule with calling parallel_schedule
> with --max-connections=1.  This'd remove the need to maintain
> two lists of regression test scripts.
>
> I got annoyed again just now about how people seem unable to
> keep the two lists in the same order, so here is a patch to
> get rid of serial_schedule in that way.

Very much +1 on this approach.

> +    # for backwards compatibility, interpret "serial" as parallel tests

This comment may seem odd without reading the commit message.  Perhaps it can
be reworded to "..as parallel tests running with a single worker" or something
along those lines?

--
Daniel Gustafsson        https://vmware.com/

On 11/05/2021 21:58, Tom Lane wrote:
> We've several times discussed doing $SUBJECT by replacing the
> makefile's use of serial_schedule with calling parallel_schedule
> with --max-connections=1.  This'd remove the need to maintain
> two lists of regression test scripts.
> 
> I got annoyed again just now about how people seem unable to
> keep the two lists in the same order, so here is a patch to
> get rid of serial_schedule in that way.

+1

> +    # for backwards compatibility, interpret "serial" as parallel tests

This comment isn't great, IMHO. How about:

# for backwards comopatibility, "serial" runs the tests in
# parallel_schedule one by one.

- Heikki

Heikki Linnakangas <hlinnaka@iki.fi> writes:
> On 11/05/2021 21:58, Tom Lane wrote:
>> +    # for backwards compatibility, interpret "serial" as parallel tests

> This comment isn't great, IMHO. How about:

> # for backwards comopatibility, "serial" runs the tests in
> # parallel_schedule one by one.

Yeah, and on closer inspection, the code is wrong too :-(.
I'd confused --max-concurrent-tests with --max-connections,
but they're different.

Also, I did take a look at pg_regress.c, and confirmed my
fear that getting it to duplicate the serial output style
would be kind of messy.

            regards, tom lane

diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile
index 95e4bc8228..5dc4bbcb00 100644
--- a/src/test/regress/GNUmakefile
+++ b/src/test/regress/GNUmakefile
@@ -83,7 +83,7 @@ regress_data_files = \
     $(wildcard $(srcdir)/output/*.source) \
     $(filter-out $(addprefix $(srcdir)/,$(input_files)),$(wildcard $(srcdir)/sql/*.sql)) \
     $(wildcard $(srcdir)/data/*.data) \
-    $(srcdir)/parallel_schedule $(srcdir)/serial_schedule $(srcdir)/resultmap
+    $(srcdir)/parallel_schedule $(srcdir)/resultmap

 install-tests: all install install-lib installdirs-tests
     $(MAKE) -C $(top_builddir)/contrib/spi install
@@ -128,7 +128,7 @@ check-tests: all | temp-install
     $(pg_regress_check) $(REGRESS_OPTS) $(MAXCONNOPT) $(TESTS) $(EXTRA_TESTS)

 installcheck: all
-    $(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/serial_schedule $(EXTRA_TESTS)
+    $(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule --max-connections=1
$(EXTRA_TESTS)

 installcheck-parallel: all
     $(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(EXTRA_TESTS)
@@ -146,7 +146,7 @@ runtest: installcheck
 runtest-parallel: installcheck-parallel

 bigtest: all
-    $(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/serial_schedule numeric_big
+    $(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule --max-connections=1 numeric_big

 bigcheck: all | temp-install
     $(pg_regress_check) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) numeric_big
diff --git a/src/test/regress/serial_schedule b/src/test/regress/serial_schedule
deleted file mode 100644
index 6e9cdf92af..0000000000
--- a/src/test/regress/serial_schedule
+++ /dev/null
@@ -1,212 +0,0 @@
-# src/test/regress/serial_schedule
-# This should probably be in an order similar to parallel_schedule.
-test: tablespace
-test: boolean
-test: char
-test: name
-test: varchar
-test: text
-test: int2
-test: int4
-test: int8
-test: oid
-test: float4
-test: float8
-test: bit
-test: numeric
-test: txid
-test: uuid
-test: enum
-test: money
-test: rangetypes
-test: pg_lsn
-test: regproc
-test: strings
-test: numerology
-test: point
-test: lseg
-test: line
-test: box
-test: path
-test: polygon
-test: circle
-test: date
-test: time
-test: timetz
-test: timestamp
-test: timestamptz
-test: interval
-test: inet
-test: macaddr
-test: macaddr8
-test: multirangetypes
-test: create_function_0
-test: geometry
-test: horology
-test: tstypes
-test: regex
-test: type_sanity
-test: opr_sanity
-test: misc_sanity
-test: comments
-test: expressions
-test: unicode
-test: xid
-test: mvcc
-test: create_function_1
-test: create_type
-test: create_table
-test: create_function_2
-test: copy
-test: copyselect
-test: copydml
-test: insert
-test: insert_conflict
-test: create_misc
-test: create_operator
-test: create_procedure
-test: create_index
-test: create_index_spgist
-test: create_view
-test: index_including
-test: index_including_gist
-test: create_aggregate
-test: create_function_3
-test: create_cast
-test: constraints
-test: triggers
-test: select
-test: inherit
-test: typed_table
-test: vacuum
-test: drop_if_exists
-test: updatable_views
-test: roleattributes
-test: create_am
-test: hash_func
-test: errors
-test: infinite_recurse
-test: sanity_check
-test: select_into
-test: select_distinct
-test: select_distinct_on
-test: select_implicit
-test: select_having
-test: subselect
-test: union
-test: case
-test: join
-test: aggregates
-test: transactions
-ignore: random
-test: random
-test: portals
-test: arrays
-test: btree_index
-test: hash_index
-test: update
-test: delete
-test: namespace
-test: prepared_xacts
-test: brin
-test: gin
-test: gist
-test: spgist
-test: privileges
-test: init_privs
-test: security_label
-test: collate
-test: matview
-test: lock
-test: replica_identity
-test: rowsecurity
-test: object_address
-test: tablesample
-test: groupingsets
-test: drop_operator
-test: password
-test: identity
-test: generated
-test: join_hash
-test: brin_bloom
-test: brin_multi
-test: create_table_like
-test: alter_generic
-test: alter_operator
-test: misc
-test: async
-test: dbsize
-test: misc_functions
-test: sysviews
-test: tsrf
-test: tid
-test: tidscan
-test: tidrangescan
-test: collate.icu.utf8
-test: incremental_sort
-test: rules
-test: psql
-test: psql_crosstab
-test: amutils
-test: stats_ext
-test: collate.linux.utf8
-test: select_parallel
-test: write_parallel
-test: publication
-test: subscription
-test: select_views
-test: portals_p2
-test: foreign_key
-test: cluster
-test: dependency
-test: guc
-test: bitmapops
-test: combocid
-test: tsearch
-test: tsdicts
-test: foreign_data
-test: window
-test: xmlmap
-test: functional_deps
-test: advisory_lock
-test: indirect_toast
-test: equivclass
-test: json
-test: jsonb
-test: json_encoding
-test: jsonpath
-test: jsonpath_encoding
-test: jsonb_jsonpath
-test: plancache
-test: limit
-test: plpgsql
-test: copy2
-test: temp
-test: domain
-test: rangefuncs
-test: prepare
-test: conversion
-test: truncate
-test: alter_table
-test: sequence
-test: polymorphism
-test: rowtypes
-test: returning
-test: largeobject
-test: with
-test: xml
-test: partition_join
-test: partition_prune
-test: reloptions
-test: hash_part
-test: indexing
-test: partition_aggregate
-test: partition_info
-test: tuplesort
-test: explain
-test: compression
-test: resultcache
-test: event_trigger
-test: oidjoins
-test: fast_default
-test: stats
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 860899c039..059f8a55b3 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -106,6 +106,11 @@ exit 0;
 sub installcheck_internal
 {
     my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+    # for backwards compatibility, "serial" runs the tests in
+    # parallel_schedule one by one.
+    $maxconn = "--max_connections=1" if $schedule eq 'serial';
+    $schedule = 'parallel' if $schedule eq 'serial';
+
     my @args = (
         "../../../$Config/pg_regress/pg_regress",
         "--dlpath=.",
@@ -132,6 +137,11 @@ sub installcheck
 sub check
 {
     my $schedule = shift || 'parallel';
+    # for backwards compatibility, "serial" runs the tests in
+    # parallel_schedule one by one.
+    $maxconn = "--max_connections=1" if $schedule eq 'serial';
+    $schedule = 'parallel' if $schedule eq 'serial';
+
     InstallTemp();
     chdir "${topdir}/src/test/regress";
     my @args = (

Daniel Gustafsson <daniel@yesql.se> writes:
> On 11 May 2021, at 20:58, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> +    # for backwards compatibility, interpret "serial" as parallel tests

> This comment may seem odd without reading the commit message.  Perhaps it can
> be reworded to "..as parallel tests running with a single worker" or something
> along those lines?

I liked Heikki's phrasing, so the v2 patch does it his way.

            regards, tom lane

> On 11 May 2021, at 21:44, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Daniel Gustafsson <daniel@yesql.se> writes:
>> On 11 May 2021, at 20:58, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> +    # for backwards compatibility, interpret "serial" as parallel tests
>
>> This comment may seem odd without reading the commit message.  Perhaps it can
>> be reworded to "..as parallel tests running with a single worker" or something
>> along those lines?
>
> I liked Heikki's phrasing, so the v2 patch does it his way.

Agreed, I like his phrasing too.

--
Daniel Gustafsson        https://vmware.com/