configure failure with bash-3.1 - fixed in SVN, but not in any release

2006-06-22 Thread Max Bowsher
Attempting to configure mod_python on a current Debian 'etch' system, or
any other using bash-3.1, fails, with the shell error:
  syntax error near unexpected token `('

This has already been fixed in mod_python SVN for 4 months (r376555,
MODPYTHON-122), and backported to 3.2.x (r383362), but has not yet made
it into a release.

With the growing prevalence of bash-3.1, I'd like to suggest that a
3.2.9 release fairly soon would be a good idea. If that's not likely to
happen, then a bug advisory on the www.modpython.org front-page would be
a useful interim support.

Max.




signature.asc
Description: OpenPGP digital signature


Re: configure failure with bash-3.1 - fixed in SVN, but not in any release

2006-06-22 Thread Max Bowsher
Jim Gallacher wrote:
 Rob Sanderson wrote:
 +1

 Seconded.  We have a kludge in our install script to replace sh with
 ksh, but it would be nice to be able to remove this.

 Thanks!
 
 Is anyone aware of any issues fixed in trunk that still need to be
 backported to 3.2.x? I'll check my notes tonight, but as I recall there
 is nothing outstanding so we should be good to go.
 
 One tiny thing that should go in is a fix for the DbmSession file
 creation mode, but that is a 30 second job.
 
 Otherwise, I'll roll a 3.2.9 release candidate tarball tonight.

Another *strong* motivator for 3.2.9 is Apache 2.2.x support.

Max.




signature.asc
Description: OpenPGP digital signature


3.2.9 release blocker? API breakage for mod_python.util.Field()

2006-06-22 Thread Max Bowsher
MODPYTHON-93, r387693, backported in r393781, changes the API of
mod_python.util.Field().

I think that it would be a very bad thing to change an API in an
incompatible way in a patch release - whilst people are likely to
understand that things may break going from 3.2.x to 3.3.x, they are
quite likely to expect an upgrade *within* the 3.2.x series to be
relatively painless.

Amongst the applications broken by this, are the 0.9.x current series of
Trac releases.

I suggest un-backporting the API-breaking change from the 3.2.x
maintenance branch.

Max.






signature.asc
Description: OpenPGP digital signature


Re: 3.2.9 release blocker? API breakage for mod_python.util.Field()

2006-06-22 Thread Jim Gallacher

Max Bowsher wrote:

MODPYTHON-93, r387693, backported in r393781, changes the API of
mod_python.util.Field().

I think that it would be a very bad thing to change an API in an
incompatible way in a patch release - whilst people are likely to
understand that things may break going from 3.2.x to 3.3.x, they are
quite likely to expect an upgrade *within* the 3.2.x series to be
relatively painless.

Amongst the applications broken by this, are the 0.9.x current series of
Trac releases.

I suggest un-backporting the API-breaking change from the 3.2.x
maintenance branch.


I agree that we should not make *public* API changes. However quoting 
from the docs for util.Field:



4.6.2 Field class

class Field()

This class is used internally by FieldStorage and is not meant to be 
instantiated by the user.



The parameters passed in the constructor are not documented so I don't 
think users should assume that the api will be invariant. If an 
application such as Trac chooses to ignore the documentation they do so 
at their own peril.


(That was the bad cop speaking. Now over to the good cop.)

On the other hand, to keep Trac and other such apps from breaking we can 
always find a compromise. The changes from 3.2.8 to branches/3.2.x 
currently look like this:

@@ -48,19 +48,8 @@
 

 class Field:
-   def __init__(self, name, file, ctype, type_options,
-disp, disp_options, headers = {}):
+   def __init__(self, name):
self.name = name
-   self.file = file
-   self.type = ctype
-   self.type_options = type_options
-   self.disposition = disp
-   self.disposition_options = disp_options
-   if disp_options.has_key(filename):
-   self.filename = disp_options[filename]
-   else:
-   self.filename = None
-   self.headers = headers

def __repr__(self):
Return printable representation.


So what if we back out of this, but re-factor __init__?
-def __init__(self, name):
+def __init__(self, name, file=None, ctype=None, type_options=None,
+   disp=None, disp_options=None, headers = {}):


That change should be compatible with the changes in FieldStorage, but 
should keep the likes of Trac happy. Ain't python grand? :)


Beyond that, the Trac people should either stop using Field directly, or 
advocate that it be designated for public use. I don't personally care 
either way, as long as we have a consistent policy. If something in 
mod_python is marked for internal use, it really does mean it is for 
internal use only.


And of course if the breakage is completely different from what I've 
detailed above, feel free to call me an idiot. ;)


Jim


Re: 3.2.9 release blocker? API breakage for mod_python.util.Field()

2006-06-22 Thread Max Bowsher
Jim Gallacher wrote:
 Max Bowsher wrote:
 MODPYTHON-93, r387693, backported in r393781, changes the API of
 mod_python.util.Field().

 I think that it would be a very bad thing to change an API in an
 incompatible way in a patch release - whilst people are likely to
 understand that things may break going from 3.2.x to 3.3.x, they are
 quite likely to expect an upgrade *within* the 3.2.x series to be
 relatively painless.

 Amongst the applications broken by this, are the 0.9.x current series of
 Trac releases.

 I suggest un-backporting the API-breaking change from the 3.2.x
 maintenance branch.
 
 I agree that we should not make *public* API changes. However quoting
 from the docs for util.Field:
 
 
 4.6.2 Field class
 
 class Field()
 
 This class is used internally by FieldStorage and is not meant to be
 instantiated by the user.
 

Ah, I see. It would probably be good to add a docstring to that effect -
there's no warning not to use it in the code itself - not even an
underscore in its name to hint at it being private.

 The parameters passed in the constructor are not documented so I don't
 think users should assume that the api will be invariant. If an
 application such as Trac chooses to ignore the documentation they do so
 at their own peril.
 
 (That was the bad cop speaking. Now over to the good cop.)
 
 On the other hand, to keep Trac and other such apps from breaking we can
 always find a compromise. The changes from 3.2.8 to branches/3.2.x
 currently look like this:
 @@ -48,19 +48,8 @@
  
 
  class Field:
 -   def __init__(self, name, file, ctype, type_options,
 -disp, disp_options, headers = {}):
 +   def __init__(self, name):
 self.name = name
 -   self.file = file
 -   self.type = ctype
 -   self.type_options = type_options
 -   self.disposition = disp
 -   self.disposition_options = disp_options
 -   if disp_options.has_key(filename):
 -   self.filename = disp_options[filename]
 -   else:
 -   self.filename = None
 -   self.headers = headers
 
 def __repr__(self):
 Return printable representation.
 
 
 So what if we back out of this, but re-factor __init__?
 -def __init__(self, name):
 +def __init__(self, name, file=None, ctype=None, type_options=None,
 +   disp=None, disp_options=None, headers = {}):
 
 
 That change should be compatible with the changes in FieldStorage, but
 should keep the likes of Trac happy. Ain't python grand? :)

That would be a good compromise for 3.2.x, deferring the breakage from a
patch version increment to the next minor version increment.

 Beyond that, the Trac people should either stop using Field directly, or
 advocate that it be designated for public use. I don't personally care
 either way, as long as we have a consistent policy. If something in
 mod_python is marked for internal use, it really does mean it is for
 internal use only.

The use has gone away in Trac 0.10, but that's still working its way
toward release.

 And of course if the breakage is completely different from what I've
 detailed above, feel free to call me an idiot. ;)

No, that's quite correct - the issue is the signature of __init__().

Max.



signature.asc
Description: OpenPGP digital signature


[jira] Created: (MODPYTHON-173) DbmSession creates world readable db file

2006-06-22 Thread Jim Gallacher (JIRA)
DbmSession creates world readable db file
-

 Key: MODPYTHON-173
 URL: http://issues.apache.org/jira/browse/MODPYTHON-173
 Project: mod_python
Type: Bug

  Components: session  
Versions: 3.2.8
Reporter: Jim Gallacher
 Assigned to: Jim Gallacher 
 Fix For: 3.2.x


DbmSession uses the default mode when creating the db file. As a result the 
file is world readable, which may be undesirable where sensitive informaiton is 
stored in the session. Currently the users are required to chmod the file 
manually. This can be fixed by using the option mode argument when the file is 
opened.

Quoting from the python anydbm documentation:

open(   filename[, flag[, mode]]

The optional mode argument is the Unix mode of the file, used only when the 
database has to be created. It defaults to octal 0666 (and will be modified by 
the prevailing umask).


-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira



[jira] Commented: (MODPYTHON-93) Improve util.FieldStorage efficiency

2006-06-22 Thread Jim Gallacher (JIRA)
[ 
http://issues.apache.org/jira/browse/MODPYTHON-93?page=comments#action_12417405 
] 

Jim Gallacher commented on MODPYTHON-93:


As part of the improvments to FieldStorage, the Field class __init__ method was 
changed in trunk (3.3-dev) and backported to branches/3.2.x. Although the 
documentation states that the Field class is for internal use by
FieldStorage, some software applications create Field instances directly. This 
change in the __init__ signature causes those applications to fail.

Breaking compatibility may be acceptable for a minor version increment (3.2  
3.2) but is undesirable for a patch release (3.2.8  3.2.9). Changing the Field 
class to be both forward and backward compatible wil not be difficult. As such 
I'll commit a fix to branches/3.2.x in time for a 3.2.9 release.


 Improve util.FieldStorage efficiency
 

  Key: MODPYTHON-93
  URL: http://issues.apache.org/jira/browse/MODPYTHON-93
  Project: mod_python
 Type: Improvement

   Components: core
 Versions: 3.2.7
 Reporter: Jim Gallacher
 Assignee: Jim Gallacher
 Priority: Minor
  Fix For: 3.3
  Attachments: modpython325_util_py_dict.patch

 Form fields are saved as a list in a FieldStorage class instance. The class 
 implements a __getitem__ method to provide dict-like behaviour.  This method 
 iterates over the complete list for every call to __getitem__. Applications 
 that need to access all the fields when processing the form will show O(n^2) 
 behaviour where n == the number of form fields. This overhead could be 
 avoided by creating a dict (to use as an index) when the FieldStorage 
 instance is created.
 Mike Looijmans has been investigating StringField and Field as well. It is 
 probably reasonable to include information on his work in this issue as well, 
 so that we can consider all of these efficiency issues in toto.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira



mod_python 3.2.9-rc1 available for testing

2006-06-22 Thread Jim Gallacher
The mod_python 3.2.9-rc1 tarball is available for testing. This release 
adds support for apache 2.2 as well as some other useful backports from 
the development branch. For information on the changes from 3.2.8 take a 
look at doc-html/node98.html in the tarball.


Here are the rules:

In order for a file to be officially announced, it has to be tested by
developers on the dev list. Anyone subscribed to this list can (and
should feel obligated to :-) ) test it, and provide feedback *to _this_
 list*! (Not the [EMAIL PROTECTED] list, and preferably not me
personally).

The files are (temporarily) available here:

http://people.apache.org/~jgallacher/mod_python/dist/
http://people.apache.org/~jgallacher/mod_python/dist/mod_python-3.2.9-rc1.tgz
http://people.apache.org/~jgallacher/mod_python/dist/mod_python-3.2.9-rc1.tgz.md5

Please download it, then do the usual

$ ./configure --with-apxs=/wherever/it/is
$ make
$ (su)
# make install

Then (as non-root user!)

$ cd test
$ python test.py

And see if any tests fail. If they pass, send a +1 to the list, if they
fail, send the details (the versions of OS, Python and Apache, the test
output, and suggestions, if any).

If this tarball looks good, I'll tag it svn and create a 3.2.9 final 
tarball.


Thank you for your assistance,
Jim Gallacher




mod_python 3.2.9-rc2 available for testing

2006-06-22 Thread Jim Gallacher

OK, this time for real. :)

The mod_python 3.2.9-rc2 tarball is available for testing. This release
adds support for apache 2.2 as well as some other useful backports from
the development branch. For information on the changes from 3.2.8 take a
look at doc-html/node98.html in the tarball.

Here are the rules:

In order for a file to be officially announced, it has to be tested by
developers on the dev list. Anyone subscribed to this list can (and
should feel obligated to :-) ) test it, and provide feedback *to _this_
 list*! (Not the [EMAIL PROTECTED] list, and preferably not me
personally).

The files are (temporarily) available here:

http://people.apache.org/~jgallacher/mod_python/dist/
http://people.apache.org/~jgallacher/mod_python/dist/mod_python-3.2.9-rc1.tgz
http://people.apache.org/~jgallacher/mod_python/dist/mod_python-3.2.9-rc1.tgz.md5

Please download it, then do the usual

$ ./configure --with-apxs=/wherever/it/is
$ make
$ (su)
# make install

Then (as non-root user!)

$ cd test
$ python test.py

And see if any tests fail. If they pass, send a +1 to the list, if they
fail, send the details (the versions of OS, Python and Apache, the test
output, and suggestions, if any).

If this tarball looks good, I'll tag it svn and create a 3.2.9 final
tarball.

Thank you for your assistance,
Jim Gallacher





[jira] Resolved: (MODPYTHON-84) req.sendfile(filename) sends an incorrect number of bytes when filename is a symlink

2006-06-22 Thread Jim Gallacher (JIRA)
 [ http://issues.apache.org/jira/browse/MODPYTHON-84?page=all ]
 
Jim Gallacher resolved MODPYTHON-84:


Fix Version: 3.2.8
 (was: 3.2.7)
 Resolution: Fixed

 req.sendfile(filename) sends an incorrect number of bytes when filename is a 
 symlink
 

  Key: MODPYTHON-84
  URL: http://issues.apache.org/jira/browse/MODPYTHON-84
  Project: mod_python
 Type: Bug

   Components: core
 Versions: 3.1.3, 3.2.7, 3.1.4
 Reporter: Jim Gallacher
 Assignee: Graham Dumpleton
  Fix For: 3.2.8


 This issue was reported by Wim Heirman on the mod_python list:
 When req.sendfile(filename) is called where filename is a symbolic link, only 
 part of the file is sent to the client (as many bytes as there are characters 
 in the symlink's file reference, so for a symlink pointing to '../index.html' 
  returns the first 13 bytes of the correct file).
 Wim suggested changing APR_READ to APR_FINFO_NORM in the apr_stat call in 
 req_sendfile().

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira



Re: svn commit: r416165 - /httpd/httpd/trunk/modules/filters/mod_deflate.c

2006-06-22 Thread Ruediger Pluem


On 06/22/2006 02:08 AM, Nick Kew wrote:
 On Thursday 22 June 2006 00:38, Ruediger Pluem wrote:
 
 
Is this correct? In fact you ignore flush buckets now.
 
 
 Yes, this ignores flush buckets - which is better than choking on them.

Yes, of course.

 I'm not aware of this being a problem worth worrying about unduly,
 but feel free to correct me.

Maybe there is reason to worry. See my possible real world example below.


   Shouldn't you add 
the flush bucket to ctx-proc_bb, pass it up the chain, clean ctx-proc_bb
afterwards and break if the result is not ok?
 
 
 In principle, yes.  Except for the break.  But  needs delving deeper
 into libz than I feel able to do right now, to figure out if there's going
 to be an issue of orphaned data.

I guess I did not described it correctly. I meant to say that in the case of
an error reported from the chain we should return this error to the calling 
filter
immediately. Of course it might be needed to clean up some zlib data structures 
before
to avoid leaving orphaned data

 
 I believe the real life flush bucket causing the error report is coming
 before the compressed data.
 

A possible real world case: A slow application on a Tomcat that is connected via
mod_proxy_ajp (flushing enabled) that delivers compressed content that should be
inflated by mod_deflate.

BTW: I noticed another possible issue:

What about the case where we have a file bucket of a compressed file that 
expands to
a very large file? As far as I understand the inflate output filter it captures 
the whole
inflated brigade until it passes it up the chain. So in this case we could use 
up
a lot of memory. Wouldn't it be better to pass every single heap bucket 
containing the inflated
data up the chain and let the core output filters decide on possible buffering 
before sending
data on the wire?

Regards

RĂ¼diger


Re: svn commit: r416265 - in /httpd/httpd/trunk/modules/ssl: mod_ssl.c ssl_engine_config.c ssl_engine_io.c ssl_private.h

2006-06-22 Thread Joe Orton
On Thu, Jun 22, 2006 at 06:13:08AM -, William Rowe wrote:
 Author: wrowe
 Date: Wed Jun 21 23:13:07 2006
 New Revision: 416265
 
 URL: http://svn.apache.org/viewvc?rev=416265view=rev
 Log:
 
   New SSLLogLevelDebugDump [ None (default) | IO (not bytes) | Bytes ]
   configures the I/O Dump of SSL traffic, when LogLevel is set to Debug.
   The default is none as this is far greater debugging resolution than 
   the typical administrator is prepared to untangle.
...
 ==
 --- httpd/httpd/trunk/modules/ssl/ssl_engine_io.c (original)
 +++ httpd/httpd/trunk/modules/ssl/ssl_engine_io.c Wed Jun 21 23:13:07 2006
...
 @@ -1793,7 +1798,7 @@
  rc, argi, (cmd == (BIO_CB_WRITE|BIO_CB_RETURN) ? to : 
 from),
  bio, argp,
  (argp != NULL ? (BIO dump follows) : (Oops, no memory 
 buffer?)));
 -if (argp != NULL)
 +if ((argp != NULL)  (sc-ssl_log_level = SSL_LOG_BYTES))
  ssl_io_data_dump(s, argp, rc);

Looks like the debug message above needs to be adjusted for this, it's 
going to say BIO dump follows when no BIO dump will follow (in the 
default config)?

joe


mod_isapi - backport of bugfixes to 2.2

2006-06-22 Thread Matt Lewandowsky
In the last few hours, wrowe has committed a few changes to mod_isapi 
to make it more robust. These changes would be nice to have in the 2.2 
tree, as they correct some fairly major issues. I have tested these 
patches against 2.2.2, and they pass my testing quite happily.


The commits to trunk, abbreviated log messages, and the bugs they 
resolve are as follows:


r416272 PR 16637, 28089, 30033
  Ensure we walk through all the methods the developer may have
  employed to report their HTTP status result code.

r416278 PR 30022
  We need to pay alot more attention here to ap_pass_brigade.

r416288 PR 29098
  Handle HTTP/1.1 200 OK style status lines correctly. We were
  also off by one in apr_cpystrn.

r416291 PR 15993
r416293 (cleanup of 416291)
  Stop appending a backslash if some trailing slash is present.

Backporting these to 2.2 is trivial as the patches will apply cleanly. 
I have already tested this group of commits on 2.2 and found no 
breakage.


Does anyone have any input on the matter?

Warmest,

--Matt

--
/*
 * Matt Lewandowsky[EMAIL PROTECTED]
 * Random Geek http://www.iamcode.net/
 * +1 (866) 606-9696   +44 (0) 844 484 8254
 */



Re: svn commit: r416265 - in /httpd/httpd/trunk/modules/ssl: mod_ssl.c ssl_engine_config.c ssl_engine_io.c ssl_private.h

2006-06-22 Thread William A. Rowe, Jr.

Joe Orton wrote:

@@ -1793,7 +1798,7 @@
 rc, argi, (cmd == (BIO_CB_WRITE|BIO_CB_RETURN) ? to : 
from),
 bio, argp,
 (argp != NULL ? (BIO dump follows) : (Oops, no memory 
buffer?)));
-if (argp != NULL)
+if ((argp != NULL)  (sc-ssl_log_level = SSL_LOG_BYTES))
 ssl_io_data_dump(s, argp, rc);


Looks like the debug message above needs to be adjusted for this, it's 
going to say BIO dump follows when no BIO dump will follow (in the 
default config)?


Yup, note that argp != null was the old condition to both.  (Well, the exception
case text doesn't apply to us ;-)   But you are right, this would be more sane,
additional commits and feedback are welcome, I'm really counting minutes now
trying to get ready to ship off to IE.

Bill


Re: mod_proxy_balancer/mod_proxy_ajp TODO

2006-06-22 Thread Darryl Miles

Henri Gomez wrote:

Well you we always indicate some sort of CPU power for a remote (a sort 
of bogomips) and use this in computation.


Why should the CPU power matter, what if the high power CPU is getting 
all the complex requests and the lower power CPU is ending up with 
simple request (static content).



You are better implementing it in control packets over AJP that can 
advertise that hosts current willingness to take on new requests on a 
32/64bit scale.  Call this a flow control back pressure packet, a 
stateless beacon of information which may or may not be used by the 
client AJP.


Then have a pluggable implementation at the server end of calculating 
that value and frequency for advertising it.  All the apache LB has to 
do is work from that load calculation.  All existing AJPs have to do is 
just ignore the packet.


In the case of different horse power CPUs that factor is better fed into 
the server AJP algorithm by biasing the advertised willingness to take 
on new requests after a certain low level is reached.  Only the server 
end of the AJP know the true request rate and how near that system is to 
breaking point.


This scheme also works where apache may not see all the work the backend 
is doing, like if you have a different apache front end clusters linked 
to the same single backend cluster.



Darryl


Re: mod_proxy_balancer/mod_proxy_ajp TODO

2006-06-22 Thread Henri Gomez

The TomcatoMips indicator was just something to tell that it's not the
raw CPU power which is important, but the estimated LOAD capacity of
an instance.

Accounting informations should included average time to execute a
request, number of thread in use (AJP/HTTP), estimated free memory...

Just to be sure that when a tomcat (for example), is near overload,
the next requests will be routed to another less loaded tomcat.

2006/6/22, Darryl Miles [EMAIL PROTECTED]:

Henri Gomez wrote:

 Well you we always indicate some sort of CPU power for a remote (a sort
 of bogomips) and use this in computation.

Why should the CPU power matter, what if the high power CPU is getting
all the complex requests and the lower power CPU is ending up with
simple request (static content).


You are better implementing it in control packets over AJP that can
advertise that hosts current willingness to take on new requests on a
32/64bit scale.  Call this a flow control back pressure packet, a
stateless beacon of information which may or may not be used by the
client AJP.

Then have a pluggable implementation at the server end of calculating
that value and frequency for advertising it.  All the apache LB has to
do is work from that load calculation.  All existing AJPs have to do is
just ignore the packet.

In the case of different horse power CPUs that factor is better fed into
the server AJP algorithm by biasing the advertised willingness to take
on new requests after a certain low level is reached.  Only the server
end of the AJP know the true request rate and how near that system is to
breaking point.

This scheme also works where apache may not see all the work the backend
is doing, like if you have a different apache front end clusters linked
to the same single backend cluster.


Darryl



Re: [PATCH] Don't cache expired objects

2006-06-22 Thread Paul Querna

Davi Arnaut wrote:

Don't cache requests with a expires date in the past; otherwise the cache code
will _always_ try to cache the URL, leading to numerous rename() errors - since
the URL is already cached.

...

Index: modules/cache/mod_cache.c
===
--- modules/cache/mod_cache.c   (revision 414393)
+++ modules/cache/mod_cache.c   (working copy)
@@ -426,6 +426,9 @@
 /* if a broken Expires header is present, don't cache it */
 reason = apr_pstrcat(p, Broken expires header: , exps, NULL);
 }
+else if (exps != NULL  exp != APR_DATE_BAD  exp  apr_time_now()) {
+reason = Expires header already expired, not cacheable;
+}
 else if (r-args  exps == NULL) {
 /* if query string present but no expiration time, don't cache it
  * (RFC 2616/13.9)



Instead of calling apr_time_now(), shouldn't it re-use r-request_time 
here, saving a possibly expensive call?


Otherwise, I think its a good fix.

-Paul


Re: [PATCH] Don't cache expired objects

2006-06-22 Thread Davi Arnaut
On Thu, 22 Jun 2006 06:35:05 -0700
Paul Querna [EMAIL PROTECTED] wrote:

 Davi Arnaut wrote:
  Don't cache requests with a expires date in the past; otherwise the cache 
  code
  will _always_ try to cache the URL, leading to numerous rename() errors - 
  since
  the URL is already cached.
 ...
  Index: modules/cache/mod_cache.c
  ===
  --- modules/cache/mod_cache.c   (revision 414393)
  +++ modules/cache/mod_cache.c   (working copy)
  @@ -426,6 +426,9 @@
   /* if a broken Expires header is present, don't cache it */
   reason = apr_pstrcat(p, Broken expires header: , exps, NULL);
   }
  +else if (exps != NULL  exp != APR_DATE_BAD  exp  apr_time_now()) {
  +reason = Expires header already expired, not cacheable;
  +}
   else if (r-args  exps == NULL) {
   /* if query string present but no expiration time, don't cache it
* (RFC 2616/13.9)
 
 
 Instead of calling apr_time_now(), shouldn't it re-use r-request_time 
 here, saving a possibly expensive call?
 
 Otherwise, I think its a good fix.
 

Good catch, thanks!

--
Davi Arnaut

Index: mod_cache.c
===
--- mod_cache.c (revision 416367)
+++ mod_cache.c (working copy)
@@ -426,6 +426,9 @@
 /* if a broken Expires header is present, don't cache it */
 reason = apr_pstrcat(p, Broken expires header: , exps, NULL);
 }
+else if (exps != NULL  exp != APR_DATE_BAD  exp  r-request_time) {
+reason = Expires header already expired, not cacheable;
+}
 else if (r-args  exps == NULL) {
 /* if query string present but no expiration time, don't cache it
  * (RFC 2616/13.9)



[PATCH] mod_disk_cache LFS-aware config

2006-06-22 Thread Niklas Edmundsson


This patch makes it possible to configure mod_disk_cache to cache 
files that are larger than the LFS limit. While at it, I implemented 
error handling so it doesn't accept things like

CacheMinFileSize barf anymore.

Actual LFS support (current code eats all address-space/memory in 
32bit boxes) will come in a separate patch once this is commited.


/Nikke
--
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
 Niklas Edmundsson, Admin @ {acc,hpc2n}.umu.se  | [EMAIL PROTECTED]
---
 Thesaurus: ancient reptile with an excellent vocabulary
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=Index: mod_disk_cache.c
===
--- mod_disk_cache.c(revision 416365)
+++ mod_disk_cache.c(working copy)
@@ -334,14 +334,14 @@ static int create_entity(cache_handle_t 
 if (len  conf-maxfs) {
 ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r-server,
  disk_cache: URL %s failed the size check 
- (% APR_OFF_T_FMT   % APR_SIZE_T_FMT ),
+ (% APR_OFF_T_FMT   % APR_OFF_T_FMT ),
  key, len, conf-maxfs);
 return DECLINED;
 }
 if (len = 0  len  conf-minfs) {
 ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r-server,
  disk_cache: URL %s failed the size check 
- (% APR_OFF_T_FMT   % APR_SIZE_T_FMT ),
+ (% APR_OFF_T_FMT   % APR_OFF_T_FMT ),
  key, len, conf-minfs);
 return DECLINED;
 }
@@ -1026,7 +1026,7 @@ static apr_status_t store_body(cache_han
 if (dobj-file_size  conf-maxfs) {
 ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r-server,
  disk_cache: URL %s failed the size check 
- (% APR_OFF_T_FMT % APR_SIZE_T_FMT ),
+ (% APR_OFF_T_FMT % APR_OFF_T_FMT ),
  h-cache_obj-key, dobj-file_size, conf-maxfs);
 /* Remove the intermediate cache file and return non-APR_SUCCESS */
 file_cache_errorcleanup(dobj, r);
@@ -1050,7 +1050,7 @@ static apr_status_t store_body(cache_han
 if (dobj-file_size  conf-minfs) {
 ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r-server,
  disk_cache: URL %s failed the size check 
- (% APR_OFF_T_FMT % APR_SIZE_T_FMT ),
+ (% APR_OFF_T_FMT % APR_OFF_T_FMT ),
  h-cache_obj-key, dobj-file_size, conf-minfs);
 /* Remove the intermediate cache file and return non-APR_SUCCESS */
 file_cache_errorcleanup(dobj, r);
@@ -1137,15 +1137,25 @@ static const char
 {
 disk_cache_conf *conf = ap_get_module_config(parms-server-module_config,
  disk_cache_module);
-conf-minfs = atoi(arg);
+
+if (apr_strtoff(conf-minfs, arg, NULL, 0) != APR_SUCCESS ||
+conf-minfs  0) 
+{
+return CacheMinFileSize argument must be a non-negative integer 
representing the min size of a file to cache in bytes.;
+}
 return NULL;
 }
+
 static const char
 *set_cache_maxfs(cmd_parms *parms, void *in_struct_ptr, const char *arg)
 {
 disk_cache_conf *conf = ap_get_module_config(parms-server-module_config,
  disk_cache_module);
-conf-maxfs = atoi(arg);
+if (apr_strtoff(conf-maxfs, arg, NULL, 0) != APR_SUCCESS ||
+conf-maxfs  0) 
+{
+return CacheMaxFileSize argument must be a non-negative integer 
representing the max size of a file to cache in bytes.;
+}
 return NULL;
 }
 
Index: mod_disk_cache.h
===
--- mod_disk_cache.h(revision 416365)
+++ mod_disk_cache.h(working copy)
@@ -88,8 +88,8 @@ typedef struct {
 apr_size_t cache_root_len;
 int dirlevels;   /* Number of levels of subdirectories */
 int dirlength;   /* Length of subdirectory names */
-apr_size_t minfs;/* minumum file size for cached files */
-apr_size_t maxfs;/* maximum file size for cached files */
+apr_off_t minfs; /* minimum file size for cached files */
+apr_off_t maxfs; /* maximum file size for cached files */
 } disk_cache_conf;
 
 #endif /*MOD_DISK_CACHE_H*/


Re: mod_proxy_balancer/mod_proxy_ajp TODO

2006-06-22 Thread Darryl Miles

Henri Gomez wrote:

The TomcatoMips indicator was just something to tell that it's not the
raw CPU power which is important, but the estimated LOAD capacity of
an instance.



But its still apache working out TomcatoMips.  I think that approach is 
still flawed.


I'm saying only the server end of the AJP knows the true situation.  The 
current setup presumes that the running apache instance has all the 
facts necessary to determine balancing.  When all it knows about is the 
work it has given the backend and the work rate its betting it back.



I'm thinking both ends apache and tomcat should make load calculations 
based on that they know at hand.  As far as I know there is no provision 
in the AJP to announce Willingness to serve.  Both ends should feed 
their available information and configuration biases it into their 
respective algorithm and come out with a result that can be compared 
against each other.  The worker would then announce as necessary (there 
maybe a minimum % change to damper information flap) down the connector 
the info to apache.  There probably need to be a random magic number and 
wrapping sequence number in the packet to help the apache end spot 
obvious problems.


This would allow kernel load avg/io load (and anything else) to be 
periodically taken into account at the tomcat end.  It would be expected 
that each member of the backend tomcat cluster is using the same 
algorithm to announce willingness.  Otherwise you get disparity when 
apache comes to make a decision.



So I suppose its just the framework to allow an LB worker to announce 
its willingness to serve I am calling for here.  Not any specific 
algorithm, that issue can be toyed with until the end of time.



An initial implementation would need to experiment and work out:
 * How that willingess value impacts/biases the existing apache LB 
calculations.
 * Guidelines on how to configure algorithm at each end up based on 
known factors (like CPUs, average background workload, relative IO 
performance).


I'm thinking with that you can hit the widest audience to make a usable 
default without giving much thought to configuration.  The type of 
approach kernels make these days, you only have to tweak and think about 
configuration in extreme scenarios but for the most it works well out of 
the box.



Darryl


Re: mod_proxy_balancer/mod_proxy_ajp TODO

2006-06-22 Thread Jean-frederic Clere

Henri Gomez wrote:


The TomcatoMips indicator was just something to tell that it's not the
raw CPU power which is important, but the estimated LOAD capacity of
an instance.

Accounting informations should included average time to execute a
request, number of thread in use (AJP/HTTP), estimated free memory...

Just to be sure that when a tomcat (for example), is near overload,
the next requests will be routed to another less loaded tomcat.


If you want such information I think Tomcat (or the backend server) has 
to provide it.


Cheers

Jean-Frederic



2006/6/22, Darryl Miles [EMAIL PROTECTED]:


Henri Gomez wrote:

 Well you we always indicate some sort of CPU power for a remote (a 
sort

 of bogomips) and use this in computation.

Why should the CPU power matter, what if the high power CPU is getting
all the complex requests and the lower power CPU is ending up with
simple request (static content).


You are better implementing it in control packets over AJP that can
advertise that hosts current willingness to take on new requests on a
32/64bit scale.  Call this a flow control back pressure packet, a
stateless beacon of information which may or may not be used by the
client AJP.

Then have a pluggable implementation at the server end of calculating
that value and frequency for advertising it.  All the apache LB has to
do is work from that load calculation.  All existing AJPs have to do is
just ignore the packet.

In the case of different horse power CPUs that factor is better fed into
the server AJP algorithm by biasing the advertised willingness to take
on new requests after a certain low level is reached.  Only the server
end of the AJP know the true request rate and how near that system is to
breaking point.

This scheme also works where apache may not see all the work the backend
is doing, like if you have a different apache front end clusters linked
to the same single backend cluster.


Darryl







Brigades and threads

2006-06-22 Thread Joachim Zobel
Hi.

Can I assume (no matter what mpm) that two brigades for the same request
will never be processed at the same time?

I am using request pool memory for buffering in filters, and this will
otherwise fail.

Thx,
Joachim