Re: [HACKERS] [PATCHES] pg_regress in C

2006-08-18 Thread Magnus Hagander
> > Per discussion at the conference:
> > In order to run the regression tests on Windows without msys,
> > pg_regress needs to be reimplemnted in C.
> 
> This has some minor portability issues (macros with ... aren't
> portable, for instance) but I think it's something we need to do.
> Barring objections I'm going to clean up and apply it.

Thanks for this, including all the followup patches :-)


//Magnus


---(end of broadcast)---
TIP 6: explain analyze is your friend


Re: [HACKERS] [PATCHES] pg_regress in C

2006-07-19 Thread Tom Lane
Martijn van Oosterhout  writes:
> On Tue, Jul 18, 2006 at 10:46:04PM -0400, Tom Lane wrote:
>> ...  One reason I didn't try to do this is I'm a bit hesitant to
>> write a signal handler that does anything as interesting as a system()
>> call, which would seem to be necessary to duplicate what the shell
>> script did.  Comments?

> It might not actually be unsafe, given system() actually blocks on
> waitpid() which is specifically listed as a "safe" function. I'm a bit
> confused though, because system() generally sets the parent to ignore
> SIGINT which running the child process. That means the postmaster is
> being killed but pg_regress is not? If this is the case, then you won't
> be able to catch SIGINT anyway.

The cases of interest are where the (new) code goes through
spawn_process, which does a fork() and then calls system() in the
child.  So the intermediate child is probably SIGINT-blocked, but
pg_regress itself isn't.

I was planning to rewrite spawn_process anyway, because I noticed that
as it's currently set up, we are actually creating four(!) processes per
parallel test: the pg_regress child, the shell invoked by system, the
psql invoked by the shell, and the connected backend.  That's even worse
than the shell script, which (at least on my system) used three processes.
I believe we can get down to two (psql and backend) if spawn_process
exec's the shell instead of using system, and also puts "exec" in front
of the command string passed to the shell.  So in that scenario there'd
not be any signal-blocking going on anyway.

That still leaves us with the question of whether pg_regress needs to do
anything special when signaled, though.

regards, tom lane

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly


Re: [HACKERS] [PATCHES] pg_regress in C

2006-07-18 Thread Martijn van Oosterhout
On Tue, Jul 18, 2006 at 10:46:04PM -0400, Tom Lane wrote:
> ...  One reason I didn't try to do this is I'm a bit hesitant to
> write a signal handler that does anything as interesting as a system()
> call, which would seem to be necessary to duplicate what the shell
> script did.  Comments?

It might not actually be unsafe, given system() actually blocks on
waitpid() which is specifically listed as a "safe" function. I'm a bit
confused though, because system() generally sets the parent to ignore
SIGINT which running the child process. That means the postmaster is
being killed but pg_regress is not? If this is the case, then you won't
be able to catch SIGINT anyway.

Also, the kernel sending it to everyone on the same terminal is (AIUI)
only true if you're running under the same session ID. postgres only
daemonizes itself to be immune from frontend terminal interrupts (using
setsid) when silent_mode is on. I think it defaults to off, which is
probably why it works at all.

Anyway, the signal handling for Windows involves a seperate thead AIUI
which may make it easier. It might be interesting to see how bash does
it. 

Hope this helps,
-- 
Martijn van Oosterhout  http://svana.org/kleptog/
> From each according to his ability. To each according to his ability to 
> litigate.


signature.asc
Description: Digital signature


Re: [HACKERS] [PATCHES] pg_regress in C

2006-07-18 Thread Tom Lane
"Magnus Hagander" <[EMAIL PROTECTED]> writes:
> Per discussion at the conference:
> In order to run the regression tests on Windows without msys, pg_regress
> needs to be reimplemnted in C.

Patch committed after significant further work.  As committed,
pg_regress.c is pretty nearly an exact replacement for the shell script;
the only significant deviation is that the --temp_install switch's
argument is required not optional.  (This is because our homegrown
version of getopt_long doesn't allow optional arguments.  Maybe that
should be fixed sometime.)

There is one possibly important loose end: the shell script makes an
effort to catch signals and shut down the temp postmaster before
exiting, while there's no such code in the C version.  I'm not sure
if it's necessary.  At least on my machine, if you type control-C while
the tests are running then the kernel sends SIGINT to everything that's
part of the terminal's process group, which will include the postmaster
--- so the shutdown happens anyway.  I have no idea if that'll work on
Windows...  One reason I didn't try to do this is I'm a bit hesitant to
write a signal handler that does anything as interesting as a system()
call, which would seem to be necessary to duplicate what the shell
script did.  Comments?

regards, tom lane

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


Re: [HACKERS] [PATCHES] pg_regress in C

2006-07-17 Thread Tom Lane
"Magnus Hagander" <[EMAIL PROTECTED]> writes:
> Per discussion at the conference:
> In order to run the regression tests on Windows without msys, pg_regress
> needs to be reimplemnted in C.

This has some minor portability issues (macros with ... aren't portable,
for instance) but I think it's something we need to do.  Barring
objections I'm going to clean up and apply it.

regards, tom lane

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly