Обсуждение: [HACKERS] separate serial_schedule useful?
I noticed that the test "hash_func" was listed in parallel_schedule but not in serial_schedule. I have seen that a few times recently where a patch proposes to add a new test file but forgets to add it to the serial_schedule. I wonder whether it's still useful to keep two separate test lists. I think we could just replace make installcheck with what make installcheck-parallel MAX_CONNECTIONS=1 does. Thoughts? -- 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
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> I noticed that the test "hash_func" was listed in parallel_schedule but
> not in serial_schedule. I have seen that a few times recently where a
> patch proposes to add a new test file but forgets to add it to the
> serial_schedule.
Yeah, this is way too routine :-(
> I wonder whether it's still useful to keep two separate test lists. I
> think we could just replace make installcheck with what make
> installcheck-parallel MAX_CONNECTIONS=1 does. Thoughts?
Hm, that seems like potentially a good idea. I can't see an argument
against it offhand.
The other routine mistake, which I see Robert just made again,
is to break the at-most-twenty-parallel-tests-at-once convention.
I wonder if we can get in some sort of automated check for 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
On Fri, Oct 6, 2017 at 4:16 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > The other routine mistake, which I see Robert just made again, > is to break the at-most-twenty-parallel-tests-at-once convention. > I wonder if we can get in some sort of automated check for that. Argh. We can argue about whether that's my mistake or Ashutosh's mistake, but I do try to catch these things. It's just that there are so many rules that require a committer to (a) know the rule and (b) remember to enforce the rule that it's really easy to miss one. And I do know that rule, but it slipped my mind in the course of trying to make sure that we'd covered all the bases in terms of the feature itself. There's no reason why pg_regress couldn't have a --bail-if-group-size-exceeds=N argument, or why we couldn't have a separate Perl script to validate the schedule file as part of the build process. I feel like the need to manually enforce so many tedious coding rules is a real limiting factor on our ability to (a) involve new people in the project and (b) get their work committed. -- 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
Robert Haas <robertmhaas@gmail.com> writes:
> On Fri, Oct 6, 2017 at 4:16 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> The other routine mistake, which I see Robert just made again,
>> is to break the at-most-twenty-parallel-tests-at-once convention.
>> I wonder if we can get in some sort of automated check for that.
> There's no reason why pg_regress couldn't have a
> --bail-if-group-size-exceeds=N argument, or why we couldn't have a
> separate Perl script to validate the schedule file as part of the
> build process.
I'd go for the former approach; seems like less new code and fewer cycles
used to enforce the rule.
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
I wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> There's no reason why pg_regress couldn't have a
>> --bail-if-group-size-exceeds=N argument, or why we couldn't have a
>> separate Perl script to validate the schedule file as part of the
>> build process.
> I'd go for the former approach; seems like less new code and fewer cycles
> used to enforce the rule.
Concretely, how about the attached? (Obviously we'd have to fix
parallel_schedule before committing this.)
regards, tom lane
diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile
index b923ea1..56cd202 100644
*** a/src/test/regress/GNUmakefile
--- b/src/test/regress/GNUmakefile
*************** tablespace-setup:
*** 124,130 ****
## Run tests
##
! REGRESS_OPTS = --dlpath=. $(EXTRA_REGRESS_OPTS)
check: all tablespace-setup
$(pg_regress_check) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(EXTRA_TESTS)
--- 124,130 ----
## Run tests
##
! REGRESS_OPTS = --dlpath=. --max-concurrent-tests=20 $(EXTRA_REGRESS_OPTS)
check: all tablespace-setup
$(pg_regress_check) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(EXTRA_TESTS)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index abb742b..ff979b8 100644
*** a/src/test/regress/pg_regress.c
--- b/src/test/regress/pg_regress.c
*************** char *launcher = NULL;
*** 78,83 ****
--- 78,84 ----
static _stringlist *loadlanguage = NULL;
static _stringlist *loadextension = NULL;
static int max_connections = 0;
+ static int max_concurrent_tests = 0;
static char *encoding = NULL;
static _stringlist *schedulelist = NULL;
static _stringlist *extra_tests = NULL;
*************** run_schedule(const char *schedule, test_
*** 1691,1696 ****
--- 1692,1704 ----
wait_for_tests(pids, statuses, NULL, 1);
/* status line is finished below */
}
+ else if (max_concurrent_tests > 0 && max_concurrent_tests < num_tests)
+ {
+ /* can't print scbuf here, it's already been trashed */
+ fprintf(stderr, _("too many tests (more than %d) in schedule file \"%s\" line %d\n"),
+ max_concurrent_tests, schedule, line_num);
+ exit(2);
+ }
else if (max_connections > 0 && max_connections < num_tests)
{
int oldest = 0;
*************** help(void)
*** 1999,2004 ****
--- 2007,2014 ----
printf(_(" tests; can appear multiple times\n"));
printf(_(" --max-connections=N maximum number of concurrent connections\n"));
printf(_(" (default is 0, meaning unlimited)\n"));
+ printf(_(" --max-concurrent-tests=N maximum number of concurrent tests in schedule\n"));
+ printf(_(" (default is 0, meaning unlimited)\n"));
printf(_(" --outputdir=DIR place output files in DIR (default \".\")\n"));
printf(_(" --schedule=FILE use test ordering schedule from FILE\n"));
printf(_(" (can be used multiple times to concatenate)\n"));
*************** regression_main(int argc, char *argv[],
*** 2048,2053 ****
--- 2058,2064 ----
{"launcher", required_argument, NULL, 21},
{"load-extension", required_argument, NULL, 22},
{"config-auth", required_argument, NULL, 24},
+ {"max-concurrent-tests", required_argument, NULL, 25},
{NULL, 0, NULL, 0}
};
*************** regression_main(int argc, char *argv[],
*** 2161,2166 ****
--- 2172,2180 ----
case 24:
config_auth_datadir = pg_strdup(optarg);
break;
+ case 25:
+ max_concurrent_tests = atoi(optarg);
+ break;
default:
/* getopt_long already emitted a complaint */
fprintf(stderr, _("\nTry \"%s -h\" for more information.\n"),
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Oct 7, 2017 at 10:19 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > I wrote: >> Robert Haas <robertmhaas@gmail.com> writes: Sorry, my bad. I wasn't aware of this rule. I should have looked at the beginning of the file for any rules. >>> There's no reason why pg_regress couldn't have a >>> --bail-if-group-size-exceeds=N argument, or why we couldn't have a >>> separate Perl script to validate the schedule file as part of the >>> build process. > >> I'd go for the former approach; seems like less new code and fewer cycles >> used to enforce the rule. > > Concretely, how about the attached? (Obviously we'd have to fix > parallel_schedule before committing this.) > Thanks, this will help. May be we should set default to 20 instead of unlimited. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers