For src/backend/postmaster/postmaster.c and src/include/c.h
(Note: should ioctlsocket_ret be initialized to 1 for BEOS too, and can it
take an unsigned parameter? If so, could simplify.)
---
WE HAVE MOVED - PLEASE NOTE OUR NEW CONTACT DETAILS:
THE BASEMENT, 33 EWELL STREET, BALMAIN NSW 2041
Of course, this only applied to head, no 7.4.
Yes, that's correct. (strictly, at the time of creation, I was working off
the WIN32_DEV branch...)
---
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these disclaimers and policies see
a
Nope. My bad. My head has an off by one error.
Drop the first part of the patch, but the second part could be retained.
Very minor.
Off to hide in shame,
Claudio
-Original Message-
From: Claudio Natoli [mailto:[EMAIL PROTECTED]
Sent: Wednesday, 26 November 2003 12:34 PM
To: [EMAIL
Hi all,
here's a patch aimed at moving along the fork/exec development. It is not
yet complete or ready for submission, but would like comments at this stage
from the postgres developer community.
The places of particular interest are denoted by FIXME: [fork/exec],
however, some things I'd like
Hi all,
Dennis Bjorklund wrote:
Also has to work on Unix too for testing.
Everything can not work in unix, CreateProcess() and fork()
are different.
True (but CreateProcess and fork followed by exec are pretty close). I
think what Bruce is implying is that, ideally, we'd like to keep
[Thanks to Tom + Bruce]
For the remaining comments...
The number of FIXME or This is ugly comments doesn't ease my mind
about this patch :-) How many of these issues do you plan to resolve?
All of them, as we go along. Treat this as a first step.
- break;
Hi Neil,
No. This isn't necessary (and what action would it take in any
case?).
It should write a log message. I'm not sure why this /shouldn't/ be
done: if an operation fails, we should log that failure. This is
standard practise.
Fair enough. Will do (although, I'd point out that
For application to HEAD, pending community review.
Continued rearrangement to permit pgstat + BootstrapMain processes to be
fork/exec'd, in the same mode as the previous patch for backends.
Cheers,
Claudio
---
Certain disclaimers and policies apply to all email sent from Memetrics.
For the
To go in src/port/ directory.
This is to allow the handles returned by pipe to be used in select() calls
(which Win32 doesn't allow with the native pipe() call), thereby obviating
the need for reworking of the select() mechanisms in pgstat.c.
Additionally, something of the sort would be
Thanks for your comments Tom.
Tom Lane writes:
Claudio Natoli [EMAIL PROTECTED] writes:
This has required some reworking of the existing code base, particularly
to
BackendFork (which now actually does the fork()). As such, I've been
anticipating that this will be the most controversial
[minor corrections (some duplication in arg passing); still under
discussion/review]
Cheers,
Claudio
-Original Message-
From: Claudio Natoli [mailto:[EMAIL PROTECTED]
Sent: Saturday, 27 December 2003 9:27 PM
To: 'Tom Lane'
Cc: '[EMAIL PROTECTED]'
Subject: Re: [PATCHES] fork/exec
patch + new file attached. Haven't tested on Windows, but
should be fine.
Would be, except for a small typo (conditional operator has an additional :
instead of a ?). Also, we might want to change to SleepEx, depending on how
we eventually get signals implemented, but that's a story for
For application to HEAD, pending community review.
Drops in the CreateProcess calls for Win32 (essentially wrapping up the
fork/exec portion of the port), and fixes a handful of whitespace issues
(that no doubt I'm to blame for; my apologies) in the source base.
Cheers,
Claudio
---
Certain
Also, do we want this one #ifdef:ed in every place, or just
#define it to nothing at all on non-windows platforms?
I imagine the latter will be cleaner.
Anyway. Comments on these points in particular, and in the whole thing in
general? Workable path?
I think it looks great. Well done!
Whoever changed this:
My fault. Thanks for the catch Jan.
proc_exit(status);
Should be proc_exit(0).
My apologies,
Claudio
PS. I take it there is no regression test for this. Would this be a
difficult one to knock up?
---
Certain disclaimers and policies apply to all email sent
Tom Lane wrote:
I've just noticed another serious bit of breakage in CVS tip (though
this might be fixed by Claudio's pending patch that reverts a lot of
the code rearrangements).
It's already been applied. Your push in the SubPostmaster direction was
really useful; a lot cleaner, and fixed
Tom Lane writes:
Actually, on further reflection a separate array to store PIDs and
cancel keys is probably a better idea.
[snip]
I still think it's unnecessary to make a separate shmem segment for it,
though.
Why is that? Don't we need the backends to have access to it to do a cancel
I think they are saying put the cancel key inside the existing shared
memory segment.
Ok. Thanks.
I don't know when we actually attach to the main shared
memory sigment in the child, but it would have to be sooner than when we
need the cancel key.
Yes. Currently, it happens too late.
Tom Lane:
Per Jan's comment, there is no need to mess with the existing
datastructure. I'd think more of *copying* the dllist into some array
in shared memory. This is code that would only need to exist in the
Windows port.
(I thought Jan was referring to the PGPROC struct)
This just
Tom Lane wrote:
I think the simplest way to make this work is to use an array that's
2*MaxBackend items long (corresponding to the max number of children the
postmaster will fork).
Actually, now that I think about it, is even that big enough. There is a
reason BackendList is a list. In
This time with a patch actually attached [doh!]
-Original Message-
From: Claudio Natoli
Sent: Monday, 12 January 2004 6:45 PM
To: '[EMAIL PROTECTED]'
Subject: win32 patch: processCancelRequest/reaper (+ various minor
changes)
For application to HEAD, pending community
Tom Lane wrote:
If you want to allow the 3-parameter form to specify a negative step
size, that's fine. But don't use a heuristic to guess the intended
step direction.
Have to agree. Ever used MatLab? They have a fairly intuitive approach:
1:5 = [1 2 3 4]
1:2:5 = [1 3 5]
5:1 = empty
not
currently appear, afaics, to be included during a cygwin build. This patch
does not address this deficiency.
Cheers,
Claudio
-Original Message-
From: Claudio Natoli [mailto:[EMAIL PROTECTED]
Sent: Thursday, 29 January 2004 8:11 PM
To: [EMAIL PROTECTED]
Cc: '[EMAIL PROTECTED]'
Subject
I'd not recommend spending a lot of time on patching the existing code.
Erroring out rather than crashing is probably sufficient.
Agreed.
This fixes the ones bugging me now. I'll provide others as I trip over them.
[Note: this'll mean some changes to the tests for Win32]
Cheers,
Claudio
I have fixed dirmod.c in CVS so it uses the same logic as
crypt. Please review and tell me how you like it.
Looks good. Thanks.
Claudio
---
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these disclaimers and policies see
a
ATM, for review, not for applying.
Comments:
* Nice to see the fix for pqselect. Still want to object to the memcpy on
the record :-) (though, as we've discussed, they'll work as things are now)
* memcpy of an array of HANDLES will work seems to be taking advantage of
knowledge of the
If there are no further comments from Claudio or anyone else, I feel
this is now ready to be applied.
Code walkthrough looks good.
One final minor point to discuss:
Is being unable to set a handler for things like CTRL_C_EVENT,
CTRL_SHUTDOWN_EVENT etc really a non-fatal error? ISTM
IIRC a separate babysitter thread just to handle message passing is
exactly what Katie Ward did for UltraSQL ... the Win32 port done at
NuSphere. Glad to see she was right about that.
Or Katie and I are both wrong :-P
Cheers,
Claudio
---
Certain disclaimers and policies apply to all
For community review and application to HEAD; minor patch.
Under Win32, stat() returns an st_ino field, but it has no meaning (on
Win2K, and possibly all Win32 variants, it is always 0). This causes a
number of problems in the dfmgr.c logic, which basically all revolve around
the fact that *any*
For application to HEAD, following community review.
* Changes incorrect CYGWIN defines to __CYGWIN__
* Some localtime returns NULL checks (when unchecked cause SEGVs under Win32
regression tests)
* Rationalized CreateSharedMemoryAndSemaphores and
AttachSharedMemoryAndSemaphores (Bruce, I
For application to HEAD, following community review.
Corrects the int8/float4/float8 tests under win32.
Cheers,
Claudio
P.S. Starting to back up on win32 related changes. If someone could look at
the patch/es I've got waiting for application (and either apply 'em or let
me know what fixes are
For application to HEAD, following community review.
* Mostly, casting etc to remove compilation warnings in win32 only code.
* main.c: set _IONBF to stdout/stderr under win32 (under win32, _IOLBF
defaults to full buffering)
* pg_resetxlog/Makefile: ensures dirmod.o gets cleaned (got bitten by
These buffering fixes - are they enough to get around the buffering
issues on the mingw console? I assume no?
Nope. :-( That's one for the mingw/msys people.
Cheers,
Claudio
---
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these disclaimers
For application to CVS HEAD, following community review. (very minor)
Adds the -W flag to the pwd call under Win32. This allows directories, which
are munged by sed, such as:
/e/cygwin/opt/diff9c/pgsql/src/test/regress/data/agg.data
to be correctly passed as:
For application to HEAD.
The win32 port backend will require the functionality provided by
canonicalize_path. Patch moves it from initdb.c to port/path.c.
---
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these disclaimers and policies see
a
For application to HEAD, following community review.
Corrects a typo, introduces missing variables, and rearranges the
initialization of stats process under EXEC_BACKEND.
[A cleaner, rationalized approach to stat/backend/SSDataBase child processes
under EXEC_BACKEND is on my TODO list. However
For application to HEAD.
[float8-small-is-zero and float8-exp-three-digits will likely need similar
changes]
---
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these disclaimers and policies see
a
For application to HEAD, following community review.
Check for EOF on pipe differs under win32, as it is based on a socket
implementation.
---
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these disclaimers and policies see
a
Claudio, how does this handle renames if the file is open by someone
else? Does this remove the need to loop over the rename?
To be honest, I don't know that it does. [Will report back later.]
Two points though:
a) This could doesn't alleviate the needs for dirmod.c, as far as I'm aware.
a) This could doesn't alleviate the needs for dirmod.c, as
This CODE doesn't alleviate the NEED for...
Sheesh,
Claudio
---
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these disclaimers and policies see
a
Agreed, we still need dirmod.c in case someone has opened it using a
non-unix mode.
Ok. Wanted to make sure I was on the same page.
My only question was whether this new mode makes rename
possible on a target file opened by another backend.
Looks good. Wrote a pair of 2 liner driver
main file. WIth Claudio's fix, this isn't necessary.
Bruce, are you sure this is true? The fix is only for files than are
open()'d, not fopen()'d (as per AllocateFile).
Cheers,
Claudio
---
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these
If your fix works, I will change the Win32-specific code to use open()
and fdopen(). The only advantage to the old code was that it would
release locks, then go into a rename loop, which is horid, and we need
to avoid the loop.
Ah, I see. Should've been obvious you had that in mind
Here's a patch implementing the thread method to workaround the bug
with socket calls in signal handlers. See details in mail to
pgsql-hackers-win32 a couple of minutes ago.
Looks ok, but wouldn't it be better placed in pgstat.c?
My $0.02,
Claudio
---
Certain disclaimers and policies
Revised patch, for application to HEAD [open.c destined for src/port]
---
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these disclaimers and policies see
a
href=http://www.memetrics.com/emailpolicy.html;http://www.memetrics.com/em
What I am wondering about now, is where else we need to change? AFAICS,
there is (at least?) one signal handler that performs sockets
ops, namely Async_NotifyHandler.
Actually, I don't think we need to do anything about that one. This
signal handler is used in the backend (not
For application to HEAD, following community review.
Supplement to previous win32 patch for dfmgr. Win32 replacement for
configure time constants like PKGLIBDIR.
---
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these disclaimers and policies
For application to HEAD.
This should take care of most, if not all, cases where a backend can be
interrupted in a blocking socket operation by a signal which itself performs
a socket operation (which interacts badly with our APC-based signal
implementation).
---
Certain disclaimers and
Tom Lane wrote:
If the problem is that we need PKGLIBDIR not to be frozen at compile
time, then let's fix the problem for everybody, not add a pile of
undocumented #ifdef WIN32 hacks. (And it is a problem for everybody;
Happy to go with whatever you can suggest. However, will point out that
Ugh. Is there a way we can insert a wrapper layer without modifying the
call sites? I'm thinking of some kind of macro hack, say
[snip]
Sure. Think we've even done this before (also, prevents developers needing
to remember to use pg_*).
The reason I think it was avoided for select(), in
I'm not expecting to see zero ifdefs --- certainly not in the port
modules ;-). But Bruce's search, further up in the thread, showed that
#ifdef WIN32's are sneaking into a lot of modules that probably
shouldn't have any platform dependencies.
For the most part, I disagree (in fact, I was
Hi all,
Magnus Hagander wrote:
The third option is to redefine all these functions into our own, and
implement our own emulation layer. This means our own select(), send(),
recv() (more? I don't think so). And have these call the native winsock
APIs (WSAEventSelect(), WSASend(), WSARecv()
Your patch has been added to the PostgreSQL unapplied patches list at:
Bruce, please cancel the original patch.
The attached patch is for (possible) application to HEAD, following
meritocratic contest [socket.c for src/port]
Been iterating with Magnus extensively off-list, and he is well on
For application to HEAD, following community review.
Corrects issues recently posted by Dann Corbit, allowing libpq/psql to be
built under VC++. Moves a pgstat win32 #def to port.h
---
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these
For application to HEAD, following community review.
* Most changes are to fix warnings issued when compiling win32
* removed a few redundant defines
* get_user_name safe under win32
* rationalized pipe read EINTR for win32
* changed all backend instances of sleep() to pg_usleep
- except for
* rationalized pipe read EINTR for win32
Should read:
* rationalized pipe read EOF for win32
Corrected patch attached.
---
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these disclaimers and policies see
a
Bruce Momjian writes:
I am confused why you are linking the timezone library into
pg_dump and pg_controldata. Is that required?
At the time, cvs HEAD required it; perhaps I caught you mid-commit. I
thought it was a fast moving target to try to hit. :-)
All looks good now.
Cheers,
Claudio
For application to HEAD.
As Andrew Dunstan appears to have cracked the regression test buffering nut,
this patch makes sure that:
* correct platform specific regression files are used under ming
* pwd prints out a usable win32 pathname
Cheers,
Claudio
---
Certain disclaimers and policies
Great to see you caught that. That's one more off my list of things to
dig into.
Are there any not listed here:
http://momjian.postgresql.org/main/writings/pgsql/win32.html
If so, they probably should be put up.
Cheers,
Claudio
---
Certain disclaimers and policies apply to all email sent
[For review only. Not for committing.]
Decided to take a crack at this in the interests of knocking over the
remaining TODO win32 items asap.
Would appreciate any comments on the code, particularly from win32 people.
An obvious missing feature is allowing the service to be registered under a
For application to HEAD, following community review.
Determines the PKGLIBDIR equivalent under win32. Requires pgsql lib and bin
directories to be packaged under the same root directory (eg. some
path/pgsql/bin and some path/pgsql/lib) for the win32 port, which does
not appear to be an onerous
Here's a rough first cut, close to application state but not a finished
work.
Might be pushed for time over the next week, so if there are major
criticisms it might need someone else to pick this up in the short term.
I'll happily redo any minor bits.
Comments?
Cheers,
Claudio
---
Certain
Dave Page writes:
Looks OK to me at a quick glance. One thought though, whilst I like the
idea of using service pause to do a SIGHUP from a convenience point of
view, it's not exactly the expected behaviour and might leave the
inexperienced user wondering why the server is still running
Tom Lane writes:
Claudio Natoli [EMAIL PROTECTED] writes:
The win32 port doesn't have a native user space spinlock
implementation yet.
It does when compiled with MingW.
Are we intending to support any other compilers/build environments
than that one?
Not currently; at least
pg_ctl --help
---skiped---
-P user name of account to register PostgreSQL server
-U password of account to register PostgreSQL server
---skiped---
I think that isn't right ;)
Good thinking... :-)
Patch attached (but probably easier for any committer to make the change
Tom Lane writes:
I can't speak to the details of the Windows process-launch code,
FWIW, it checks out fine to me.
Cheers,
Claudio
---
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these disclaimers and policies see
a
Could someone please eyeball the attached patch for errors and as a
correction for:
http://archives.postgresql.org/pgsql-bugs/2004-08/msg00307.php
Also, could someone (Maksim?) please reproduce the bug on their system, and
then confirm that the patch corrects (as I seem unable to reproduce the
Just outta curiosity, why wasn't that detected by the
regression tests?
There is a test that depends on statement_timeout working ...
For the record, this was reported here a couple days after the plpgsql test
was changed:
http://archives.postgresql.org/pgsql-hackers/2004-08/msg00482.php
Magnus Hagander writes:
Here's a more complete patch that addresses all the places where
WaitForxxx can block. The previous fix only fixed
statement_timeout, not the deadlock situation.
I can confirm that the patch corrects the stated issues.
Cheers,
Claudio
---
Certain disclaimers and
Win32 compile fixes for pgbench, pgcrypto, and tsearch.
Note: pgbench is the only one I actually use, and hence have tested
Cheers,
Claudio
---
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these disclaimers and policies see
a
Win32 compile fix for misc_utils.
Note: As before, un-tested.
---
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these disclaimers and policies see
a
href=http://www.memetrics.com/emailpolicy.html;http://www.memetrics.com/em
ailpolicy.html/a
Hello guys,
it's been a while, but...
What's bugging me is that 0 and O_EXCL give the same answer, and
O_TRUNC and O_TRUNC | O_EXCL give the same answer,
This is ok, as (iirc) O_EXCL only has effect in the presence of O_CREAT.
(a comment to this effect would help here, as well as perhaps
Magnus Hagander writes:
Now, I still twist my head around the lines:
if ((fd = _open_osfhandle((long) h, fileFlags O_APPEND)) 0
||
(fileFlags (O_TEXT | O_BINARY) (_setmode(fd,
fileFlags (O_TEXT | O_BINARY)) 0)))
With the _setmode() call deep in the if
73 matches
Mail list logo
