Re: Win32 testing needed

Lists: pgsql-hackers-win32
From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers-win32(at)postgresql(dot)org
Subject: Win32 testing needed
Date: 2004-08-05 23:51:30
Message-ID: 3975.1091749890@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

I have just committed a somewhat trimmed-down version of Andreas Pflug's
syslogger patch. It seems to work okay on Unix (with or without
EXEC_BACKEND) but I'm not in a position to test it on Windows. Would
someone give it a try and report back? Please check in particular that
the logger shuts down cleanly after the postmaster is gone.

Also, Andreas' patch to enable symlinks (ie tablespaces) on Windows
looks very interesting, but with the hour so late we need confirmation
from someone else that it works. Anyone?

regards, tom lane


From: markir(at)coretech(dot)co(dot)nz
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers-win32(at)postgresql(dot)org
Subject: Re: Win32 testing needed
Date: 2004-08-06 04:11:05
Message-ID: 1091765465.95f86f85b5aff@mail.coretech.co.nz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

I can build and check both these on win 2003 server at approx 1800-1900 my time
(it's 1615 here right now). In that advent that I cannot get access to said
machine, I can do the checks on win 2000 pro tomorrow morning.

Let me know if that sort of time frame is ok.

regards

Mark

Quoting Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:

> I have just committed a somewhat trimmed-down version of Andreas Pflug's
> syslogger patch. It seems to work okay on Unix (with or without
> EXEC_BACKEND) but I'm not in a position to test it on Windows. Would
> someone give it a try and report back? Please check in particular that
> the logger shuts down cleanly after the postmaster is gone.
>
> Also, Andreas' patch to enable symlinks (ie tablespaces) on Windows
> looks very interesting, but with the hour so late we need confirmation
> from someone else that it works. Anyone?
>
> regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster
>


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: markir(at)coretech(dot)co(dot)nz
Cc: pgsql-hackers-win32(at)postgresql(dot)org
Subject: Re: Win32 testing needed
Date: 2004-08-06 04:15:31
Message-ID: 6669.1091765731@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

> Let me know if that sort of time frame is ok.

Sure... much appreciated ...

regards, tom lane


From: Mark Kirkwood <markir(at)coretech(dot)co(dot)nz>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers-win32(at)postgresql(dot)org
Subject: Re: Win32 testing needed
Date: 2004-08-06 05:40:40
Message-ID: 411319D8.6070001@coretech.co.nz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

just tried to build on win2003 and get an error:

./configure --prefix="c:/progra~1/pgsql" --with-libs=/usr/local/lib
--with-includes=/usr/local/include
make

......

dlltool --dllname postgres.exe --output-exp postgres.exp --def postgres.def

gcc -O2 -fno-strict-aliasing -Wall -Wmissing-prototypes
-Wmissing-declarations -L../../src/port -L/usr/local/lib -o
postgres.exe -Wl,--base-file,postgres.base postgres.exp access/SUBSYS.o
bootstrap/SUBSYS.o catalog/SUBSYS.o parser/SUBSYS.o commands/SUBSYS.o
executor/SUBSYS.o lib/SUBSYS.o libpq/SUBSYS.o main/SUBSYS.o
nodes/SUBSYS.o optimizer/SUBSYS.o port/SUBSYS.o postmaster/SUBSYS.o
regex/SUBSYS.o rewrite/SUBSYS.o storage/SUBSYS.o tcop/SUBSYS.o
utils/SUBSYS.o ../../src/timezone/SUBSYS.o -lpgport -lz -lwsock32 -lm
-lws2_32

../../src/port/libpgport.a(dirmod.o)(.text+0x33a):dirmod.c: undefined
reference to `_imp__CurrentMemoryContext'

../../src/port/libpgport.a(dirmod.o)(.text+0x39b):dirmod.c: undefined
reference to `_imp__CurrentMemoryContext'

make[2]: *** [postgres] Error 1

make[2]: Leaving directory
`/home/markir/develop/c/postgresql-8.0devel/src/backend'


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Mark Kirkwood <markir(at)coretech(dot)co(dot)nz>
Cc: pgsql-hackers-win32(at)postgresql(dot)org
Subject: Re: Win32 testing needed
Date: 2004-08-06 05:57:50
Message-ID: 7831.1091771870@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

Mark Kirkwood <markir(at)coretech(dot)co(dot)nz> writes:
> ../../src/port/libpgport.a(dirmod.o)(.text+0x33a):dirmod.c: undefined
> reference to `_imp__CurrentMemoryContext'

> ../../src/port/libpgport.a(dirmod.o)(.text+0x39b):dirmod.c: undefined
> reference to `_imp__CurrentMemoryContext'

Hmmm ... the only recent change I can see that looks likely to be
related is this one:

2004-08-01 14:07 tgl

* src/backend/Makefile: Add libpgport to postgres.def for Windows
build. Per Magnus Hagander.

If you back that out, does it help?

regards, tom lane


From: Andreas Pflug <pgadmin(at)pse-consulting(dot)de>
To: Mark Kirkwood <markir(at)coretech(dot)co(dot)nz>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers-win32(at)postgresql(dot)org
Subject: Re: Win32 testing needed
Date: 2004-08-06 10:14:44
Message-ID: 41135A14.9030605@pse-consulting.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

Mark Kirkwood wrote:
>
> ../../src/port/libpgport.a(dirmod.o)(.text+0x33a):dirmod.c: undefined
> reference to `_imp__CurrentMemoryContext'
>

I know this message. It does not appear for me if I do a clean checkout,
and make from the pgsql top directory. If I make from src/backend (to
skip the lengthy locale makes on my slow machine), that said undefined
reference comes up, and won't go even if I make from the top dir unless
dirmod.c is touched.

Apparently the reason is crossover referencing between libs, i.e.
libpgport.a using libpostgres.a and vice versa.

So it's better if src/port sources don't use backend's functions.

Regards,
Andreas


From: Andreas Pflug <pgadmin(at)pse-consulting(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers-win32(at)postgresql(dot)org, Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
Subject: Re: Win32 testing needed
Date: 2004-08-06 13:09:13
Message-ID: 411382F9.2060905@pse-consulting.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

Tom Lane wrote:
> I have just committed a somewhat trimmed-down version of Andreas Pflug's
> syslogger patch. It seems to work okay on Unix (with or without
> EXEC_BACKEND) but I'm not in a position to test it on Windows. Would
> someone give it a try and report back? Please check in particular that
> the logger shuts down cleanly after the postmaster is gone.

Attached a patch with several issues resolved; only win32 checked.

After your changes, the error from ReadFile is not ERROR_HANDLE_EOF any
more, but ERROR_PIPE_BROKEN (which should be expected either), check is
done for both now.

The logger should *not* use proc_exit but exit(0), because proc_exit
might try to elog something, after we just closed the log file. IMHO
there's nothing left to cleanup anyway.

Changing setvbuf to use line buffered mode broke win32; apparently a
line is not a line there.... changed back to _IONBUF which should be
identical in result because we're always writing a complete line.

An observation I didn't track down so far:
Some LOG messages (e.g. the final logger shutdown, or "received fast
shutdown request") don't have proper CRLF line ending in win32, but only
LF.

If the logger subprocess is killed, it will come up again ok, but
redirecting to NULL_DEV doesn't work (open returns -1; that's what I had
realStdErr for). Opening c:\temp\dummy instead does help, but that's not
a solution. So what now? I'd propose inheriting the old stderr handle
instead of redirection_done so it can be reused in this case, as in my
original posting.

Finally, you don't seem to be a friend of a logfile rotation user
trigger... Please consider including it anyway.

Regards,
Andreas

Attachment Content-Type Size
logfile.patch text/x-patch 5.5 KB

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andreas Pflug <pgadmin(at)pse-consulting(dot)de>
Cc: pgsql-hackers-win32(at)postgresql(dot)org, Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
Subject: Re: Win32 testing needed
Date: 2004-08-06 14:17:07
Message-ID: 11498.1091801827@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

Andreas Pflug <pgadmin(at)pse-consulting(dot)de> writes:
> Attached a patch with several issues resolved; only win32 checked.

> After your changes, the error from ReadFile is not ERROR_HANDLE_EOF any
> more, but ERROR_PIPE_BROKEN (which should be expected either), check is
> done for both now.

Got it.

> The logger should *not* use proc_exit but exit(0), because proc_exit
> might try to elog something, after we just closed the log file. IMHO
> there's nothing left to cleanup anyway.

No good. If we can't survive exiting via proc_exit() then we're in deep
trouble anyway, because that is the path that any elog(ERROR) will take.
It might be best to just leave syslogFile open --- it should be properly
flushed and closed by exit() anyway, no?

> Changing setvbuf to use line buffered mode broke win32; apparently a
> line is not a line there.... changed back to _IONBUF which should be
> identical in result because we're always writing a complete line.

[ looks around ... ] Oh, we dealt with this before: Windows treats
_IOLBF the same as _IOFBF, which we don't want. Okay, ifdef time.
We do want _IOLBF on every other platform.

> An observation I didn't track down so far:
> Some LOG messages (e.g. the final logger shutdown, or "received fast
> shutdown request") don't have proper CRLF line ending in win32, but only
> LF.

Weird. No ideas about that. Can you determine whether the data coming
through the pipe has the problem?

> If the logger subprocess is killed, it will come up again ok, but
> redirecting to NULL_DEV doesn't work (open returns -1; that's what I had
> realStdErr for).

Why doesn't it work? Do we just need a different spelling of
"/dev/null" for Windows? Worst case, we could forcibly close
fileno(stderr) and just not have it pointing at anything if the
open of NULL_DEV doesn't work ... we never check ferror(stderr)
so it wouldn't really matter if output fails ...

> So what now? I'd propose inheriting the old stderr handle
> instead of redirection_done so it can be reused in this case, as in my
> original posting.

No, that was too much of a mess to solve a non-problem. We do not
actually care whether stderr works in the logger, because everything it
has to say should go through elog anyway. All we really care about is
that stderr is *not* still connected to the pipe.

I left stderr output enabled in the first instantation of the logger,
because it was easy to do and might provide a way to help debug
otherwise difficult logger problems. But I don't think we need to go
out of our way to keep it enabled in re-instantiations, seeing that we
don't really expect the logger to crash anyway (see below).

> Finally, you don't seem to be a friend of a logfile rotation user
> trigger...

Nope, I'm not. I think it's a bad solution to a nonexistent problem.
The logger's control parameters are more than sufficient. Furthermore,
we'd really prefer that the logger doesn't ever crash (the restart
business is a bit ticklish) and so the fewer features it has, the better.

regards, tom lane


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andreas Pflug <pgadmin(at)pse-consulting(dot)de>
Cc: pgsql-hackers-win32(at)postgresql(dot)org, Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
Subject: Re: Win32 testing needed
Date: 2004-08-06 15:48:20
Message-ID: 16792.1091807300@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

Andreas Pflug <pgadmin(at)pse-consulting(dot)de> writes:
> ereport(LOG,
> (errcode_for_file_access(),
> - errmsg("could not read from logger pipe: %m")));
> + errmsg("could not read from logger pipe: %lu", error)));

