You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by David Whitmarsh <da...@dial.pipex.com> on 1999/11/24 04:23:47 UTC

[PATCH] Re: Time to roll the 1.3.10 tarball

On Mon, 22 Nov 1999 11:02:39 -0500 (EST), you wrote:


>
>I doubt very much that this issue will be solved for Apache 1.3 either.
>Windows 95 is not a platform that was ever designed for use as a server of
>any kind.  There will always be somethings that won't work on 95/98.
>Unbuffered CGI's for instance won't work.  There have been some reports
>about why mod_proxy won't load on 95/98, but it isn't really worth it to
>fix this problem for 1.3.  There has been a lot of talk recently of
>re-writing mod_proxy for Apache 2.0.  Mos_proxy is a VERY complicated
>module, and it makes more sense to leave 1.3's alone and get 1.3.10 out
>the door, than to try to fix a major problem in the last release of a
>series.
>
>Of course, this is all MHO, and if somebody can submit a patch for either
>of these issues, then I would love to test and commit it.  :-)
>

This patch fixes the proxymodule load problem under 95, and also the
garbage collection problem.

I have a binary for anyone who needs it at
http://dspace.dial.pipex.com/david.whitmarsh/



************************************

David Whitmarsh

Sparkle Computer Co Ltd

Sybase C C++ perl UNIX NT

************************************

Re: [PATCH] Re: Time to roll the 1.3.10 tarball

Posted by Bill Stoddard <st...@raleigh.ibm.com>.
David, please generate seperate patches (unified diffs) for each fix/new
function (against the current development tree, of course) and give us a
brief overview of the new config directives. Also, you need to check out the
patches commited by Martin on Oct. 11. I think your 'mingcage' variable will
have the same problem.

http://www.apache.org/websrc/cvsweb.cgi/apache-1.3/src/modules/proxy/mod_pro
xy.c

Regards,
Bill



Re: [PATCH] Re: Time to roll the 1.3.10 tarball

Posted by David Whitmarsh <da...@dial.pipex.com>.
On Wed, 15 Dec 1999 23:44:58 +0100, you wrote:

>-1 on this. You cannot make changes all over the place which break
>on unix, only to make it run on windows. Yes you can, but then
>you must #ifdef WIN32 it.
>
>Like for instance...
>    previously, a cache file was opened (tests existance), then
>    its modification was derived using fstat (cheap operation because
>    the inode is in the buffer cache) and then the first line was
>    read.
>
>    now, you stat() the file (tests existance and gets mod times),
>    THEN you fopen() it (but it may no longer be the same file because
>    another process might have unlinked() and replaced it) and
>    then you read the first line. This is more than about twice as
>    expensive as the previous solution, therefore I veto it (because
>    the cache has millions of files, so I DO care).
>

Fair enough. Easily fixed.
>
>And please submit the individual patches (yes, that was good) in the
>correct MIME type (text/plain comes to mind, not application/octet-stream)
>to make it easier to review them. Thank you.

A pleasure.

>
>I'm not sure how many of the patches are already in a #ifdef WIN32
>block. If they aren't, then things like TLS will be difficult (if
>it compiles at all, then it will be less performant).

tls is very win32 specific and is ifdef'd.

gc_fix is now ifdef'd. gc_thread is ifdef'd where necessary. I don't
think there is anything else in there that would affect functionality
under unix

child_args.patch affects a chunk of code that is already WIN32
specific.

************************************

David Whitmarsh

Sparkle Computer Co Ltd

Sybase C C++ perl UNIX NT

************************************

Re: [PATCH] Re: Time to roll the 1.3.10 tarball

Posted by Martin Kraemer <Ma...@Mch.SNI.De>.
-1 on this. You cannot make changes all over the place which break
on unix, only to make it run on windows. Yes you can, but then
you must #ifdef WIN32 it.

Like for instance...
    previously, a cache file was opened (tests existance), then
    its modification was derived using fstat (cheap operation because
    the inode is in the buffer cache) and then the first line was
    read.

    now, you stat() the file (tests existance and gets mod times),
    THEN you fopen() it (but it may no longer be the same file because
    another process might have unlinked() and replaced it) and
    then you read the first line. This is more than about twice as
    expensive as the previous solution, therefore I veto it (because
    the cache has millions of files, so I DO care).

> Here are the broken out patches, in unified diff format

And please submit the individual patches (yes, that was good) in the
correct MIME type (text/plain comes to mind, not application/octet-stream)
to make it easier to review them. Thank you.


> tls.patch allows the proxy module to be loaded dynamically under
> windows 95.

I'm not sure how many of the patches are already in a #ifdef WIN32
block. If they aren't, then things like TLS will be difficult (if
it compiles at all, then it will be less performant).

    Martin
