|From:||Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>|
|To:||Peter Eisentraut <peter_e(at)gmx(dot)net>|
|Subject:||Re: improving speed of make check-world|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
Here is a review:
The version 2 of the patch applies cleanly on current head.
The ability to generate and reuse a temporary installation for different
tests looks quite useful, thus putting install out of pg_regress and in
make seems reasonnable.
However I'm wondering whether there could be some use case of pg_regress
where doing the install might be useful nevertheless, say for manually
doing things on the command line, in which case keeping the feature, even
if not used by default, could be nice?
About changes: I think that more comments would be welcome, eg
There is not a single documentation change. Should there be some? Hmmm...
I have not found much documentation about "pg_regress"...
I have tested make check, check-world, as well as make check in contrib
sub-directories. It seems to work fine in sequential mode.
Running "make -j2 check-world" does not work because "initdb" is not found
by "pg_regress". but "make -j1 check-world" does work fine. It seems that
some dependencies might be missing and there is a race condition between
temporary install and running some checks?? Maybe it is not expected to
work anyway? See below suggestions to make it work.
However in this case the error message passed by pg_regress contains an
pg_regress: initdb failed
Examine /home/fabien/DEV/GIT/postgresql/contrib/btree_gin/log/initdb.log for the reason.
Command was: "initdb" -D "/home/fabien/DEV/GIT/postgresql/contrib/btree_gin/./tmp_check/data" --noclean --nosync > "/home/fabien/DEV/GIT/postgresql/contrib/btree_gin/./tmp_check/log/initdb.log" 2>&1
make: *** [check] Error 2
The error messages point to non existing log file (./tmp_check is
missing), the temp_instance variable should be used in the error message
as well as in the commands. Maybe other error messages have the same
I do not like much the MAKELEVEL hack in a phony target. When running in
parallel, several makes may have the same level anyway, not sure what
would happen... Maybe it is the origin of the race condition which makes
initdb not to be found above. I would suggest to try to rely on
dependences, maybe something like the following could work to ensure that
an installation is done once and only once per make invocation, whatever
the underlying parallelism & levels, as well as a possibility to reuse the
# must be defined somewhere common to all makefiles
# define a nanosecond timestamp
export MAKE_NONCE := $(shell date +%s.%N)
# actual new tmp installation
$(RM) -r ./tmp_install
# create tmp installation...
# tmp installation for the nonce
# generate a new tmp installation by default
# "make USE_TMP_INSTALL=1 ..." reuses a previous installation if available
TMP_INSTALL = .tmp_install
TMP_INSTALL = .tmp_install.$(MAKE_NONCE)
endif # USE_TMP_INSTALL
# do EXTRA_INSTALL
> MSVC build is not yet adjusted for this. Looking at vcregress.pl, this
> should be fairly straightforward.
No idea about that point.
|Next Message||Christoph Berg||2014-08-31 10:01:11||Re: pg_filedump for 9.4?|
|Previous Message||Fabien COELHO||2014-08-31 06:44:01||Re: postgresql latency & bgwriter not doing its job|