Does %m actually give a wrong result here? Because if it does,
errcode_for_file_access() is wrong too.

regards, tom lane


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andreas Pflug <pgadmin(at)pse-consulting(dot)de>
Cc: pgsql-hackers-win32(at)postgresql(dot)org, Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
Subject: Re: Win32 testing needed
Date: 2004-08-06 16:12:13
Message-ID: 18894.1091808733@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

Andreas Pflug <pgadmin(at)pse-consulting(dot)de> writes:
> Apparently, this is a mixture of binary and text file mode. Initially,
> stderr is in text mode. When redirecting with dup2, it will be binary;
> this must be corrected with

> dup2(_open_osfhandle(...., _O_APPEND | _O_TEXT), ...

Okay, I added _O_TEXT to that call; the #ifdef WIN32 part now looks like

fflush(stderr);
if (dup2(_open_osfhandle((long)syslogPipe[1],
_O_APPEND | _O_TEXT),
_fileno(stderr)) < 0)
ereport(FATAL,
(errcode_for_file_access(),
errmsg("could not redirect stderr: %m")));
/* Now we are done with the write end of the pipe. */
CloseHandle(syslogPipe[1]);
syslogPipe[1] = 0;

One question about this: isn't this coding leaking a file descriptor?
That is, shouldn't we catch the result of _open_osfhandle and do a
CloseHandle on it after the dup2 step?

BTW, is it correct to use 0 as "invalid handle"? Or should we be using
-1 or some such?

> Now, the pipe ReadFile will receive completely formatted data, which
> must be written binary (otherwise we will get CRCRLF), OTOH, the
> logger's calls to write_syslogger_file should write in text mode or
> replace \n by \r\n. Seems we need another function for elog to call.

Yeah. What do you think is the most convenient way to do that? I'd
be inclined to build a function that just expands \n to \r\n and then
calls write_syslogger_file, but maybe there's an easier way.

regards, tom lane


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andreas Pflug <pgadmin(at)pse-consulting(dot)de>
Cc: pgsql-hackers-win32(at)postgresql(dot)org
Subject: Re: Win32 testing needed
Date: 2004-08-06 16:48:48
Message-ID: 20312.1091810928@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

Andreas Pflug <pgadmin(at)pse-consulting(dot)de> writes:
> We could have *two* handles open to the syslogger file, but that's
> probably more fragile. Making the current write_syslogger_file static
> and providing another function that converts in a local buffer and
> calles write_syslogger_file seems best.

Agreed (although actually I was thinking of preserving
write_syslogger_file as the externally visible name and renaming the
existing function to a static "write_syslogger_file_binary" or some such
--- fewer files to touch that way). Do you have time to write and check
this today? I could write it but am not in a good position to test it.

regards, tom lane


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andreas Pflug <pgadmin(at)pse-consulting(dot)de>
Cc: pgsql-hackers-win32(at)postgresql(dot)org, Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
Subject: Re: Win32 testing needed
Date: 2004-08-06 16:59:01
Message-ID: 20388.1091811541@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

Andreas Pflug <pgadmin(at)pse-consulting(dot)de> writes:
> Maybe we should consider examining GetLastError() and FormatMessage()
> (the equivalent of strerror, a sample is in the symlink/junction code)
> for %m under win32; these will work for standard posix operations also
> and might give much more detailed messages in many situations.

Hmm. It's a bit attractive but it would break a lot of existing code
that assumes saving/restoring errno is a sufficient way to not clobber
the error result info between getting an error and reporting it. For
instance, xlog.c has several repetitions of this pattern:

errno = 0;
if ((int) write(fd, zbuffer, sizeof(zbuffer)) != (int) sizeof(zbuffer))
{
int save_errno = errno;

/*
* If we fail to make the file, delete it to release disk
* space
*/
unlink(tmppath);
/* if write didn't set errno, assume problem is no disk space */
errno = save_errno ? save_errno : ENOSPC;

ereport(PANIC,
(errcode_for_file_access(),
errmsg("could not write to file \"%s\": %m", tmppath)));
}

and I don't think I want to add Windows #ifdefs to every such place.

What would be more supportable is something that assigns a suitable
value to errno after a failure of a Windows-specific call. Thus
something like

if (!ReadFile(syslogPipe[0], logbuffer, sizeof(logbuffer),
&bytesRead, 0))
{
DWORD error = GetLastError();

if (error == ERROR_HANDLE_EOF ||
error == ERROR_BROKEN_PIPE)
break;
errno = Win32ErrorToErrno(error);
ereport(LOG,
(errcode_for_file_access(),
errmsg("could not read from logger pipe: %m")));
}

While this is admittedly ugly, it confines the ugliness to the fairly
small number of places where we call Windows-specific routines (ie,
we're already inside an #ifdef WIN32). If we do the other, the
messiness is going to spread into what had been perfectly good
Posix-standard code.

regards, tom lane


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andreas Pflug <pgadmin(at)pse-consulting(dot)de>
Cc: pgsql-hackers-win32(at)postgresql(dot)org
Subject: Re: Win32 testing needed
Date: 2004-08-06 17:25:50
Message-ID: 20688.1091813150@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

Andreas Pflug <pgadmin(at)pse-consulting(dot)de> writes:
> The attached code snippet works. I hesitated to use palloc(count*2) to
> avoid problems in low mem conditions; might be paranoid.

No, I quite agree. What we can do is adjust this to call
write_syslogger_file_binary more than once if the convbuf gets full;
then we can use a pretty small convbuf without fear.

Will fix and commit --- thanks!

regards, tom lane


From: Andreas Pflug <pgadmin(at)pse-consulting(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers-win32(at)postgresql(dot)org, Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
Subject: Re: Win32 testing needed
Date: 2004-08-06 17:28:52
Message-ID: 4113BFD4.7080603@pse-consulting.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

Tom Lane wrote:
> It might be best to just leave syslogFile open --- it should be properly
> flushed and closed by exit() anyway, no?

Agreed.

> Windows treats _IOLBF the same as _IOFBF, which we don't want.
> Okay, ifdef time.

:->

>>An observation I didn't track down so far:
>>Some LOG messages (e.g. the final logger shutdown, or "received fast
>>shutdown request") don't have proper CRLF line ending in win32, but only
>>LF.
>
>
> Weird. No ideas about that. Can you determine whether the data coming
> through the pipe has the problem?

It has, that's where I noticed. It is restricted to the postmaster and
the syslogger; all other processes ereport correctly.
Apparently, this is a mixture of binary and text file mode. Initially,
stderr is in text mode. When redirecting with dup2, it will be binary;
this must be corrected with

dup2(_open_osfhandle(...., _O_APPEND | _O_TEXT), ...

which solves the issue for postmaster. Child processes will have stderr
in text mode automatically, even if inherited and redirected into a pipe
(which is always binary).

Now, the pipe ReadFile will receive completely formatted data, which
must be written binary (otherwise we will get CRCRLF), OTOH, the
logger's calls to write_syslogger_file should write in text mode or
replace \n by \r\n. Seems we need another function for elog to call.

>
> Why doesn't it work? Do we just need a different spelling of
> "/dev/null" for Windows?

Er, /dev/null? no such beast under win32.
Just checked, closing stderr works.

>
>>Finally, you don't seem to be a friend of a logfile rotation user
>>trigger...
>
>
> Nope, I'm not. I think it's a bad solution to a nonexistent problem.
> The logger's control parameters are more than sufficient. Furthermore,
> we'd really prefer that the logger doesn't ever crash (the restart
> business is a bit ticklish) and so the fewer features it has, the better.

It's no additional feature, it's just using an existing communication
path (signal), which has to be handled anyway, to setting an existing
flag. The code path for sighup is certainly much larger.

Regards,
Andreas


From: Andreas Pflug <pgadmin(at)pse-consulting(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers-win32(at)postgresql(dot)org, Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
Subject: Re: Win32 testing needed
Date: 2004-08-06 18:26:34
Message-ID: 4113CD5A.6070200@pse-consulting.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

Tom Lane wrote:
> Andreas Pflug <pgadmin(at)pse-consulting(dot)de> writes:
>
>> ereport(LOG,
>> (errcode_for_file_access(),
>>- errmsg("could not read from logger pipe: %m")));
>>+ errmsg("could not read from logger pipe: %lu", error)));
>
>
> Does %m actually give a wrong result here? Because if it does,
> errcode_for_file_access() is wrong too.

Yes, errno is not set. Same issue for CreatePipe; I missed that.

Maybe we should consider examining GetLastError() and FormatMessage()
(the equivalent of strerror, a sample is in the symlink/junction code)
for %m under win32; these will work for standard posix operations also
and might give much more detailed messages in many situations.

Regards,
Andreas


From: Andreas Pflug <pgadmin(at)pse-consulting(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers-win32(at)postgresql(dot)org
Subject: Re: Win32 testing needed
Date: 2004-08-06 18:41:40
Message-ID: 4113D0E4.60703@pse-consulting.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

Tom Lane wrote:
> if (dup2(_open_osfhandle((long)syslogPipe[1],
> _O_APPEND | _O_TEXT),
> _fileno(stderr)) < 0)
> ereport(FATAL,
> (errcode_for_file_access(),
> errmsg("could not redirect stderr: %m")));
> /* Now we are done with the write end of the pipe. */
> CloseHandle(syslogPipe[1]);
> syslogPipe[1] = 0;
>
> One question about this: isn't this coding leaking a file descriptor?
> That is, shouldn't we catch the result of _open_osfhandle and do a
> CloseHandle on it after the dup2 step?

Yes, it does.
int fd=_open_osfhandle(....); // additional fdes from winhandle
_dup2(fd, ...)
close(fd);

>
> BTW, is it correct to use 0 as "invalid handle"? Or should we be using
> -1 or some such?

0 is usually ok, valid handles are never 0. The official value is
INVALID_HANDLE_VALUE which expands to (HANDLE)-1.

>
>
>>Now, the pipe ReadFile will receive completely formatted data, which
>>must be written binary (otherwise we will get CRCRLF), OTOH, the
>>logger's calls to write_syslogger_file should write in text mode or
>>replace \n by \r\n. Seems we need another function for elog to call.
>
>
> Yeah. What do you think is the most convenient way to do that? I'd
> be inclined to build a function that just expands \n to \r\n and then
> calls write_syslogger_file, but maybe there's an easier way.

We could have *two* handles open to the syslogger file, but that's
probably more fragile. Making the current write_syslogger_file static
and providing another function that converts in a local buffer and
calles write_syslogger_file seems best.

Regards,
Andreas


From: Andreas Pflug <pgadmin(at)pse-consulting(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers-win32(at)postgresql(dot)org
Subject: Re: Win32 testing needed
Date: 2004-08-06 19:17:04
Message-ID: 4113D930.2060406@pse-consulting.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

Tom Lane wrote:
> Andreas Pflug <pgadmin(at)pse-consulting(dot)de> writes:

>
> Agreed (although actually I was thinking of preserving
> write_syslogger_file as the externally visible name and renaming the
> existing function to a static "write_syslogger_file_binary" or some such
> --- fewer files to touch that way). Do you have time to write and check
> this today? I could write it but am not in a good position to test it.

The attached code snippet works. I hesitated to use palloc(count*2) to
avoid problems in low mem conditions; might be paranoid.

Regards,
Andreas

Attachment Content-Type Size
write_syslogger.c text/x-csrc 358 bytes

From: markir(at)coretech(dot)co(dot)nz
To: Andreas Pflug <pgadmin(at)pse-consulting(dot)de>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers-win32(at)postgresql(dot)org
Subject: Re: Win32 testing needed - 1 Tablespaces
Date: 2004-08-07 00:37:49
Message-ID: 1091839069.f42478d5384bc@mail.coretech.co.nz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

Managed to compile by adding #define FRONTEND to dirmod.c - not sure why I
didn't think to try that last night....

So, started up and tried to create a tablespace :

template1=# create tablespace big1 location 'c:/databases/pgtablespaces/big1';
ERROR: tablespaces are not supported on this platform

...not quite what I expected. I did a cvs update -d -P this morning. I am on win
2000 with latest updates.

any ideas? (I will boot back into Freebsd and check I have the latest
updates...)

regards

Mark

Quoting Andreas Pflug <pgadmin(at)pse-consulting(dot)de>:
> Mark Kirkwood wrote:
> >
> > ../../src/port/libpgport.a(dirmod.o)(.text+0x33a):dirmod.c: undefined
> > reference to `_imp__CurrentMemoryContext'
> >
>
>
> Apparently the reason is crossover referencing between libs, i.e.
> libpgport.a using libpostgres.a and vice versaa.


From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: markir(at)coretech(dot)co(dot)nz
Cc: Andreas Pflug <pgadmin(at)pse-consulting(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers-win32(at)postgresql(dot)org
Subject: Re: Win32 testing needed - 1 Tablespaces
Date: 2004-08-07 00:48:16
Message-ID: 200408070048.i770mGb28226@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32


We have a symlink patch to make it work but it isn't in CVS yet.

---------------------------------------------------------------------------

markir(at)coretech(dot)co(dot)nz wrote:
> Managed to compile by adding #define FRONTEND to dirmod.c - not sure why I
> didn't think to try that last night....
>
> So, started up and tried to create a tablespace :
>
> template1=# create tablespace big1 location 'c:/databases/pgtablespaces/big1';
> ERROR: tablespaces are not supported on this platform
>
> ...not quite what I expected. I did a cvs update -d -P this morning. I am on win
> 2000 with latest updates.
>
> any ideas? (I will boot back into Freebsd and check I have the latest
> updates...)
>
> regards
>
> Mark
>
> Quoting Andreas Pflug <pgadmin(at)pse-consulting(dot)de>:
> > Mark Kirkwood wrote:
> > >
> > > ../../src/port/libpgport.a(dirmod.o)(.text+0x33a):dirmod.c: undefined
> > > reference to `_imp__CurrentMemoryContext'
> > >
> >
> >
> > Apparently the reason is crossover referencing between libs, i.e.
> > libpgport.a using libpostgres.a and vice versaa.
>
>
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://archives.postgresql.org
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073


From: Mark Kirkwood <markir(at)coretech(dot)co(dot)nz>
To: markir(at)coretech(dot)co(dot)nz
Cc: Andreas Pflug <pgadmin(at)pse-consulting(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers-win32(at)postgresql(dot)org
Subject: Re: Win32 testing needed - 1 Tablespaces
Date: 2004-08-07 00:52:04
Message-ID: 411427B4.8010409@coretech.co.nz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

Sorry guys.... looks like I misunderstood.....I thought Tom had
committed the SYMLINK patch (and enabled tablespaces) as well as the
logging one!

markir(at)coretech(dot)co(dot)nz wrote:

>Managed to compile by adding #define FRONTEND to dirmod.c - not sure why I
>didn't think to try that last night....
>
>So, started up and tried to create a tablespace ....
>
>
>


From: markir(at)coretech(dot)co(dot)nz
To: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
Cc: Andreas Pflug <pgadmin(at)pse-consulting(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers-win32(at)postgresql(dot)org
Subject: Re: Win32 testing needed - 1 Tablespaces
Date: 2004-08-07 01:29:00
Message-ID: 1091842140.c7dabc5a0cc75@mail.coretech.co.nz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

Well, the only suitable punishment seemed to be to apply the patch and check it
out :-)

Looks good :

benchw=# create tablespace big1 location 'c:/databases/pgtablespaces/big1';
CREATE TABLESPACE
benche=# CREATE TABLE dim0 (
d0key INTEGER NOT NULL,
ddate DATE NOT NULL,
dyr INTEGER NOT NULL,
dmth INTEGER NOT NULL,
dday INTEGER NOT NULL
) TABLESPACE big1;
CREATE
benchw=# COPY dim0 FROM 'c:/databases/dim0.dat'
USING DELIMITERS ',';
COPY

benchw=# analyze verbose dim0;
INFO: analyzing "public.dim0"
INFO: "dim0": scanned 74 of 74 pages, containing 10000 live rows and 0 dead row
s; 3000 rows in sample, 10000 estimated total rows
ANALYZE

regards

Mark

Quoting Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>:
>
> We have a symlink patch to make it work but it isn't in CVS yet.


From: markir(at)coretech(dot)co(dot)nz
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers-win32(at)postgresql(dot)org
Subject: Re: Win32 testing needed - 2 Logger
Date: 2004-08-07 03:18:18
Message-ID: 1091848698.97901c3352557@mail.coretech.co.nz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

This seems to work well, for both destination types 'stderr' (with
redirect_stderr set), and 'eventlog'. The only minor gripe is that the
'eventlog' case is a bit laiden with extraneous DLL warnings.

The logger seems to shutdown cleanly, for both the case where pg_ctl is used,
and where the service manager is.

Here are some sample outputs:

i) Eventlog

Logging information gets into the event log ok: (including errors and shutdown
notification)

An error (select from non-existent table):

The description for Event ID ( 0 ) in Source ( PostgreSQL ) cannot be found.
The local computer may not have the necessary registry information or message
DLL files to display messages from a remote computer. The following information
is part of the event: ERROR: relation "pg_classes" does not exist

Final shutdown:

The description for Event ID ( 0 ) in Source ( PostgreSQL ) cannot be found.
The local computer may not have the necessary registry information or message
DLL files to display messages from a remote computer. The following information
is part of the event: LOG: database system is shut down

ii) Stderr

new files are created for seach startup, and file contents includes both error
and notification messages. Note that this case was shutdown with the service
manager (while I had an idle connection still open)

LOG: database system was shut down at 2004-08-07 15:04:08 New Zealand Standard
Time
LOG: checkpoint record is at 0/6A3FB998
LOG: redo record is at 0/6A3FB998; undo record is at 0/0; shutdown TRUE
LOG: next transaction ID: 545; next OID: 10047239
LOG: database system is ready
ERROR: relation "pg_classes" does not exist
LOG: received fast shutdown request
LOG: aborting any active transactions
FATAL: terminating connection due to administrator command
LOG: shutting down
LOG: database system is shut down
LOG: logger shutting down

regards

Mark

Quoting Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> I have just committed a somewhat trimmed-down version of Andreas Pflug's
> syslogger patch. It seems to work okay on Unix (with or without
> EXEC_BACKEND) but I'm not in a position to test it on Windows. Would
> someone give it a try and report back? Please check in particular that
> the logger shuts down cleanly after the postmaster is gone.


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: markir(at)coretech(dot)co(dot)nz
Cc: pgsql-hackers-win32(at)postgresql(dot)org
Subject: Re: Win32 testing needed - 2 Logger
Date: 2004-08-07 03:24:03
Message-ID: 25761.1091849043@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

markir(at)coretech(dot)co(dot)nz writes:
> This seems to work well, for both destination types 'stderr' (with
> redirect_stderr set), and 'eventlog'. The only minor gripe is that the
> 'eventlog' case is a bit laiden with extraneous DLL warnings.

Just for clarity --- when did you pull the snapshot you're testing?
I made several commits this morning (US east coast time, ie, some eight
or ten hours ago now) in response to Andreas' early feedback.

The most certain way to say what you tested is to report the file
version number in the $PostgreSQL$ tag in
src/backend/postmaster/syslogger.c.

regards, tom lane


From: markir(at)coretech(dot)co(dot)nz
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers-win32(at)postgresql(dot)org
Subject: Re: Win32 testing needed - 2 Logger
Date: 2004-08-07 04:03:36
Message-ID: 1091851416.9bcefd633e36b@mail.coretech.co.nz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

I should have mentioned that, sorry. About 9:30 NZST (about 6.5 hours ago)
The tag for syslogger.c is:

$PostgreSQL: pgsql-server/src/backend/postmaster/syslogger.c,v 1.4 200
4/08/06 19:17:31 tgl Exp $

Quoting Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:

> markir(at)coretech(dot)co(dot)nz writes:
> > This seems to work well, for both destination types 'stderr' (with
> > redirect_stderr set), and 'eventlog'. The only minor gripe is that the
> > 'eventlog' case is a bit laiden with extraneous DLL warnings.
>
> Just for clarity --- when did you pull the snapshot you're testing?
> I made several commits this morning (US east coast time, ie, some eight
> or ten hours ago now) in response to Andreas' early feedback.
>
> The most certain way to say what you tested is to report the file
> version number in the $PostgreSQL$ tag in
> src/backend/postmaster/syslogger.c.
>
> regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 8: explain analyze is your friend
>


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: markir(at)coretech(dot)co(dot)nz
Cc: pgsql-hackers-win32(at)postgresql(dot)org
Subject: Re: Win32 testing needed - 2 Logger
Date: 2004-08-07 04:16:26
Message-ID: 26125.1091852186@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

markir(at)coretech(dot)co(dot)nz writes:
> I should have mentioned that, sorry. About 9:30 NZST (about 6.5 hours ago)
> The tag for syslogger.c is:

> $PostgreSQL: pgsql-server/src/backend/postmaster/syslogger.c,v 1.4 200
> 4/08/06 19:17:31 tgl Exp $

Okay, that's CVS tip from my perspective too. Anyone have comments
about why the eventlog log is so noisy? It's outside my competence...

regards, tom lane


From: markir(at)coretech(dot)co(dot)nz
To: Andreas Pflug <pgadmin(at)pse-consulting(dot)de>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers-win32(at)postgresql(dot)org
Subject: Re: Eventlog
Date: 2004-08-07 09:34:32
Message-ID: 1091871272.35d16b50e28e6@mail.coretech.co.nz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

Presumably this is because I compiled from source rather than using the Pg
Installer?

Quoting Andreas Pflug <pgadmin(at)pse-consulting(dot)de>:
>
> This looks like an installation problem; I don't see that on my machine.
>
> The eventlog will receive an eventID (postgresql uses only 0), which is
> an index into a message table provided by the service (it's a binary
> resource). If that message is not present, or if the message provider
> isn't registered for the server, the text mentioned is displayed.
> For pgsql, this message is just "%s", to use it generically.
>
> To check if the dll is registered correctly:
>
> You should have
>
>
[HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\Eventlog\Application\PostgreSQL]
> "EventMessageFile"="C:\\Program Files\\ PostgreSQL\\7.5\\lib\\pgevent.dll"
>
> or another valid pgevent.dll path. Adding this might need a machine
> restart (yes, it's win...)
>
> I noticed a trailing '.' on a single line in every eventlog entry, the
> attached patch will remove this.
>
> Regards,
> Andreas
>


From: Andreas Pflug <pgadmin(at)pse-consulting(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: markir(at)coretech(dot)co(dot)nz, pgsql-hackers-win32(at)postgresql(dot)org
Subject: Re: Eventlog
Date: 2004-08-07 10:53:07
Message-ID: 4114B493.3010303@pse-consulting.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

Tom Lane wrote:

>
>
> Okay, that's CVS tip from my perspective too. Anyone have comments
> about why the eventlog log is so noisy? It's outside my competence...

This looks like an installation problem; I don't see that on my machine.

The eventlog will receive an eventID (postgresql uses only 0), which is
an index into a message table provided by the service (it's a binary
resource). If that message is not present, or if the message provider
isn't registered for the server, the text mentioned is displayed.
For pgsql, this message is just "%s", to use it generically.

To check if the dll is registered correctly:

You should have

[HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\Eventlog\Application\PostgreSQL]
"EventMessageFile"="C:\\Program Files\\ PostgreSQL\\7.5\\lib\\pgevent.dll"

or another valid pgevent.dll path. Adding this might need a machine
restart (yes, it's win...)

I noticed a trailing '.' on a single line in every eventlog entry, the
attached patch will remove this.

Regards,
Andreas

Attachment Content-Type Size
msgevent.patch text/x-patch 401 bytes

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andreas Pflug <pgadmin(at)pse-consulting(dot)de>
Cc: markir(at)coretech(dot)co(dot)nz, pgsql-hackers-win32(at)postgresql(dot)org
Subject: Re: Eventlog
Date: 2004-08-07 16:13:50
Message-ID: 192.1091895230@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

Andreas Pflug <pgadmin(at)pse-consulting(dot)de> writes:
> I noticed a trailing '.' on a single line in every eventlog entry, the
> attached patch will remove this.

This kinda bears out Peter's point about not wanting binary files in
CVS: I can install the patch as given, but I don't think it will fix
anything. May I trouble you for updated copies of the derived files
in src/bin/pgevent?

regards, tom lane


From: Andreas Pflug <pgadmin(at)pse-consulting(dot)de>
To: markir(at)coretech(dot)co(dot)nz
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers-win32(at)postgresql(dot)org
Subject: Re: Eventlog
Date: 2004-08-07 17:07:02
Message-ID: 41150C36.7050409@pse-consulting.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

markir(at)coretech(dot)co(dot)nz wrote:
> Presumably this is because I compiled from source rather than using the Pg
> Installer?

Yes, probably. I'd recommend using the installer and continuing from
source then.

Regards,
Andreas


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andreas Pflug <pgadmin(at)pse-consulting(dot)de>
Cc: markir(at)coretech(dot)co(dot)nz, pgsql-hackers-win32(at)postgresql(dot)org
Subject: Re: Eventlog
Date: 2004-08-07 17:57:02
Message-ID: 1872.1091901422@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

Andreas Pflug <pgadmin(at)pse-consulting(dot)de> writes:
> I noticed a trailing '.' on a single line in every eventlog entry, the
> attached patch will remove this.

Applied.

regards, tom lane


From: Andreas Pflug <pgadmin(at)pse-consulting(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers-win32(at)postgresql(dot)org
Subject: Re: Eventlog
Date: 2004-08-07 18:59:41
Message-ID: 4115269D.80606@pse-consulting.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

Tom Lane wrote:
> Andreas Pflug <pgadmin(at)pse-consulting(dot)de> writes:
>
>>I noticed a trailing '.' on a single line in every eventlog entry, the
>>attached patch will remove this.
>
>
> This kinda bears out Peter's point about not wanting binary files in
> CVS: I can install the patch as given, but I don't think it will fix
> anything. May I trouble you for updated copies of the derived files
> in src/bin/pgevent?

No problem.
Unfortunately, AFAIK there's no way around this binary resource to
address the eventlog service.
There are more issues about eventlog, but we can leave that for
after-beta-1 time.

Regards,
Andreas

Attachment Content-Type Size
MSG00001.bin application/octet-stream 28 bytes