I wrote:
> But I'll happily concede the point, and prove it by knocking
> up a patch for it over the weekend (unless anyone else
> particularly wants to).
Occurs to me I could kill 2 birds with one stone, and also implement another
Win32 sticking point, namely the waitpid changes for the Postma
Done and attached.
---
Tom Lane wrote:
> Bruce Momjian <[EMAIL PROTECTED]> writes:
> > Claudio specified the attached fix, which I have applied (this time).
>
> The ereport must vanish back into its black hole, also.
> Proc
Bruce Momjian <[EMAIL PROTECTED]> writes:
> Claudio specified the attached fix, which I have applied (this time).
The ereport must vanish back into its black hole, also.
ProcessStartupPacket has already issued any appropriate log message.
> *** 2450,2456
> {
> ereport(LOG
Tom Lane wrote:
> Jan Wieck <[EMAIL PROTECTED]> writes:
> > Something after 2003/11/20 enhanced the query cancel handling. Namely,
> > CVS tip now responds to a query cancel with a postmaster restart
> > canceling all queries. Could the fork/exec stuff be responsible for this?
>
> Whoever change
Tom Lane wrote:
> Jan Wieck <[EMAIL PROTECTED]> writes:
> > Something after 2003/11/20 enhanced the query cancel handling. Namely,
> > CVS tip now responds to a query cancel with a postmaster restart
> > canceling all queries. Could the fork/exec stuff be responsible for this?
>
> Whoever change
Claudio Natoli <[EMAIL PROTECTED]> writes:
> Actually, if I was going to argue anything, I'd say that if a backend goes
> nuts and zeroes the whole shmem segment you've probably some bigger things
> to worry about (Aside: Would postgres actually recover from such an
> occurence? BTW, I'd be pretty
Tom Lane wrote:
> What it comes down to is I don't want the postmaster to be keeping its
> own state in shared memory --- that is, the array must be write-only
> memory as far as the postmaster is concerned. If we eliminate the
Ok. Well, I can appreciate things from that point of view.
> post
Claudio Natoli <[EMAIL PROTECTED]> writes:
> I wasn't suggesting that we somehow put the Dllist in shared memory. I was
> advocating completely replacing the BackendList dlllist with this array,
> under both Win32 and *nix case.
Right, that's what I understood you to be proposing, and I dislike it
Claudio Natoli <[EMAIL PROTECTED]> writes:
> 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 enou
Claudio Natoli <[EMAIL PROTECTED]> writes:
> 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.
> Also, let's g
Bruce Momjian:
> > 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 r
Claudio Natoli wrote:
>
> 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 referr
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
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 s
Bruce Momjian <[EMAIL PROTECTED]> writes:
> Uh, why would you need more than maxbackends? Can't it be indexed by
> slot number --- each backend has a slot? Maybe I am missing something.
The postmaster has no way to know what slot number each backend will
get. For that matter, a sub-postmaster d
Claudio Natoli <[EMAIL PROTECTED]> writes:
> Just to be clear, this would involve turning the BackendList dlllist into an
> array in shared memory, right? If so, a couple of questions:
Per Jan's comment, there is no need to mess with the existing
datastructure. I'd think more of *copying* the dll
Claudio Natoli wrote:
>
> I wrote:
> > Just to be clear, this would involve turning the BackendList dlllist into
> an
> > array in shared memory, right? If so, a couple of questions:
>
> Bruce Momjian wrote:
> > I assumed a much simpler solution. I thought we would just have:
> >
> > s
I wrote:
> Just to be clear, this would involve turning the BackendList dlllist into
an
> array in shared memory, right? If so, a couple of questions:
Bruce Momjian wrote:
> I assumed a much simpler solution. I thought we would just have:
>
> struct {
> pid_t pid;
>
Claudio Natoli wrote:
>
> > 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 k
Bruce Momjian wrote:
Claudio Natoli wrote:
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 backend
> 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
Claudio Natoli wrote:
>
> 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 t
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
reque
Tom Lane wrote:
Jan Wieck <[EMAIL PROTECTED]> writes:
It doesn't hurt to keep the locations and code as much in sync as
possible. I think Tom's idea to move the information into the PGPROC
entry is the winner and does not need any OS specific handling.
Actually, on further reflection a separate
Jan Wieck <[EMAIL PROTECTED]> writes:
> It doesn't hurt to keep the locations and code as much in sync as
> possible. I think Tom's idea to move the information into the PGPROC
> entry is the winner and does not need any OS specific handling.
Actually, on further reflection a separate array to s
Bruce Momjian wrote:
Tom Lane wrote:
> Claudio Natoli wrote:
>> The only things I've thought of so far are:
>> a) sticking the PID/cancel key list in shared mem [yeech]
>> b) rearranging the entire cancel handling to occur in the postmaster [double
>> yeech]
(a) seems like the lesser of the availab
Tom Lane wrote:
> > Claudio Natoli wrote:
> >> The only things I've thought of so far are:
> >> a) sticking the PID/cancel key list in shared mem [yeech]
> >> b) rearranging the entire cancel handling to occur in the postmaster [double
> >> yeech]
>
> (a) seems like the lesser of the available evi
> Claudio Natoli wrote:
>> The only things I've thought of so far are:
>> a) sticking the PID/cancel key list in shared mem [yeech]
>> b) rearranging the entire cancel handling to occur in the postmaster [double
>> yeech]
(a) seems like the lesser of the available evils (unless someone has a
brigh
Claudio Natoli wrote:
>
> > BTW, how are we going to do cancels in Windows-land? The sub-postmaster
> > isn't gonna have access to the postmaster's list of child PIDs and
> > cancel keys ...
>
> Good question (the Win32/EXEC_BACKEND case is #def'd out to issue an
> altogether unhelpful abort(),
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 fixe
> BTW, how are we going to do cancels in Windows-land? The sub-postmaster
> isn't gonna have access to the postmaster's list of child PIDs and
> cancel keys ...
Good question (the Win32/EXEC_BACKEND case is #def'd out to issue an
altogether unhelpful abort(), so I know it is there).
The only t
Bruce Momjian <[EMAIL PROTECTED]> writes:
> When you say sub-postmaster, you mean we fork, then process the cancel
> request? Seems we might need special handling in there, yea.
We fork before we even read the request. Otherwise an attacker can
trivially lock up the postmaster by sending a parti
> 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:
> Bruce Momjian <[EMAIL PROTECTED]> writes:
> > My guess is that this used to proc_exit the postgres backend, but now
> > proc_exits the postmaster, but that is only a guess.
>
> No. This is post-fork (and had better remain so). The change causes
> the sub-postmaster that has jus
Tom Lane wrote:
> Bruce Momjian <[EMAIL PROTECTED]> writes:
> > My guess is that this used to proc_exit the postgres backend, but now
> > proc_exits the postmaster, but that is only a guess.
>
> No. This is post-fork (and had better remain so). The change causes
> the sub-postmaster that has jus
Bruce Momjian <[EMAIL PROTECTED]> writes:
> My guess is that this used to proc_exit the postgres backend, but now
> proc_exits the postmaster, but that is only a guess.
No. This is post-fork (and had better remain so). The change causes
the sub-postmaster that has just finished handling a cancel
Tom Lane wrote:
> Jan Wieck <[EMAIL PROTECTED]> writes:
> > Something after 2003/11/20 enhanced the query cancel handling. Namely,
> > CVS tip now responds to a query cancel with a postmaster restart
> > canceling all queries. Could the fork/exec stuff be responsible for this?
>
> Whoever change
Jan Wieck <[EMAIL PROTECTED]> writes:
> Something after 2003/11/20 enhanced the query cancel handling. Namely,
> CVS tip now responds to a query cancel with a postmaster restart
> canceling all queries. Could the fork/exec stuff be responsible for this?
Whoever changed this:
status = Proces
Claudio Natoli wrote:
Something after 2003/11/20 enhanced the query cancel handling. Namely,
CVS tip now responds to a query cancel with a postmaster restart
canceling all queries. Could the fork/exec stuff be responsible for this?
Jan
Hi Bruce,
Claudio, where are we on this patch? I see an
Claudio Natoli wrote:
>
> Hi Bruce,
>
> > Claudio, where are we on this patch? I see an even newer version in the
> archives:
> >
> >
> > http://archives.postgresql.org/pgsql-patches/2003-12/msg00361.php
>
> [Weird you and Google groups "missed" it!]
No, seems only _I_ missed it. The go
Hi Bruce,
> Claudio, where are we on this patch? I see an even newer version in the
archives:
>
>
> http://archives.postgresql.org/pgsql-patches/2003-12/msg00361.php
[Weird you and Google groups "missed" it!]
> Anyway, Tom has looked at your newest patch and it looks good to him.
I'
Claudio, where are we on this patch? I see an even newer version in the
archives:
http://archives.postgresql.org/pgsql-patches/2003-12/msg00361.php
However, I have never seen that version, and Google doesn't show it
either:
http://groups.google.com/groups?hl=en&lr=&ie=UTF-8&c2
Your patch has been added to the PostgreSQL unapplied patches list at:
http://momjian.postgresql.org/cgi-bin/pgpatches
I will try to apply it within the next 48 hours.
---
Claudio Natoli wrote:
>
> [patch edited
#x27;
> Subject: Re: [PATCHES] fork/exec patch: pre-CreateProcess
> finalization
>
>
>
> [patch edited + resubmitted for review; not for committing]
>
> Hi Tom,
>
> figuring that, on balance, you are in fact going to prefer to take a
> potential future hit in
> > What I was suggesting with b) was to format up the command line for the
> > items prefixed by * in the postmaster,
> > do the fork (or fork/exec), and then run the authentication in, say
> > PostgresMain. (Note: this is essentially what the fork/exec case
currently
> > does).
>
> Yeah, I n
Claudio Natoli <[EMAIL PROTECTED]> writes:
>> I don't follow your thinking here. The information that has to be
>> reloaded *must* be passed across the fork/exec boundary in
>> either case.
> Yes. But not all of it.
I'll throw that same argument back at you: some of the information
needed for P
[patch edited + resubmitted for review; not for committing]
Hi Tom,
figuring that, on balance, you are in fact going to prefer to take a
potential future hit in duplicated work (in passing context in the fork/exec
case) over moving the client auth code to PostgresMain, I've just gone ahead
and m
> > Here's the critical difference in our thinking:
>
> > ISTM that we'll have to go to a lot of work to get the fork/exec
> > case to re-load the context required to format up the argument list to
> > PostgresMain. Certainly, it is a lot more work than allowing the
postmaster
> > itself (n
Claudio Natoli <[EMAIL PROTECTED]> writes:
> Ok. So, in conclusion we are in agreement on at least one thing: the current
> patch sucks.
Good, I'm glad we see eye to eye on that ;-)
> Here's the critical difference in our thinking:
> ISTM that we'll have to go to a lot of work to get the
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 controv
I said:
> We should either find a way to make the fork/exec path more like the
> existing code, or bite the bullet and change them both.
It occurs to me that you probably need some concrete suggestions about
what to do. Here's one.
There are really three separate sections in this code: the postm
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 of the fork/exec
> patches, so critique away :-)
Y
For application to HEAD, pending community review.
Final rearrangement of main postgresql child process (ie.
BackendFork/SSDataBase/pgstat) startup, to allow fork/exec calls to closely
mimic (the soon to be provided) Win32 CreateProcess equivalent calls.
Whereas the existing code forks and allow
53 matches
Mail list logo