-- 
  <Ma...@MchP.Siemens.De>      |       Fujitsu Siemens
       <ma...@apache.org>              |   81730  Munich,  Germany

Re: [PATCH] Re: Time to roll the 1.3.10 tarball

Posted by David Whitmarsh <da...@dial.pipex.com>.
On Wed, 8 Dec 1999 10:20:11 -0500, you wrote:

>David,
>I am reviewing your patch now. Don't know much about mod_proxy so I cannot
>promise I can get this integrated. I'd like to geet some feedback from some
>of the other members first.
>
>A couple of suggestions:
>1. In general it is best not to submit patches that combine unrelated fixes.
>Submit seperate patches for each. Folks are more likely to review small
>easily understandable patches than large ones.
>
>2. Submit patches as unified diffs rather than context diffs (i.e., use
>diff -u). The developer's pages suggests using diff -C3, but that is only
>because unified diffs are not supported on all platforms.  Cygnus diff
>supports unified diffs.
>
>Bill

Here are the broken out patches, in unified diff format

child_args.patch fixes a bug whereby options with spaces aren't passed
correctly to child processes

gc_fix.patch allows basic functioning of garbage collection under
windows but does not deal with the problem of garbage collection
occurring in the thread that is servicing a response

gc_thread.patch incorporates the changes in gc_fix.patch but also
spawns a separate thread for garbage collection

tls.patch allows the proxy module to be loaded dynamically under
windows 95.

All patches are relative to the 1.3.9 release.


David

************************************

David Whitmarsh

Sparkle Computer Co Ltd

Sybase C C++ perl UNIX NT

************************************

Re: [PATCH] Re: Time to roll the 1.3.10 tarball

Posted by David Whitmarsh <da...@dial.pipex.com>.
On Wed, 8 Dec 1999 10:20:11 -0500, you wrote:


>A couple of suggestions:
>1. In general it is best not to submit patches that combine unrelated fixes.
>Submit seperate patches for each. Folks are more likely to review small
>easily understandable patches than large ones.
>

Apologies, but the patch I submitted contains all sorts of gubbins
relating to accessing the cache in an off-line mode. I'll break out
only the relevant parts into separate patches and repost.


David

************************************

David Whitmarsh

Sparkle Computer Co Ltd

Sybase C C++ perl UNIX NT

************************************

Re: [PATCH] Re: Time to roll the 1.3.10 tarball

Posted by Bill Stoddard <st...@raleigh.ibm.com>.
David,
I am reviewing your patch now. Don't know much about mod_proxy so I cannot
promise I can get this integrated. I'd like to geet some feedback from some
of the other members first.

A couple of suggestions:
1. In general it is best not to submit patches that combine unrelated fixes.
Submit seperate patches for each. Folks are more likely to review small
easily understandable patches than large ones.

2. Submit patches as unified diffs rather than context diffs (i.e., use
diff -u). The developer's pages suggests using diff -C3, but that is only
because unified diffs are not supported on all platforms.  Cygnus diff
supports unified diffs.

Bill


Re: [PATCH] Re: Time to roll the 1.3.10 tarball

Posted by Bill Stoddard <st...@raleigh.ibm.com>.
Cool! Thanks! I'll take a look at it next week. 

Bill

----- Original Message ----- 
From: David Whitmarsh <da...@dial.pipex.com>
To: <ne...@apache.org>
Sent: Tuesday, November 23, 1999 10:23 PM
Subject: [PATCH] Re: Time to roll the 1.3.10 tarball 


On Mon, 22 Nov 1999 11:02:39 -0500 (EST), you wrote:


>
>I doubt very much that this issue will be solved for Apache 1.3 either.
>Windows 95 is not a platform that was ever designed for use as a server of
>any kind.  There will always be somethings that won't work on 95/98.
>Unbuffered CGI's for instance won't work.  There have been some reports
>about why mod_proxy won't load on 95/98, but it isn't really worth it to
>fix this problem for 1.3.  There has been a lot of talk recently of
>re-writing mod_proxy for Apache 2.0.  Mos_proxy is a VERY complicated
>module, and it makes more sense to leave 1.3's alone and get 1.3.10 out
>the door, than to try to fix a major problem in the last release of a
>series.
>
>Of course, this is all MHO, and if somebody can submit a patch for either
>of these issues, then I would love to test and commit it.  :-)
>

This patch fixes the proxymodule load problem under 95, and also the
garbage collection problem.

I have a binary for anyone who needs it at
http://dspace.dial.pipex.com/david.whitmarsh/



************************************

David Whitmarsh

Sparkle Computer Co Ltd

Sybase C C++ perl UNIX NT

************************************