Re: Ceph patches for httpd

[Justin Erenkrantz]

On Thu, Jan 2, 2014 at 11:04 AM, Kean Johnston  wrote:
> Why don't you move the integer comparison to ahead of the apr_table_get and
> strcmp()? That way relatively expensive options get boolean short-circuited.

If we switch it to the default, then yah, that can help short-circuit
it.  If it's not the default, I don't think it'll matter too much.

Given the holidays (even with Jim and Ask's +1), I'll let this sit
until early next week then I'll commit it with that tweak if I don't
hear more feedback.  -- justin

Re: Ceph patches for httpd

[Justin Erenkrantz]

On Sun, Dec 29, 2013 at 3:48 PM, Jim Jagielski  wrote:
> Yeah, I'm leaning towards agreeing w/ you there.

Both directives (HttpContentLengthHeadZero and HttpExpectStrict)
committed in r1554303.  See below for how it affects the wire
protocol.

Now, let's discuss the defaults.  =)  I'm +1 to the below patch.  -- justin

Index: modules/http/http_filters.c
===
--- modules/http/http_filters.c (revision 1554304)
+++ modules/http/http_filters.c (working copy)
@@ -1254,7 +1254,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_heade
 if (r->header_only
 && (clheader = apr_table_get(r->headers_out, "Content-Length"))
 && !strcmp(clheader, "0")
-&& conf->http_cl_head_zero != AP_HTTP_CL_HEAD_ZERO_ENABLE) {
+&& conf->http_cl_head_zero == AP_HTTP_CL_HEAD_ZERO_DISABLE) {
 apr_table_unset(r->headers_out, "Content-Length");
 }

---
% touch foo.txt

No HttpContentLengthHeadZero directive
% curl -I http://127.0.0.1:8080/foo.txt
HTTP/1.1 200 OK
Date: Mon, 30 Dec 2013 19:26:49 GMT
Server: Apache/2.5.0-dev (Unix) mod_fcgid/2.3.10-dev
Last-Modified: Mon, 30 Dec 2013 19:24:39 GMT
ETag: "0-4eec56519cbc0"
Accept-Ranges: bytes
Content-Type: text/plain

HttpContentLengthHeadZero on
% curl -I http://127.0.0.1:8080/foo.txt
HTTP/1.1 200 OK
Date: Mon, 30 Dec 2013 19:26:10 GMT
Server: Apache/2.5.0-dev (Unix) mod_fcgid/2.3.10-dev
Last-Modified: Mon, 30 Dec 2013 19:24:39 GMT
ETag: "0-4eec56519cbc0"
Accept-Ranges: bytes
Content-Length: 0
Content-Type: text/plain

HttpContentLengthHeadZero off
% curl -I http://127.0.0.1:8080/foo.txt
HTTP/1.1 200 OK
Date: Mon, 30 Dec 2013 19:27:05 GMT
Server: Apache/2.5.0-dev (Unix) mod_fcgid/2.3.10-dev
Last-Modified: Mon, 30 Dec 2013 19:24:39 GMT
ETag: "0-4eec56519cbc0"
Accept-Ranges: bytes
Content-Type: text/plain

---

% curl -H "Expect: 100-continue" -I http://127.0.0.1:8080/foo.txt
HTTP/1.1 200 OK
Date: Mon, 30 Dec 2013 19:46:36 GMT
Server: Apache/2.5.0-dev (Unix) mod_fcgid/2.3.10-dev
Last-Modified: Mon, 30 Dec 2013 19:24:39 GMT
ETag: "0-4eec56519cbc0"
Accept-Ranges: bytes
Content-Length: 0
Connection: close
Content-Type: text/plain

No HttpExpectStrict directive
% curl -H "Expect: 100-contine" -I http://127.0.0.1:8080/foo.txt
HTTP/1.1 417 Expectation Failed
Date: Mon, 30 Dec 2013 19:48:19 GMT
Server: Apache/2.5.0-dev (Unix) mod_fcgid/2.3.10-dev
Content-Length: 0
Content-Type: text/html; charset=iso-8859-1

HttpExpectStrict on
% curl -H "Expect: 100-contine" -I http://127.0.0.1:8080/foo.txt
HTTP/1.1 417 Expectation Failed
Date: Mon, 30 Dec 2013 19:46:46 GMT
Server: Apache/2.5.0-dev (Unix) mod_fcgid/2.3.10-dev
Content-Length: 0
Content-Type: text/html; charset=iso-8859-1

HttpExpectStrict off
% curl -H "Expect: 100-contine" -I http://127.0.0.1:8080/foo.txt
HTTP/1.1 200 OK
Date: Mon, 30 Dec 2013 19:47:53 GMT
Server: Apache/2.5.0-dev (Unix) mod_fcgid/2.3.10-dev
Last-Modified: Mon, 30 Dec 2013 19:24:39 GMT
ETag: "0-4eec56519cbc0"
Accept-Ranges: bytes
Content-Length: 0
Content-Type: text/plain

Re: Ceph patches for httpd

[Justin Erenkrantz]

On Sun, Dec 29, 2013 at 10:16 AM, Jim Jagielski  wrote:
> As defaults, I'm -0, but as controllable alternatives: +1

I will whip up a patch for these two - will probably commit straight
to trunk.  We can discuss the defaults separately.  But, the more I
think about it, hiding C-L of 0 on HEAD seems like an incorrect
out-of-the-box behavior.  -- justin

Re: OT (Was: Re: Ceph patches for httpd)

[Justin Erenkrantz]

This is why I am trying to fold the patches back in.  There is a definite
difference between open source and a real community.  =)  -- justin
On Dec 29, 2013 10:17 AM, "Jim Jagielski"  wrote:

> It is for reasons like this that I tend to dislike Github,
> simply because it creates this mentality...
>
> What mentality you may say? The mentality to work separate
> from the community. I am sure that there are useful things
> in this repo, but instead of working w/ us, and making
> us aware of them, they instead just spin out something
> separate. Why?
>
> IMO, this tendency to "do your own thing" is one reason
> why BSD failed while Linux succeeded... The result is
> a *more* fragmented community, not a more cohesive and
> collaborative one.
>
> On Dec 27, 2013, at 8:57 AM, Justin Erenkrantz 
> wrote:
>
> > Hi all!
> >
> > There are two patches that the Ceph community has applied to their httpd
> packages in combination with radosgw (S3 endpoint) - (see
> https://github.com/ceph/apache2).
> >
> > One of them is to allow Content-Length of '0' to be emitted from HEAD
> requests:
> >
> >
> https://github.com/ceph/apache2/commit/0d9948f1e483386adef0841896484db8422127b2
> >
> > The use case here is that someone could store a zero-byte file inside of
> radosgw.  Amazon's S3 clients expect to see a Content-Length on HEAD
> requests - IOW, they don't infer the lack of a Content-Length as being '0'.
>  If we weren't comfortable allowing this as a default, I'm guessing that we
> could expose this as a directive override.
> >
> > The other patch is to relax some of the checks around Expect headers as
> not all S3 clients emit compliant headers:
> >
> >
> https://github.com/ceph/apache2/commit/5ae1b4a081b05fcacf55e7114eec87d9b2a0a5da
> >
> > Again, I guess that we could apply a directive here to relax the check.
> >
> > If we go the directive route, both are relatively easy to whip up
> patches for, but I wanted to get some feedback before I commit anything to
> trunk.
> >
> > Cheers.  -- justin
>
>

Re: mod_fcgid and streaming large request bodies

[Justin Erenkrantz]

On Fri, Dec 27, 2013 at 6:42 PM, Jeff Trawick  wrote:
> It would be quite valuable if there is a limit on how much can be pre-read
> (0 for pure streaming).  Pre-reading the request body reduces the number of
> application processes or threads required, and they are usually fatter
> (sometimes dramatically so) than httpd workers.  While the current
> implementation is truly ridiculous, there's some goodness that could be
> kept.  I'll try to look through this in the next few days and see if it is
> practical to keep the best of both versions.  If no round tuits are
> available we should at least proceed with this, perhaps changing the
> directive syntax to allow a hybrid implementation in the future with no
> legacy cruft.  (Wild suggestion: FcgiPreReadRequestBody
> Off|On|Unlimited|K, where On is K)

I definitely agree that doing some type of pre-read may be often
useful before we start sending stuff via FastCGI.  Yet, the current
code structure doesn't lend itself to that very well - I guess what we
could do is convert the IPC to be opened lazily rather than always
open it.  If we do that, then I think the code changes would be
manageable.  (In our use case, reading the first say 64KB into RAM is
fine, but we definitely want to avoid spooling to disk.)

I'll see if I can find some round tuits...

Cheers.  -- justin

Re: mod_fcgid and streaming large request bodies

[Justin Erenkrantz]

On Fri, Dec 27, 2013 at 8:49 AM, Justin Erenkrantz
 wrote:
> Anyway, I'm diving into the code a bit - but, I figured it might be useful
> to see if anyone else has any thoughts about how to handle large request
> bodies with mod_fcgid.

Here's a first-cut patch that compiles at least.  Cheers.  -- justin

Add FcgidStreamRequestBody directive to send request body as it arrives without
storing it in memory or on disk.

* modules/fcgid/mod_fcgid.c
  (fcgid_cmds): Add FcgidStreamRequestBody directive.
* modules/fcgid/fcgid_conf.h
  (fcgid_server_conf): Add stream_request_body/stream_request_body_set flags.
* modules/fcgid/fcgid_conf.c
  (DEFAULT_STREAM_REQUEST_BODY): Set to 0.
  (create_fcgid_server_config): Init stream_request_body flag.
  (merge_fcgid_server_config): Merge new stream_request_body flag.
  (set_stream_request_body): New config helper.
* modules/fcgid/fcgid_bridge.c
  (add_request_body): Add a forward declaration to reduce diff (for now);
  take fcgid_bucket_ctx param; if stream_request_body is set, don't copy
  bucket and then call proc_write_ipc to send out that brigade and clear it
  out before next loop iteration.
  (bridge_request): Delay reading the request body until later.
  (handle_request_ipc): Move add_request_body call to here and append the EOS
  bucket before we do the final write of the request body.

Index: modules/fcgid/fcgid_bridge.c
===
--- modules/fcgid/fcgid_bridge.c (revision 1553671)
+++ modules/fcgid/fcgid_bridge.c (working copy)
@@ -287,6 +287,10 @@ static int getsfunc_fcgid_BRIGADE(char *buf, int l
 return done;
 }

+static int add_request_body(request_rec *r, apr_pool_t *request_pool,
+apr_bucket_brigade *output_brigade,
+fcgid_bucket_ctx *bucket_ctx);
+
 static int
 handle_request_ipc(request_rec *r, int role,
apr_bucket_brigade *output_brigade,
@@ -295,9 +299,21 @@ handle_request_ipc(request_rec *r, int role,
 int cond_status;
 apr_status_t rv;
 apr_bucket_brigade *brigade_stdout;
+apr_bucket *bucket_eos;
 char sbuf[MAX_STRING_LEN];
 const char *location;

+if (role == FCGI_RESPONDER) {
+rv = add_request_body(r, r->pool, output_brigade, bucket_ctx);
+if (rv) {
+return rv;
+}
+}
+
+/* The eos bucket now */
+bucket_eos = apr_bucket_eos_create(r->connection->bucket_alloc);
+APR_BRIGADE_INSERT_TAIL(output_brigade, bucket_eos);
+
 /* Write output_brigade to fastcgi server */
 if ((rv = proc_write_ipc(&bucket_ctx->ipc,
  output_brigade)) != APR_SUCCESS) {
@@ -304,6 +320,7 @@ handle_request_ipc(request_rec *r, int role,
 bucket_ctx->has_error = 1;
 return HTTP_INTERNAL_SERVER_ERROR;
 }
+apr_brigade_cleanup(output_brigade);

 /* Create brigade */
 brigade_stdout =
@@ -522,7 +539,8 @@ handle_request(request_rec * r, int role, fcgid_cm
 }

 static int add_request_body(request_rec *r, apr_pool_t *request_pool,
-apr_bucket_brigade *output_brigade)
+apr_bucket_brigade *output_brigade,
+fcgid_bucket_ctx *bucket_ctx)
 {
 apr_bucket *bucket_input, *bucket_header;
 apr_file_t *fd = NULL;
@@ -563,8 +581,6 @@ static int add_request_body(request_rec *r, apr_po
 return HTTP_INTERNAL_SERVER_ERROR;
 }

-
-
 while ((bucket_input = APR_BRIGADE_FIRST(input_brigade)) !=
APR_BRIGADE_SENTINEL(input_brigade)) {
 const char *data;
 apr_size_t len;
@@ -615,7 +631,9 @@ static int add_request_body(request_rec *r, apr_po
 return HTTP_INTERNAL_SERVER_ERROR;
 }

-if (request_size > sconf->max_mem_request_len) {
+if (sconf->stream_request_body) {
+bucket_stdin = bucket_input;
+} else if (request_size > sconf->max_mem_request_len) {
 apr_size_t wrote_len;
 static const char *fd_key = "fcgid_fd";

@@ -701,6 +719,16 @@ static int add_request_body(request_rec *r, apr_po
 }
 APR_BRIGADE_INSERT_TAIL(output_brigade, bucket_header);
 APR_BRIGADE_INSERT_TAIL(output_brigade, bucket_stdin);
+
+if (sconf->stream_request_body) {
+/* Write output_brigade to fastcgi server */
+if ((rv = proc_write_ipc(&bucket_ctx->ipc,
+ output_brigade)) != APR_SUCCESS) {
+bucket_ctx->has_error = 1;
+return HTTP_INTERNAL_SERVER_ERROR;
+}
+apr_brigade_cleanup(output_brigade);
+}
 }

 apr_brigade_cleanup(input_brigade);
@@ -731,7 +759,6 @@ static int add_request_body(request_rec *

Re: Ceph patches for httpd

[Justin Erenkrantz]

On Fri, Dec 27, 2013 at 10:53 AM, Ask Bjørn Hansen wrote:

>
> On Dec 27, 2013, at 14:57, Justin Erenkrantz 
> wrote:
>
> > The use case here is that someone could store a zero-byte file inside of
> radosgw.  Amazon's S3 clients expect to see a Content-Length on HEAD
> requests - IOW, they don't infer the lack of a Content-Length as being '0'.
>  If we weren't comfortable allowing this as a default, I'm guessing that we
> could expose this as a directive override.
>
> Why would that not be a reasonable default?
>

As we approach 2014, I'd be fine with allowing '0' as a legitimate C-L
value.  Ryan has a comment in line 1237 that says:

---
/* This is a hack, but I can't find anyway around it.  The idea is that
 * we don't want to send out 0 Content-Lengths if it is a head request.
 * This happens when modules try to outsmart the server, and return
 * if they see a HEAD request.  Apache 1.3 handlers were supposed to
 * just return in that situation, and the core handled the HEAD.  In
 * 2.0, if a handler returns, then the core sends an EOS bucket down
 * the filter stack, and the content-length filter computes a C-L of
 * zero and that gets put in the headers, and we end up sending a
 * zero C-L to the client.  We can't just remove the C-L filter,
 * because well behaved 2.0 handlers will send their data down the
stack,
 * and we will compute a real C-L for the head request. RBB
 */
if (r->header_only
&& (clheader = apr_table_get(r->headers_out, "Content-Length"))
&& !strcmp(clheader, "0")) {
apr_table_unset(r->headers_out, "Content-Length");
}
---

The commit log is:

r86976 | rbb | 2000-11-15 17:08:44 -0500 (Wed, 15 Nov 2000) | 6 lines

Do not send a content-length if and only if this is a HEAD request and
the content-length is 0.  The problem is that the C-L on a HEAD response
has to be the correct C-L, but if a handler returns saying the handled
the request without sending data, the core sends an EOS down the filter
stack, and we compute a 0 C-L.



+1 for just removing this whole block; but, I guess that there could
*still* be some broken filters/generators that set C-L to 0 when they
shouldn't.  -- justin

Ceph patches for httpd

[Justin Erenkrantz]

Hi all!

There are two patches that the Ceph community has applied to their httpd
packages in combination with radosgw (S3 endpoint) - (see
https://github.com/ceph/apache2).

One of them is to allow Content-Length of '0' to be emitted from HEAD
requests:

https://github.com/ceph/apache2/commit/0d9948f1e483386adef0841896484db8422127b2

The use case here is that someone could store a zero-byte file inside of
radosgw.  Amazon's S3 clients expect to see a Content-Length on HEAD
requests - IOW, they don't infer the lack of a Content-Length as being '0'.
 If we weren't comfortable allowing this as a default, I'm guessing that we
could expose this as a directive override.

The other patch is to relax some of the checks around Expect headers as not
all S3 clients emit compliant headers:

https://github.com/ceph/apache2/commit/5ae1b4a081b05fcacf55e7114eec87d9b2a0a5da

Again, I guess that we could apply a directive here to relax the check.

If we go the directive route, both are relatively easy to whip up patches
for, but I wanted to get some feedback before I commit anything to trunk.

Cheers.  -- justin

mod_fcgid and streaming large request bodies

[Justin Erenkrantz]

Hi all!

I'm currently using FastCGI for an application - in particular, Ceph's
radosgw (S3 endpoint).  I was told to use mod_fastcgi as mod_fcgid doesn't
handle large request bodies appropriately.  Yet, when I looked at the
mod_fastcgi code, I shrieked in horror.  =)

In looking at add_request_body in fcgid_bridge.c, it looks like we read the
request body into memory and if it crosses a threshold, we spool it to
disk.  After everything is read from the client, then mod_fcgid begins to
write to the FastCGI socket.  In my use case, we would be PUT'ing large
request bodies that are many GB in size - so, I'd rather just stream it to
the FastCGI socket and take the hit on potentially blocking other incoming
requests.  (In our case, it makes no sense to spool the request body to
local disk.)

Anyway, I'm diving into the code a bit - but, I figured it might be useful
to see if anyone else has any thoughts about how to handle large request
bodies with mod_fcgid.

Happy holidays!  -- justin

Re: [VOTE] Release Apache httpd 2.4.4 as GA

[Justin Erenkrantz]

On Thu, Feb 21, 2013 at 5:27 AM, Justin Erenkrantz wrote:

> Anybody know if it still exists in Illumos?  This sounds like a fun thing
> to tackle next week in Portland.  =)  (I'll be there all week.)  -- justin


As far as I can tell, multiple listeners and graceful are fine with event
MPM on SmartOS, but I'll take a look at prefork tomorrow.  If it's just
prefork-related, then it's probably reasonable to just disable prefork on
Solaris.  =)  (Oh, heck, let's remove it everywhere!)  *duck*  -- justin

Re: [VOTE] Release Apache httpd 2.4.4 as GA

[Justin Erenkrantz]

Anybody know if it still exists in Illumos?  This sounds like a fun thing
to tackle next week in Portland.  =)  (I'll be there all week.)  -- justin

On Tue, Feb 19, 2013 at 8:01 PM, Jim Jagielski  wrote:

> That is one ugly bug...
>
> On Feb 19, 2013, at 7:50 PM, Rainer Jung  wrote:
>
> > On 20.02.2013 01:20, Eric Covener wrote:
> >> On Tue, Feb 19, 2013 at 7:05 PM, Rainer Jung 
> wrote:
> >>> Here's what I see concerning the graceful restart problem on Solaris.
> >>> Setup using the prefork MPM with two http listeners. Accept mutex is
> >>> pthread.
> >>
> >> maybe https://issues.apache.org/bugzilla/show_bug.cgi?id=49504 ?
> >
> > Yep, that's it, thanks!
> >
> > As described in the ticket the mutex is no longer initialized. So old
> > problem and for some reason I haven't experienced it before.
> >
> > So on to the next problem (crashes etc).
> >
> > Regards,
> >
> > Rainer
> >
>
>

Re: Apache httpd at FOSDEM?

[Justin Erenkrantz]

On Tue, Jan 29, 2013 at 12:50 PM, Rainer Jung wrote:

> On 29.01.2013 11:19, Micha Lenk wrote:
> > Hi all,
> >
> > I am going to attend (first time) the FOSDEM this year and would like to
> > know if there is a chance to meet Apache httpd developers there. I
> > didn't find a dedicated httpd event in the schedule. Is there any other
> > typical event or occasion where to meet some of you?
>
> I will be there. I'll send you my cellphone number via direct mail just
> in case. I think the OpenOffice people will have a booth somehow, so at
> least that could be a place to meet. I didn't yet look at the program
> what would be a good time.
>

I arrive in Brussels on Friday morning and head back on Monday.  Given how
big FOSDEM is, it'll probably be best to coordinate a time/place to meet
rather than expecting we'll just stumble into each other.  =)  -- justin

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

[Justin Erenkrantz]

On Tue, May 24, 2011 at 8:53 PM, Ruediger Pluem  wrote:
> On 05/24/2011 07:41 PM, Justin Erenkrantz wrote:
>> On Mon, May 23, 2011 at 8:08 PM, Ruediger Pluem  wrote:
>>> Indeed this sounds like a general problem, but
>>> ap_remove_output_filter does adjust r->output_filters if f->r is set and the
>>> filter is the first one in the chain.
>>> Which responses exactly suffer from this behaviour?
>>
>> Use "SetOutputFilter DEFLATE" and issue a GET request for a file w/o
>> Accept-Encoding headers to Subversion (via mod_dav).  You'll see that
>> mod_deflate tries to remove itself, but mod_dav keeps invoking it.
>> It's at the point where most companies who support Subversion refuse
>> to support mod_deflate/gzip due to this issue.
>
> Thanks. IMHO this is a design flaw in the DAV provider API in conjunction
> with the current filter API:
>
>  dav_error * (*deliver)(const dav_resource *resource,
>                           ap_filter_t *output);
>
> deliver is usually called with r->output_filters as a second parameter and
> thus makes a copy of the contents of r->output_filters and hence does not
> notice if r->output_filters changes during the process of pushing content thru
> the filter chain by multiple calls to ap_pass_brigade.
> My idea would be to change the DAV API to either
>
> dav_error * (*deliver)(const dav_resource *resource,
>                           request_rec *r);
>
> and using r->output_filters in the implementations of the deliver method in
> the dav_providers or
>
> dav_error * (*deliver)(const dav_resource *resource,
>                           ap_filter_t **output);
>
> and using *output in the implementations of the deliver method in
> the dav_providers and calling deliver with &(r->output_filters) as second 
> parameter.
>
> BTW: Same thing applies to deliver_report and merge.

Sure - changing APIs like this is one way to solve it; but that's not
a legitimate option for 2.2.x.  When this was discussed on
dev@subversion, I know Greg was reluctant to see the mod_dav APIs
revved after so long.  =)  -- justin

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

[Justin Erenkrantz]

On Mon, May 23, 2011 at 8:08 PM, Ruediger Pluem  wrote:
> Indeed this sounds like a general problem, but
> ap_remove_output_filter does adjust r->output_filters if f->r is set and the
> filter is the first one in the chain.
> Which responses exactly suffer from this behaviour?

Use "SetOutputFilter DEFLATE" and issue a GET request for a file w/o
Accept-Encoding headers to Subversion (via mod_dav).  You'll see that
mod_deflate tries to remove itself, but mod_dav keeps invoking it.
It's at the point where most companies who support Subversion refuse
to support mod_deflate/gzip due to this issue.

HTH.  -- justin

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

[Justin Erenkrantz]

On Mon, May 23, 2011 at 6:00 AM, Graham Leggett  wrote:
> On 23 May 2011, at 5:57 PM, Justin Erenkrantz wrote:
>
>> mod_dav uses r->output_filters - but, the pointer never gets updated
>> when it is the first one in the chain.  Hence, we call mod_deflate all
>> the time even on a request that can't support it - so we have to avoid
>> repeated memory allocations in that case.  -- justin
>
> This sounds like a general problem though, are other modules potentially
> affected?

Yes - this why I said in my earlier post that we'd likely need to
re-architect filters to deal with it.  -- justin

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

[Justin Erenkrantz]

On Mon, May 23, 2011 at 4:54 AM, Ruediger Pluem  wrote:
> Hm, why is it not sufficient that we call ap_remove_output_filter and remove 
> the
> filter from the chain if we do not initialize?
> Does ap_remove_output_filter not remove the filter when it is the first one
> in the chain?

mod_dav uses r->output_filters - but, the pointer never gets updated
when it is the first one in the chain.  Hence, we call mod_deflate all
the time even on a request that can't support it - so we have to avoid
repeated memory allocations in that case.  -- justin

Re: 1.7 Performance via HTTP

[Justin Erenkrantz]

On Thu, May 12, 2011 at 9:24 AM, Justin Erenkrantz
 wrote:
> This patch should fix the memory leak.  As I mentioned in a follow-up,
> we can discuss in Dublin or Berlin if there is a better way to solve
> this dangling filter reference...but, this will do the trick and
> should be back-portable to 2.2 (and 2.0 too, I guess).  -- justin

Committed a slight variant of this to httpd trunk in r1103315.  -- justin

Re: 1.7 Performance via HTTP

[Justin Erenkrantz]

[ adding dev@httpd ]

On Thu, May 12, 2011 at 12:25 AM, Ivan Zhakov  wrote:
> On Thu, May 12, 2011 at 11:18, Justin Erenkrantz  
> wrote:
>> On Thu, May 12, 2011 at 12:16 AM, Ivan Zhakov  wrote:
>>> Unfortunately huge memory leak in mod_deflate/mod_dav prevents
>>> enabling gzip on compression on production servers, especially on
>>> Windows where single process mode is used:
>>> http://svn.haxx.se/dev/archive-2009-08/0274.shtml
>>
>> *shrug*
>>
>> Sounds like that would be a good thing to work on in either Dublin or
>> Berlin.  Again, the fix is pretty straightforward...  -- justin
>>
> It would be great! Since I'm not familiar with this part of codebase
> and cannot fix it myself.

This patch should fix the memory leak.  As I mentioned in a follow-up,
we can discuss in Dublin or Berlin if there is a better way to solve
this dangling filter reference...but, this will do the trick and
should be back-portable to 2.2 (and 2.0 too, I guess).  -- justin

Index: modules/filters/mod_deflate.c
===
--- modules/filters/mod_deflate.c   (revision 1102210)
+++ modules/filters/mod_deflate.c   (working copy)
@@ -303,6 +303,7 @@ typedef struct deflate_ctx_t
 unsigned char *validation_buffer;
 apr_size_t validation_buffer_length;
 int inflate_init;
+int filter_init;
 } deflate_ctx;

 /* Number of validation bytes (CRC and length) after the compressed data */
@@ -445,6 +446,8 @@ static apr_status_t deflate_out_filter(ap_filter_t
 char *token;
 const char *encoding;

+ctx = f->ctx = apr_pcalloc(r->pool, sizeof(*ctx));
+
 /*
  * Only work on main request, not subrequests,
  * that are not a 204 response with no content
@@ -563,7 +566,7 @@ static apr_status_t deflate_out_filter(ap_filter_t
  */

 if (r->status != HTTP_NOT_MODIFIED) {
-ctx = f->ctx = apr_pcalloc(r->pool, sizeof(*ctx));
+ctx->filter_init = 1;
 ctx->bb = apr_brigade_create(r->pool, f->c->bucket_alloc);
 ctx->buffer = apr_palloc(r->pool, c->bufferSize);
 ctx->libz_end_func = deflateEnd;
@@ -583,6 +586,7 @@ static apr_status_t deflate_out_filter(ap_filter_t
  * We are not able to init libz and pass data down the chain
  * uncompressed.
  */
+ctx->filter_init = 0;
 ap_remove_output_filter(f);
 return ap_pass_brigade(f->next, bb);
 }
@@ -629,6 +633,11 @@ static apr_status_t deflate_out_filter(ap_filter_t
 /* initialize deflate output buffer */
 ctx->stream.next_out = ctx->buffer;
 ctx->stream.avail_out = c->bufferSize;
+} else if (!ctx->filter_init) {
+/* Hmm.  We've run through the filter init before as we have a ctx,
+ * but we never initialized.  We probably have a dangling ref.  Bail.
+ */
+return ap_pass_brigade(f->next, bb);
 }

 while (!APR_BRIGADE_EMPTY(bb))

Re: Tuning recommendations for Apache HTTP Server w/ra_serf clients (MaxKeepAliveRequests)

[Justin Erenkrantz]

On Sun, Jan 23, 2011 at 11:26 AM, Stefan Sperling  wrote:
> On Sun, Jan 23, 2011 at 09:27:18AM -0800, Justin Erenkrantz wrote:
>> If we also have a tuning section, we should remind folks to also
>> enable mod_deflate (SetOutputFilter DEFLATE in their Location block)
>> as that'll help a bit when transferring XML content back and forth.
>> mod_deflate will come at a slight latency penalty, but that'll be
>> offset for folks with slower connections taking less time to transfer
>> the responses overall.
>
> Has this problem been fixed, then?
> http://svn.haxx.se/dev/archive-2009-08/0274.shtml
> I don't think we should recommend mod_deflate if that problem still exists.

No, it doesn't look like it has been fixed yet.

However, how many real-world HTTP clients are out there that don't
speak zlib?  =)

> I don't think that thread came to a conclusion.
> Greg was implying the bug was in mod_deflate rather than svn:
> http://svn.haxx.se/dev/archive-2009-08/0290.shtml

Greg is correct in that the patch doesn't go far enough - mod_dav_svn
should change all uses of output to be a ** rather than *.

As far as mod_deflate being broken, yah, it probably shouldn't be
trying to do any memory usage until it knows it is
activated...but...that's a topic for another list (dev@httpd).
mod_deflate should probably memorize the fact that the checks have
already run and get out of the way...probably with setting no-gzip in
subprocess_env?  -- justin

Re: httpd trunk broken with Subversion: ap_log_rerror busted

[Justin Erenkrantz]

On Wed, Aug 25, 2010 at 2:51 PM, Stefan Fritsch  wrote:
> This is a good idea. I have started it in r989377.

Thanks!  -- justin

Re: httpd trunk broken with Subversion: ap_log_rerror busted

[Justin Erenkrantz]

On Wed, Aug 25, 2010 at 12:14 PM, Ruediger Pluem  wrote:
> I guess his comment was more about the macro wrappers and the C99 stuff,
> which saves us the function call if we would not log due to the loglevel
> currently set. I think this is one aspect of the changes and one were
> we might lack a prove that it really improves performance.

Exactly.

We changed the prototype for ap_log_rerror and friends, but made it so
that it is near-impenetrable to know what changed.  The #ifdef DOXYGEN
approach I outlined earlier would go a way to make it easier for
humans to clearly understand what the altered API is intended to be.
-- justin

Re: httpd trunk broken with Subversion: ap_log_rerror busted

[Justin Erenkrantz]

On Wed, Aug 25, 2010 at 4:48 AM, Stefan Fritsch  wrote:
> I agree that the comments/documentation should be improved. I will write a
> how-to for adjusting modules to the new API.

Here is a constructive suggestion (*grin*): in APR, for some of the
more complex declarations (see apr_pools.h and apr_pool_create), what
we have done is to create a #ifdef DOXYGEN tag which indicates the
normalized reduced form of the function declaration.  The #else clause
contains the optimized, macro-ized version.  (In httpd, I don't know
what the appropriate #ifdef should be though.)

This way those reading http_log.h will be able to see what the
declaration for ap_log_rerror actually reduces to without trying to go
through several layers of indirection.  -- justin

Re: httpd trunk broken with Subversion: ap_log_rerror busted

[Justin Erenkrantz]

On Wed, Aug 25, 2010 at 1:59 AM, "Plüm, Rüdiger, VF-Group"
 wrote:
> Basicly all this stuff is done to use the advanced features of C99 compilers
> that allow us to check with low effort whether a call to the error logging
> function is needed at all given the currently configured loglevel.
> This should improve performance, especially as the new logging API encourages
> to log even more details as it is now possible to limit the level of log 
> detail
> to specific modules or even requests.
> For the gory details the dev@ archives should be consulted. See various
> threads in March, April and May that all have loglevel in the subject.

I read through the archives and only saw anecdotal evidence that it'd
give a performance improvement.  (If there is a pointer indicating the
specific speed-up of all of this indirection and I just missed it,
lemme know.)

This is the crux of my concern - "should" improve performance at the
very substantial cost of increasing complexity for understanding the
codebase (especially without comments).

Turning on grumpy old man mode, I don't think we ever used to tradeoff
understanding of the code for .1% performance improvement.  It
smells of premature optimization and the amount of indirection makes
very very hard to track down what is going wrong when the API is
busted.  -- justin

httpd trunk broken with Subversion: ap_log_rerror busted

[Justin Erenkrantz]

In r951893, httpd modified a #define for APLOG_MARK to add in a new
parameter called APLOG_MODULE_INDEX (in addition to file and line
info).

This busts Subversion - specifically, mod_authz_svn which has a function called:

static void
log_access_verdict(const char *file, int line,
   const request_rec *r, int allowed,
   const char *repos_path, const char *dest_repos_path)

it is called with:

  log_access_verdict(APLOG_MARK, r, 1, repos_path, dest_repos_path);

Reading the very obtuse (unhelpful) commit log for r951893 as well as
any comments in that general area of http_log.h (which are lacking &
unhelpful), I have no idea what this APLOG_MODULE_INDEX is or why it's
even there.

Furthermore, this error case is very very hard to track down because
we're relying upon multiple levels of #define's and indirections
(hidden static variables??, weird STDC wrappers, redefinitions of
function names, etc, etc.).  So, it's not going to be obvious to
downstream developers what is going on and why it is broken.  In
general, I'm not quite sure it's a good idea to bust a #define that
has been the same since the 1.3 days (if not earlier actually).  If we
want modules to use a new optimized log API, then we should introduce
a new optimized log API and not bust the old one in a way that the
failure case isn't obvious to track down.

Yes, we could fix this by making mod_authz_svn conditional on the new
MMN, but - again, it's about even figuring out that the API is changed
and what to do about it.  The root cause is just way too obscured,
IMO.

(I wish I could veto this whole commit just on over-complication alone
- it's not an elegant solution at all.)

My $.02.  -- justin

Re: RFC: drop support for OpenSSL < 1.0 in trunk/2.3?

[Justin Erenkrantz]

On Wed, Jun 2, 2010 at 4:23 PM, Joe Orton  wrote:
> Thanks very much for all the responses.  There is strong consensus for
> retaining support for some varieties of 0.9.8 and possibly some 0.9.7.
>
> A new RFC, then, for trunk/2.3 and beyond:
>
> - support and build warning-free with OpenSSL >= 0.9.8
> - support and build with OpenSSL >= 0.9.7a, albeit with (harmless)
>  compiler warnings about argument const-ness all over the shop
> - drop support for OpenSSL < 0.9.7a
> - drop support for non-OpenSSL/derivatives of OpenSSL
>
> (I have tried this out and it seems perfectly feasible.)

+1.  -- justin

Re: [PATCH] Support compiling httpd against APR/apr-util 1.x when buildconf against 2.x/trunk

[Justin Erenkrantz]

On Sat, May 29, 2010 at 7:25 PM, William A. Rowe Jr.
 wrote:
> Is your thinking to use a find_ap[r|u] setup that can resolve either/or
> at ./configure-time?

Yes.

> Since we've acknowledged that apr/apu are hand-in-hand with 2.0, does it
> make sense to merge these and distribute both, or to teach find_apu that
> a found-apr satisfied its prerequisite?

We could merge the APR-util macro into find_apr.m4 - seems like a
bikeshed whether it's in separate files or the same.  (I have no real
preference.)

> It would be good to make this painless for combining httpd and other apps
> to either 1.x or 2.x apr.

+1.  -- justin

Re: [PATCH] Support compiling httpd against APR/apr-util 1.x when buildconf against 2.x/trunk

[Justin Erenkrantz]

On Sat, May 29, 2010 at 7:39 PM, Sander Temme  wrote:
> Wouldn't we rather move to a construct where APR is not needed to buildconf?  
> If all we're doing is copy in find_apr.m4, can't we just fork that and allow 
> it to evolve into our own idea of finding an appropriate APR against which to 
> configure?
>
> That would render the issue of buildconf against 2.x/trunk vs. configure 
> against 1.x moot.

The reason we added find_ap{ru}.m4 initially was to prevent exactly this fork.

For example, Subversion shouldn't have to re-create this logic either.
 So, I'd rather keep find_apr.m4 in apr - especially given all of the
other source dependencies in buildconf.  My itch-scratching doesn't
move me to want to replace buildconf at this time.  -- justin

Re: RFC: drop support for OpenSSL < 1.0 in trunk/2.3?

[Justin Erenkrantz]

On Tue, May 25, 2010 at 6:14 AM, Jeff Trawick  wrote:
> There's no ready answer to this, but I wonder:  How much of the
> current conditional logic is required to support the OpenSSL in
>
> fully patched RHEL 4
> fully patched Solaris 10
> (some other typical server platform that bundles OpenSSL)

Please add Mac OS X to this list.

Making mod_ssl in trunk not work on Mac OS X is very yucky, IMO.  -- justin

[PATCH] Support compiling httpd against APR/apr-util 1.x when buildconf against 2.x/trunk

[Justin Erenkrantz]

I would like to go commit the following to apr and httpd trunk.

I don't think it is appropriate for the local buildconf-time
environment to make it impossible at configure-time to use an older
APR configuration.  Restoring find_apu.m4 to apr trunk resolves this
issue for me.  (I'm leaving the ifdef in httpd configure.in to not
force everyone to update their APR/httpd trunks in unison; but I can
be convinced to remove the ifdef...)

Thoughts?  -- justin

(for APR)
svn cp 
https://svn.apache.org/repos/asf/apr/apr-util/branches/1.5.x/build/find_apu.m4
https://svn.apache.org/repos/asf/apr/apr/trunk/build/find_apu.m4

(for httpd)
Index: configure.in
===
--- configure.in(revision 949455)
+++ configure.in(working copy)
@@ -120,8 +120,6 @@
 elif test "x$with_included_apr" = "xyes"; then
apu_found=reconfig
 else
-   dnl If httpd is buildconf'ed against an apr 2.x tree, then 1.x
-   dnl isn't supported.
ifdef([APR_FIND_APU], [
  APR_FIND_APU("$srcdir/srclib/apr-util", "./srclib/apr-util",
   1, ${apr_major_version})
Index: buildconf
===
--- buildconf   (revision 949455)
+++ buildconf   (working copy)
@@ -170,6 +170,8 @@
$apr_src_dir/build/find_apr.m4 build
 if [ $apr_major_version -lt 2 ] ; then
 cp $apu_src_dir/build/find_apu.m4 build
+else
+cp $apr_src_dir/build/find_apu.m4 build
 fi

 # Remove any libtool files so one can switch between libtool 1.3

Re: svn commit: r923712 - in /httpd/httpd/trunk/docs/manual: ./ mod/

[Justin Erenkrantz]

On Wed, Mar 17, 2010 at 12:17 PM, Noirin Shirley  wrote:
> On Wed, Mar 17, 2010 at 12:54 PM, Dan Poirier  wrote:
>>
>> How about Apache Web Server?  Httpd is just the name of one of the
>> files, and not even the one people run to start it most of the time.
>> Apache HTTP Server is fine, but Apache Web Server is equally correct,
>> easier to pronounce, and less geeky :-)
>
> I also like this option.

>From the peanut gallery, eww.  =P

+1 to "Apache HTTP Server" (long name) and "httpd" (short name).

I don't see a compelling reason to rebrand now - we would only want to
do so if we wanted to do that as a major 'publicity' push which I
doubt is on our collective radar screen.

(BTW, the InConSisteNt capitalization always bugged me to no end...)  -- justin

Re: mod_dav inconsistent behaviour for GET requests

[Justin Erenkrantz]

On Sun, Jan 31, 2010 at 3:01 AM, Stefan Fritsch  wrote:
> On Sat, 30 Jan 2010, Justin Erenkrantz wrote:
>>
>> I don't see how your patch would "intentionally" break - the failure
>> mechanism is that the source scripts are served - not that a
>> configuration error stops the server from running.  -- justin
>
> Surely a fatal server error is not a necessary condition for something to be
> broken?

When it has the capability of exposing source that would not otherwise
be served, absolutely.

The fundamental flaw with this patch is that dav_fixups runs after
core_override_type - so the "none" handler simply won't get reassigned
by the rest of the applicable configs - ie set to CGI or PHP or
whatnot.  So, it would simply fall through and go to the default
handler.  Ouch.  -- justin

Re: mod_dav inconsistent behaviour for GET requests

[Justin Erenkrantz]

On Sat, Jan 30, 2010 at 2:04 AM, Stefan Fritsch  wrote:
> The recommended setup is to map separate URLs for DAV and script
> execution to the content. It has been like this since at least 2.0.
>
> The patch intentionally breaks existing configs that rely on the
> ability to use the same URLs for DAV and script execution. Is this not
> an acceptable change from 2.2 to 2.4 (if properly documented), as it
> makes life a lot easier for people who use the recommended setup?

I don't see how your patch would "intentionally" break - the failure
mechanism is that the source scripts are served - not that a
configuration error stops the server from running.  -- justin

Re: httpd meetup/hackathon, January?

[Justin Erenkrantz]

On Mon, Dec 14, 2009 at 3:52 PM, Jeff Trawick  wrote:
> On Mon, Dec 14, 2009 at 6:27 PM, Paul Querna  wrote:
>> Update: looks like we will be doing it on January Tuesday 26th at the Y! 
>> Campus.
>
> one day only?

Perhaps Monday or Wednesday those of us out-of-towners can find
somewhere else to hang if Y! can't find somethin'?

Even from SoCal, a day trip is kinda silly...so I'd probably do an
overnight.  -- justin

Re: httpd meetup/hackathon, January?

[Justin Erenkrantz]

On Wed, Dec 9, 2009 at 5:05 AM, Sander Temme  wrote:
>
> On Dec 9, 2009, at 4:53 AM, Jeff Trawick wrote:
>
>> On Tue, Dec 8, 2009 at 7:49 PM, Paul Querna  wrote:
>>> On Mon, Dec 7, 2009 at 11:50 AM, Paul Querna  wrote:
 Hi Everyone,

 Over on TraffiicServer, there is rough talk of doing some kind of
 meetup/hackathon the week of January 27th 2010, in Silicon Valley.
>>>
>>> Alternative is the week of January 13th -- would this earlier date
>>> change the ability of anyone to attend?
>>
>> no difference to me
>
> I'd prefer the later time slot.

+1.  Jan 27th is a possibility for me, but Jan 13th is not.  -- justin

Re: Events, Destruction and Locking

[Justin Erenkrantz]

On Mon, Jul 6, 2009 at 10:20 PM, Paul Querna wrote:
> I am looking for an alternative that doesn't expose all this crazyness
> of when to free, destruct, or lock things.  The best idea I can come
> up with is for each Connection, it would become 'semi-sticky' to a
> single thread.  Meaning each worker thread would have its own queue of
> upcoming events to process, and all events for connection X would sit
> on the same 'queue'.  This would prevent two threads waiting for
> destruction, and other cases of a single connection's mutex locking up
> all your works, essentially providing basic fault isolation.
>
> These queues could be mutable, and you could 'move' a connection
> between queues, but you would always take all of its events and
> triggers, and move them together to a different queue.
>
> Does the 'connection event queue' idea make sense?

I think I see where you're going with this...being so dependent upon
mutexes seems...like going into a jungle full of guerillas armed with
only a dull kitchen knife.

So, a connection gets assigned to a 'thread' - but it has only two
states: running or waiting for a network event.  The critical part is
that the thread *never* blocks on network traffic...all the 'network
event' thread does is detect "yup, ready to go" and throws it back to
that 'assigned' thread to process the event.  Seems trivial enough to
do with a serf-centric system.  =)

> I'm not sure I'm expressing the idea fully over email but I'll be
> at OSCON in a few weeks if anyone wants beer :)

I'll take you up on the beer and we can mull it over...  -- justin

Re: svn commit: r759711 - /httpd/httpd/trunk/os/unix/unixd.c

[Justin Erenkrantz]

On Mon, Mar 30, 2009 at 11:36 PM, William A. Rowe, Jr.
 wrote:
> Plüm, Rüdiger, VF-Group wrote:
>>
>> IMHO we shouldn't require APR 2.0 for trunk (although trunk should be
>> capable to run with 2.0).
>
> +1

+1.  -- justin

Re: criteria for axing MPMs from the tree

[Justin Erenkrantz]

2009/3/27 William A. Rowe, Jr. :
> abandoned, but in theory might work for 90% of the cases.  Do we work
> to preserve them, or work to recover them if we break them, or do we
> truly bother either way?

I wouldn't bother - older versions would work just fine.  But, maybe
it'd truly matter to someone.  =)  -- justin

Re: svn commit: r758597 - /httpd/httpd/trunk/server/mpm/config.m4

[Justin Erenkrantz]

On Thu, Mar 26, 2009 at 1:00 PM, Paul Querna  wrote:
> I think the default should be event for the time being.

+1.  -- justin

Re: criteria for axing MPMs from the tree

[Justin Erenkrantz]

2009/3/26 Ruediger Pluem :
> We should axe
>
> leader
> perchild
> threadpool
>
> immediately in trunk as IMHO they are not maintained for years.

+1.  -- justin

Re: mod_wombat and mod_lua

[Justin Erenkrantz]

On Wed, Feb 25, 2009 at 5:50 PM, Paul Querna  wrote:
> 23-27 March 2009, Amsterdam:
> http://www.eu.apachecon.com/c/aceu2009/
>
> I'll be there at least

Ditto.  -- justin

Re: accept mod_fcgid codebase into httpd project

[Justin Erenkrantz]

On Sun, Jan 11, 2009 at 7:53 PM, William A. Rowe, Jr.
 wrote:
>  [ ]  Accept mod_fcgid into httpd

+1.

Thanks.  -- justin

Re: svn commit: r729579 - in /httpd/httpd/trunk: CHANGES modules/generators/mod_cgid.c

[Justin Erenkrantz]

On Tue, Jan 6, 2009 at 7:11 AM, Jeff Trawick  wrote:
> AFAIK it is resolved by 120664-01 or later (or x86-equiv patch), from
> several years ago...

Yah, we already have configure checks for this.  I don't like adding
unnecessary complexity to work around an OS bug that we've detected
and flagged for years.  -- justin

Re: svn commit: r730984 - /httpd/httpd/trunk/configure.in

[Justin Erenkrantz]

On Sat, Jan 3, 2009 at 5:37 AM,   wrote:
> Author: rjung
> Date: Sat Jan  3 05:37:05 2009
> New Revision: 730984
>
> URL: http://svn.apache.org/viewvc?rev=730984&view=rev
> Log:
> Fix correct linking against PCRE.
> r730882 broke builds on MacOSX.

Works fine now on Mac OS X w/jlibtool.  Thanks.  -- justin

RE: svn commit: r730882 - /httpd/httpd/trunk/configure.in

[Justin Erenkrantz]

[ It's odd as I didn't get the email for this commit...anyway... ]

> Author: rjung
> Date: Fri Jan  2 17:01:56 2009
> New Revision: 730882
>
> URL: http://svn.apache.org/viewvc?rev=730882&view=rev
> Log:
> Only link libhttpd against pcre.
>
> Modified:
>httpd/httpd/trunk/configure.in
>
> Modified: httpd/httpd/trunk/configure.in
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/configure.in?rev=730882&r1=730881&r2=730882&view=diff
> ==
> --- httpd/httpd/trunk/configure.in (original)
> +++ httpd/httpd/trunk/configure.in Fri Jan  2 17:01:56 2009
> @@ -182,7 +182,8 @@
>   fi
>   AC_MSG_NOTICE([Using external PCRE library from $PCRE_CONFIG])
>   APR_ADDTO(CFLAGS, [`$PCRE_CONFIG --cflags`])
> -  APR_ADDTO(LIBS, [`$PCRE_CONFIG --libs`])
> +  APR_ADDTO(LTLIBRARY_LIBADD, [`$PCRE_CONFIG --libs`])
> +  APACHE_SUBST(LTLIBRARY_LIBADD)
> else
>   AC_MSG_ERROR([pcre-config for libpcre not found. PCRE is required and 
> available from http://pcre.org/])
> fi

This change busts my build because it tries to link a dynamic library
(pcre) against a static library (libmain).  You can't do that.
(jlibtool barfs on this.)  It is only legal to link static libraries
against other static libraries.  Please revert.  -- justin

Re: [VOTE] Release Apache HTTP server 2.3.1-alpha

[Justin Erenkrantz]

On Fri, Jan 2, 2009 at 10:45 PM, Paul Querna  wrote:
>  [ -1 ]  Release httpd-2.3.1 as Alpha

For me, r730882 busts the build on Mac OS X.  (It can't find pcre.)

More in my reply to the commit msg...  -- justin

Re: Configuration change for c...@httpd?

[Justin Erenkrantz]

On Fri, Jan 2, 2009 at 5:08 PM, Roy T. Fielding  wrote:
> I am completely uninterested in "fixing" the config just because some
> person reflexively does a reply-all and then doesn't edit their own
> destination addresses.  There is nothing to fix here.  A bounce is what
> they are supposed to receive to keep the discussion on dev.

It is not only one person who is running up against this.  Instead of
unilaterally stating that there is nothing to fix, I asked first to
get a consensus of folks on-list - I never asked Joe to activate any
changes (as I didn't think we had a consensus yet) - he did so on his
own volition.

I believe that asking folks to change their sending address, or live
with bounces, or remembering to remove addresses every time is busy
work for no real purpose.  It creates an additional barrier to
commenting and should be avoided.  We have software that should
conform to what we desire - we shouldn't need to work around broken
configs.

BTW, if you maintain that the list config is immutable, then my
suggestion would be to shut down cvs@ and route commits to d...@.  This
way we don't place unnecessary barriers to discussions around commits.
 If folks can't live with the commit traffic, they shouldn't be on
dev@ in the first place...  -- justin

Re: Configuration change for c...@httpd?

[Justin Erenkrantz]

On Fri, Jan 2, 2009 at 11:30 AM, Mads Toftum  wrote:
> seems to me that this will be almost as broken as the use of
> undisclosed. you're still removing what was used to filter on. Existing
> filters are still broken.

No, correct filters should be keying off List-Post/List-Id.

File a bug with whomever makes your mail client then.  It's worked
fine for ages.

*grin*  -- justin

Re: Configuration change for c...@httpd?

[Justin Erenkrantz]

On Fri, Jan 2, 2009 at 11:26 AM, Mads Toftum  wrote:
> On Fri, Jan 02, 2009 at 10:52:49AM -0800, Justin Erenkrantz wrote:
>> I don't want crap every time I hit reply either.  Remembering every
>> time to cut the address or living with the blowback is even more
>> unacceptable.  -- justin
>
> File a bug with whomever makes your mail client then. It's worked fine
> for ages.

Considering mutt, Gmail, and Thunderbird all include c...@httpd in the
replies when you do 'reply-all', I have no clue what you're talking
about.  Is there any MUA that doesn't include c...@httpd in the CC line
when you hit 'reply-all'?  -- justin

Re: Configuration change for c...@httpd?

[Justin Erenkrantz]

On Fri, Jan 2, 2009 at 10:54 AM, Joe Schaefer  wrote:
> nob...@apache.org routes to the bit-bucket, so that would work for the To:
> header.  That's what I would recommend doing at this point, although I've
> just implemented Justin's recommended config and will stop there until
> the list comes to a decision.

nobody@ sounds fine as long as it doesn't blow up too many people's filters.

My filters are all based on List-Id/List-Post, so that should be
okay..@svn just went through that whole debacle a few months ago
as CollabNet did a migration that removed List-Id/List-Post from all
of their list headers...*grin*  -- justin

Re: Configuration change for c...@httpd?

[Justin Erenkrantz]

On Fri, Jan 2, 2009 at 10:48 AM, William A. Rowe, Jr.
 wrote:
> I for one don't want the duplicate messages; reply-to says dev@ and that is
> where patches for discussion should be discussed.  Re: Crap doesn't belong
> on a commits notification list.

I don't want crap every time I hit reply either.  Remembering every
time to cut the address or living with the blowback is even more
unacceptable.  -- justin

Re: Configuration change for c...@httpd?

[Justin Erenkrantz]

On Fri, Jan 2, 2009 at 10:33 AM, Joe Schaefer  wrote:
> Andre and Roy are the only ones signed up for moderation of c...@httpd.
> I think the people who want it to go back to being a moderated list
> should first step up and offer to moderate it before we revert the
> config.

Well, it doesn't have to be moderated to have an -allow list.  You can
get ezmlm to accept the addresses w/o having human moderators - we
already have the custom accept for @apache.org senders...  -- justin

Re: Configuration change for c...@httpd?

[Justin Erenkrantz]

On Fri, Jan 2, 2009 at 9:55 AM, Joe Schaefer  wrote:
> Shrug, I can try completely removing the To: header, but I'm fairly
> certain that some MTA's will add one back (and a missing To: header
> will trigger anti-spam).  Other than that, it's pick your poison
> time, since no universal solution seems at hand.

To: d...@httpd would likely be mis-leading...how about re-enabling
-allow/-subscribe lists so that folks who are subscribed don't get the
bounce?  Or?  -- justin

Re: Configuration change for c...@httpd?

[Justin Erenkrantz]

On Fri, Jan 2, 2009 at 9:38 AM, Joe Schaefer  wrote:
> Nice.  How do people actually handle moderation at the ASF then?
> You're *supposed* to be using Reply-All when you want to accept
> a post.  Are none of these clients out there ezmlm-compatible?

Gmail does the same thing in my last set of moderation emails (the
last list I moderated got deleted last month).  Reply-all yields
-accept, not found, -allow.

I don't *recall* that being the behavior before with Gmail, but it
seems that's what it does now...  -- justin

Re: Configuration change for c...@httpd?

[Justin Erenkrantz]

On Fri, Jan 2, 2009 at 8:58 AM, Joe Schaefer  wrote:
> So, is that better or worse than getting a bounce message?  I'll point out
> that Yahoo! used to have a similar problem, but have since resolved it.  I
> wonder what gmail does now?

When you hit 'reply-all' now, GMail adds in:

list not shown 

and then gives an error complaining about the address when you hit
'send'.  -- justin

Re: Configuration change for c...@httpd?

[Justin Erenkrantz]

On Wed, Dec 31, 2008 at 7:09 AM, Ruediger Pluem  wrote:
> No. +1 to go ahead with this.

As the person who generally replies the most to commit messages, I was
sort of wondering what you've been doing as I noticed that your
replies all go to dev@ and don't have cvs@ in the 'to' line.  *grin*
-- justin

Configuration change for c...@httpd?

[Justin Erenkrantz]

A while back, we made a configuration change to all of our commits@
(aka cvs@) lists across the foundation so that they are unmoderated.
As a side-effect, it means that all posts require @apache.org
addresses or you get a bounce back from MAILER-DAEMON.

The side-effect of this (for at least Thunderbird and GMail) is that
you can no longer hit 'reply all' to our commit emails without
automatically also snarfing in c...@httpd as a recipient.  So, if you
hit 'reply all' and send from a non @apache.org address, you will get
a "nice" bounce message each time you reply to the commit message
unless you manually remove the recipient.  From looking at the
archives, I know that myself, Paul, Philip, and likely Jim have all
hit this blowback to varying degrees over the last month.  In light of
this, Joe has suggested adding:

To: undisclosed-recipients:;

to headeradd for c...@httpd, overriding the normal To:
c...@httpd.apache.org.  That should prevent Reply-All from picking up
the commit list, since there won't be anything in the To: or CC:
fields for it to use.

This should restore the ability to hit 'reply all' with Thunderbird
and GMail for those of us who don't use @apache.org from addresses.

Are there any objections to doing so?

Thanks and have a healthy and happy new year with all of your friends
and family.  -- justin

Re: svn commit: r729641 - /httpd/mod_mbox/trunk/scripts/site-sitemap.py

[Justin Erenkrantz]

On Sat, Dec 27, 2008 at 8:51 AM,   wrote:
> Author: pquerna
> Date: Sat Dec 27 08:51:31 2008
> New Revision: 729641
>
> URL: http://svn.apache.org/viewvc?rev=729641&view=rev
> Log:
> Change the sitemap index generator to split the sitemap indexes every 500 
> entries, as the great GOOG doesn't like big sitemap indexes.

FYI, you should be able to support 1,000 entries as long as you don't
hit the size limitations first.  -- justin

Re: changing mod_wombat's name

[Justin Erenkrantz]

On Mon, Dec 8, 2008 at 5:04 PM, Roy T. Fielding <[EMAIL PROTECTED]> wrote:
> mod_luau  

+1.  -- justin

Re: [VOTE] Release Apache HTTP server 2.3.0-alpha

[Justin Erenkrantz]

On Mon, Dec 8, 2008 at 12:20 PM, Sander Temme <[EMAIL PROTECTED]> wrote:
>
> On Dec 7, 2008, at 6:33 PM, Henning Schmiedehausen wrote:
>
>> Our production sites IMHO shouldn't be guinea pigs for -alpha versions.
>
> Best. Place. Ever. to try out our stuff to find and fix bugs.
>
> Especially nasty edge cases like this one.
>
> www.apache.org should run dogfood whenever possible.  How else can we
> justify asking the general public to try our stuff out?

++1.  -- justin

Re: [wombat] supporting functionless blocks

[Justin Erenkrantz]

On Sat, Nov 8, 2008 at 2:16 AM, Paul Querna <[EMAIL PROTECTED]> wrote:
> I think it can be done in wombat_request_rec_hook_harness, if
> hook_spec->function_name is NULL, then assume that we need to set these
> globals -- and then apw_run_wombat_request should be able to continue as
> normal.
>
> Thoughts?

Seems reasonable to me.  -- justin

Re: svn commit: r708462 - in /httpd/httpd/trunk/server/mpm: ./ simple/

[Justin Erenkrantz]

On Mon, Nov 3, 2008 at 11:23 AM, Jim Jagielski <[EMAIL PROTECTED]> wrote:
> You miss my point. If we just have one MPM, and make it
> such that other MPMs are at a significant advantage, then
> why even have MPMs? Hardcode in simple and be done.

(Assume you may have meant 'disadvantage'?)

> In any case, we can pretty much do that with Worker now,
> as well as Simple...

I think MPMs are a design mis-feature.  The fact that the same
directive value can be (and is!) interpreted in non-uniform ways by
different MPMs bugs me to no end.  -- justin

Re: svn commit: r708462 - in /httpd/httpd/trunk/server/mpm: ./ simple/

[Justin Erenkrantz]

On Thu, Oct 30, 2008 at 6:06 AM, Jim Jagielski <[EMAIL PROTECTED]> wrote:
> I tend to agree... as long as the architectural design doesn't
> prevent people from creating (or keeping) MPMs, then this is all
> good. Believe it or not, as people on the front-lines of handling
> Apache setup and support for organizations can attest to, MPMs are a
> valued feature.

I disagree.  It's incredibly confusing to have compile-time
definitions and inability to adjust them at run-time.

I'd prefer that we remove all MPMs and teach whatever remains to be
able to emulate prefork (multiple-process/single-thread) or worker
(multiple-process/multiple-thread) at run-time.

My $.02.  -- justin

Re: svn commit: r708902 - /httpd/httpd/trunk/modules/metadata/mod_expires.c

[Justin Erenkrantz]

On Sat, Nov 1, 2008 at 4:54 AM, Ruediger Pluem <[EMAIL PROTECTED]> wrote:
> But IMHO the code is now correct after the patch and the tests are wrong. The 
> following
> patch makes the tests pass again:

+1.  -- justin

Re: [flood] Critical section for flood_report_relative_times.c

[Justin Erenkrantz]

On Sat, Sep 20, 2008 at 11:40 PM, Ohad Lutzky <[EMAIL PROTECTED]> wrote:
>> *SNIP* patch *SNIP*
>
> The patch seems to work well, thanks! :)

Cool - committed in r697920.  Thanks.  -- justin

Re: svn commit: r696062 - /httpd/httpd/branches/2.2.x/STATUS

[Justin Erenkrantz]

On Tue, Sep 16, 2008 at 2:44 PM, Ruediger Pluem <[EMAIL PROTECTED]> wrote:
> I think we only vote on code and not on docco.
> So IMHO you could backport without votes.

Ya, but since I only made the commit to trunk this morning, I figured
I'd let it sit.  If Jim wants to backport, he can.  -- justin

Re: svn commit: r696047 - in /httpd/flood/trunk: flood_subst_file.c flood_subst_file.h

[Justin Erenkrantz]

On Tue, Sep 16, 2008 at 2:36 PM, William A. Rowe, Jr.
<[EMAIL PROTECTED]> wrote:
> I suppose the proper answer then is
>
> /* //# why mess around, therefore static */
>
> ? ;-)  Justin, I think you could have dropped the #

Hey, bub, you're the one who committed this code - not me.  =)  -- justin

Re: svn commit: r696047 - in /httpd/flood/trunk: flood_subst_file.c flood_subst_file.h

[Justin Erenkrantz]

On Tue, Sep 16, 2008 at 2:19 PM, Ruediger Pluem <[EMAIL PROTECTED]> wrote:
>> +  //# why mess around, therefore static
>
> C++ style comments are not liked by all C compilers.

True, but if you look at the hardcoded paths a few lines up, well, ya,
there's a lot of work that would need to be done to make this stub
code even run.

Anyway, I removed the comment in r696077.  =)  -- justin

Re: [PATCH] Dynamic locking upcalls in mod_ssl

[Justin Erenkrantz]

On Thu, Aug 14, 2008 at 10:04 PM, Sander Temme <[EMAIL PROTECTED]> wrote:
> Folks,
>
> The following patch against trunk adds dynamic locking callbacks to mod_ssl.
>  OpenSSL uses these in several places, including the CHIL engine that
> interfaces with the nCipher products.  I work at nCipher, and this patch
> makes the CHIL engine load into a stock, unpatched openssl 0.9.8.
>
> I've tested this on Linux (Ubuntu 7.10 w/ OpensSSL 0.9.8e-5ubuntu3.2) and
> put some load on this on Solaris 10 x86_64 with OpenSSL 0.9.8h.  It's not a
> lot of code, and I don't think it gets in anyone's way.  Opinions
> appreciated, especially on whether I'm doing the right thing with that pool:

Going into the wayback machine a little bit, but it's sad how often we
need to keep putting this code everywhere.  =)

http://svn.apache.org/repos/asf/httpd/flood/trunk/flood_net_ssl.c
http://code.google.com/p/serf/source/browse/trunk/buckets/ssl_buckets.c

I had serious flashbacks when I reviewed this patch.  *grin*  -- justin

Re: STATUS file, 2.2.10 and voting

[Justin Erenkrantz]

On Sun, Sep 14, 2008 at 7:02 AM, Ruediger Pluem <[EMAIL PROTECTED]> wrote:
> Given the fact that Jim plans to T&R 2.2.10 mid-, late September
> I triaged through the STATUS file. There are 10 proposal left
> where 6 of them only miss one vote. From the remaining 4 with only
> one vote two are still missing the vote of the proposer.
> So c'mon guys it is voting time :-).

Hey, look ma!, I voted.  =P  -- justin

Re: Patch for INSTALL

[Justin Erenkrantz]

On Tue, Sep 16, 2008 at 11:41 AM, Tim Bray <[EMAIL PROTECTED]> wrote:
> Just a piece of advice that, had I'd received it, would have saved me a
> couple days of yak shaving.  -T

Doh.  =)

Committed in r696006.  Thanks.  -- justin

Re: [flood] Critical section for flood_report_relative_times.c

[Justin Erenkrantz]

On Wed, Sep 3, 2008 at 2:28 AM, Ohad Lutzky <[EMAIL PROTECTED]> wrote:
> From flood_report_relative_times.c (SVN):
>
>/* FIXME: this call may need to be in a critical section */
> #if APR_HAS_THREADS
>apr_file_printf(local_stdout, "%s %ld %s\n", buf,
> apr_os_thread_current(), req->uri);
> #else
>apr_file_printf(local_stdout, "%s %d %s\n", buf, getpid(), req->uri);
> #endif
>
> The comment is right - it does need to be in a critical section. For
> the case of multiple processes, flock can be used. However, for the
> case of a single process, this doesn't work, and a mutex would be in
> order. What would be the correct way to go about organizing such a
> per-process mutex? Note that apr_file_printf indirectly calls
> apr_file_write, which is thread-safe - however, it call it multiple
> times, which can (and does) lead to interlacing in output.

Yup, I've seen that before, but I generally tend to ignore it.  =)

I believe the best way would be to add a mutex in the config_t
structure and lock that in the report function.  Rough patch below.

What do you think?  -- justin

Index: flood_farm.c
===
--- flood_farm.c(revision 695997)
+++ flood_farm.c(working copy)
@@ -231,6 +231,7 @@ apr_status_t run_farm(config_t *config, const char
 #if APR_HAS_THREADS
 farm->farmers = apr_pcalloc(pool,
 sizeof(apr_thread_t*) * (usefarmer_count + 1));
+apr_thread_mutex_create(&config->mutex, APR_THREAD_MUTEX_DEFAULT, pool);
 #else
 farm->farmers = apr_pcalloc(pool,
 sizeof(apr_proc_t*) * (usefarmer_count + 1));
Index: flood_config.h
===
--- flood_config.h  (revision 695997)
+++ flood_config.h  (working copy)
@@ -31,6 +31,9 @@
 typedef struct {
 apr_xml_doc *doc;
 apr_pool_t *pool;
+#if APR_HAS_THREADS
+apr_thread_mutex_t *mutex;
+#endif
 } config_t;

 /**
Index: flood_report_relative_times.c
===
--- flood_report_relative_times.c   (revision 695997)
+++ flood_report_relative_times.c   (working copy)
@@ -28,9 +28,18 @@
 extern apr_file_t *local_stdout;
 extern apr_file_t *local_stderr;

+struct relative_report_t {
+config_t *config;
+};
+typedef struct relative_report_t relative_report_t;
+
 apr_status_t relative_times_report_init(report_t **report, config_t *config,
   const char *profile_name, apr_pool_t *pool)
 {
+relative_report_t *rr = apr_palloc(pool, sizeof(relative_report_t));
+rr->config = config;
+
+*report = rr;
 return APR_SUCCESS;
 }

@@ -39,6 +48,7 @@ apr_status_t relative_times_process_stats(report_t
 #define FLOOD_PRINT_BUF 256
 apr_size_t buflen;
 char buf[FLOOD_PRINT_BUF];
+relative_report_t *rr = (relative_report_t*)report;

 buflen = apr_snprintf(buf, FLOOD_PRINT_BUF,
   "%" APR_INT64_T_FMT " %" APR_INT64_T_FMT
@@ -61,9 +71,10 @@ apr_status_t relative_times_process_stats(report_t
 apr_snprintf(buf+buflen, FLOOD_PRINT_BUF-buflen, " %d ", verified);
 }

-/* FIXME: this call may need to be in a critical section */
 #if APR_HAS_THREADS
+apr_thread_mutex_lock(rr->config->mutex);
 apr_file_printf(local_stdout, "%s %ld %s\n", buf,
apr_os_thread_current(), req->uri);
+apr_thread_mutex_unlock(rr->config->mutex);
 #else
 apr_file_printf(local_stdout, "%s %d %s\n", buf, getpid(), req->uri);
 #endif

Re: [flood] [patch] Compilation error fix

[Justin Erenkrantz]

On Wed, Sep 3, 2008 at 1:47 AM, Ohad Lutzky <[EMAIL PROTECTED]> wrote:
> Hello,
>
> We've been using flood for a project, and couldn't get revision 672044
> to compile in gcc-4.2.3-2ubuntu7 without the attached patch. The error
> we got was:
>
> flood_round_robin.c: In function 'round_robin_profile_init':
> flood_round_robin.c:910: error: lvalue required as left operand of assignment
> flood_round_robin.c:920: error: lvalue required as left operand of assignment

Thanks!  Committed in r695998.  -- justin

Survey on peer review processes within Apache HTTP Server

[Justin Erenkrantz]

Hi everyone,

Peter is a PhD student at University of Victoria who is looking into
how open source projects perform peer reviews.  I've met him a few
times and he's a good guy - so if you could please take the time to
fill out his survey below, it'd be most appreciated.  =)

Thanks!  -- justin

---

Greetings Apache developers,

The purpose of this study is to understand how peer reviews are
conducted in Free and Open Source Software projects.

We are writing to you as a team of researchers from the department of
Computer Science at the University of Victoria, BC, Canada. We are
looking for Apache http developers who have in the past or currently
conduct regular peer reviews on this project.

Should you wish to consider participation, please go to

http://helium.cs.uvic.ca/phpESP/public/survey.php?name=peer_review_apache_httpd

You may participate in a survey which takes between 10 to 25 minutes
and/or an interview which takes up to 1 hour.

As the principal investigator, I would be happy to talk with you in
order to answer any questions you might have. My contact details are
below.

Sincerely,

Peter Rigby
Email: [EMAIL PROTECTED]
www: http://www.cs.uvic.ca/~pcr

Research Team:
Peggy Storey ([EMAIL PROTECTED]), Daniel German ([EMAIL PROTECTED]), Laura
Cowen, Ahmed Hassan

P.S. If you are interested in our quantitative findings regarding the
Apache peer review processes, I will be happy to send you a copy of
our paper that appeared in the 30th International Conference on
Software Engineering, "Open Source Software Peer Review Practices: A
Case Study of the Apache Server".

Re: [PROPOSAL] merge mod_limitipconn into httpd

[Justin Erenkrantz]

On Fri, Jun 20, 2008 at 3:14 AM, Nick Kew <[EMAIL PROTECTED]> wrote:
> If David is happy to subscribe to this list and make it his
> primary forum for the module, then +1.

+1.

Embrace and extend.  =)  -- justin

Re: AuthzMergeRules directive

[Justin Erenkrantz]

On Wed, Apr 16, 2008 at 9:31 PM, Brad Nicholes <[EMAIL PROTECTED]> wrote:
>  I could go along with switching the default merging rule from OR to AND, 
> even within a dir block.  The reason why it is OR today was basically for 
> backward compatibility.  Since there really wasn't any kind of logic before, 
> OR was just the default.  If we switch to AND as being the default within a 
> dir block, it may break some existing configurations.  However I also think 
> that AND is a safer merging rule going forward.

+1.  AND makes more sense as the default - if they want OR, they can
use a directive...  -- justin

Re: [PROPOSAL] Time Based Releases

[Justin Erenkrantz]

On Sat, Apr 12, 2008 at 9:52 PM, Ruediger Pluem <[EMAIL PROTECTED]> wrote:
> > My proposal is for every 2 months, we do a release of the main stable
> branch, which at this time is 2.2.x.
> >
>
>  I would like to go for 3 month, so four times per year or once each
> quarter.

I think it's a good idea - except I think 2 months is a little too
frequent as well.  That means 1 week out of every 8 that we're
spending checking and testing the release.  1 out of 12 weeks eases it
a bit...so +1 for every three months.  -- justin

Re: Ship 1.3.0 apr in httpd 2.2.9

[Justin Erenkrantz]

On Fri, Apr 11, 2008 at 4:39 PM, William A. Rowe, Jr.
<[EMAIL PROTECTED]> wrote:
>  So if APR ships 1.3.0 - would we be prepared to accept 1.3.0 as the new
>  minimum version, modulo the minor MMN bump?

+1.  -- justin

Re: 2.4 (Was: Re: Configuration Issues to Address [was Re: Dynamic configuration for the hackathon?])

[Justin Erenkrantz]

On Thu, Apr 3, 2008 at 8:53 AM, Akins, Brian <[EMAIL PROTECTED]> wrote:
>  Very rough draft.  But this is not necessarily slow... ;)

Right.

Even then, the user/admin may be willing to burn CPU cycles anyway to
get a simpler config.  Plus, if they were to use mod_rewrite, they've
already blown a huge chunk of CPU cycles!  =)  -- justin

Re: Dynamic configuration for the hackathon?

[Justin Erenkrantz]

On Tue, Apr 1, 2008 at 6:20 AM, Jim Jagielski <[EMAIL PROTECTED]> wrote:
> IMO, we work best when we feel empowered to scratch our itches...
>  As we've also seen, sometimes all it takes is someone to create
>  a rough framework of an implementation for people to get excited
>  by it and jump right on in, adding, extending and tuning it.
>
>  This reminds me: a serf BOF or session would, I think, go over
>  quite well :)

Lieven and I are going to be focusing on cutting a serf release during
the Hackathon.  I promised that we'd get a new serf release out before
Subversion 1.5 final comes out.  =P

If folks are interested in a serf BOF or 'beer session' at the bar,
I'm sure we could swing something.  =)  -- justin

Re: Dynamic configuration for the hackathon?

[Justin Erenkrantz]

On Mon, Mar 31, 2008 at 7:41 PM, William A. Rowe, Jr.
<[EMAIL PROTECTED]> wrote:
>  I sympathize, but this doesn't reflect the addition of  blocks...
>  those blocks can be trivially implemented as a loadable module ;-)

As I grok it, the point would be throw out our ridiculous config
syntax entirely (or at best write a compatibility module or a
converter to the new format) and expose a real config API (hello
providers! *chuckle*) and then build a pure LUA-based config format on
top of that.

I'd be definitely curious to see what would come of that - 'cuz really
it can't be much worse than the garbage we're stuck with now.  --
justin

Re: Dynamic configuration for the hackathon?

[Justin Erenkrantz]

On Mon, Mar 31, 2008 at 11:15 AM, Roy T. Fielding <[EMAIL PROTECTED]> wrote:
>  Unfortunately, after last year's experience of being the only server
>  person around who wasn't working on a Joost release,

*hides*

> I decided to delay
>  my arrival until Tuesday rather than attend the hackathon.  Please have
>  fun, finish the server, and let me know what happened when I arrive;

For other reasons, I too won't arrive until Tuesday morning.  I look
forward to seeing what happens on Monday.  =P  -- justin

Re: RFC: extracting the mod_ssl session cache interface

[Justin Erenkrantz]

On Mon, Feb 25, 2008 at 2:12 PM, Joe Orton <[EMAIL PROTECTED]> wrote:
> The session cache interface in mod_ssl on the trunk is now mostly SSL-
>  and mod_ssl-agnostic with respect to the data storage and configuration.
>  There is still some tight coupling between the session cache and the
>  ssl_mutex interface, but that's next on my hit list.
>
>  Is there any interest in seeing this extracted from mod_ssl and made
>  available for general use?  It could probably e.g. be used by
>  mod_auth_digest for the MD5-sess code, and I can think of some
>  third-party modules which could probably use it too (mod_gnutls).
>
>  My vague plan would be to finish de-SSL-ifying the code, then moving it
>  to modules/cache and calling it mod_sesscache or mod_socache ("small
>  object") or something along those lines.

"mod_small_cache"?

>  Thoughts?

In general, it sounds like a cool plan.  -- justin

Re: ping on mod_dns

[Justin Erenkrantz]

On Feb 18, 2008 2:49 PM, Jorge Schrauwen <[EMAIL PROTECTED]> wrote:
> true but like Roy said if mod_dns is allready in use a new name will
> need to be found no?

The mod_dns listed on Freshmeat looks like it was lasted changed in
2001 and was only for 1.3.  We could drop a line to the guy and see if
he minds us using the name for our module.  If he replies that's it's
okay, I'd like to see us use mod_dns.  But, if he doesn't reply or
says no, then we should find another name...  -- justin

Re: ping for http in mod_proxy

[Justin Erenkrantz]

On Fri, Feb 15, 2008 at 8:26 AM, Jim Jagielski <[EMAIL PROTECTED]> wrote:
>  Not quite... there's nothing in providers, per se, that limit
>  how they are *used* to be oneshot... Just ask Brad and aaa :)

+1.

I personally find the vtable interface easier to deal with - you have
a 'set' of functions that all go together in one structure - whereas
for a hook, everything must be individual and one-off.  *shrug*  --
justin

Re: PATCH -- make the cache modules part of 'all'.

[Justin Erenkrantz]

On Feb 9, 2008 7:37 PM, Dirk-Willem van Gulik <[EMAIL PROTECTED]> wrote:
> Unless you swap in some third party (memcached, distcache, commercial
> netapp) module.

But, if we want a 'good' out-of-the-box experience, then it sounds
like we should enable mod_disk_cache too.  Similarly, it wouldn't make
sense to have mod_proxy enabled if we don't enable http or ajp
providers by default either.  =)  -- justin

Re: store_headers in memcache and diskcache

[Justin Erenkrantz]

On Feb 9, 2008 6:00 PM, Dirk-Willem van Gulik <[EMAIL PROTECTED]> wrote:
> Whichever is easiest. Happy to plop it in there at google. As it has a
> while to go before we should/could offer this for inclusion at httpd/
> apache.

Well, it could be fleshed out in trunk - it won't be backported to
2.2.x (at least not soon), but there's no reason to keep it out of our
tree - especially if you keep talking about it on [EMAIL PROTECTED]  =)  --
justin

Re: store_headers in memcache and diskcache

[Justin Erenkrantz]

On Feb 9, 2008 5:36 AM, josh rotenberg <[EMAIL PROTECTED]> wrote:
> If you want to collaborate on the google project let me know and I can
> add you, or you can take over, or however.

Or, better yet, how about adding mod_memcache_cache (gah - crappy
name) to trunk?  There's no real reason why we shouldn't have it if
there are interested developers to work and maintain it.  -- justin

Re: Is async the answer

[Justin Erenkrantz]

On Jan 18, 2008 2:30 PM, Ruediger Pluem <[EMAIL PROTECTED]> wrote:
> IMHO it doesn't for the first request of the entity (the request that causes
> the entity to be cached)

I'd expect the predominance of large numbers would reduce the impact
of the one-time performance hit...but that conversion away from a fd
and into flat buffers, I feel, is more of a bucket brigade problem
than an intrinsic fault of mod_disk_cache.  -- justin

Re: Is async the answer

[Justin Erenkrantz]

On Jan 18, 2008 10:52 AM, Akins, Brian <[EMAIL PROTECTED]> wrote:
> Which is why I hate to see a ton of work go into async core if it actually
> does very little to help performance (or if it hurts it) and makes writing
> modules harder.  It braindead simple nowadays to write well behaved high
> performance modules (well, mostly) bcs you rarely worry about threads,
> reads/writes, etc.  Full async programming is just as challenging as
> handling a ton of threads yourself.

Speaking for myself, I think writing and using buckets with serf is
more straightforward than our complicated bucket brigade system with
mixed push/pull paradigms.

YMMV.  -- justin

Re: Pre-release test tarballs of httpd 1.3.40, 2.0.62 and 2.2.7 available

[Justin Erenkrantz]

On Jan 7, 2008 2:34 AM, Joe Orton <[EMAIL PROTECTED]> wrote:
> I've added a test case to t/modules/proxy.t in perl-framework which
> exercises the non-blockingness of the chunked encoding parser a little
> by dripfeeding a chunked response at it, byte by byte, it passes with
> the trunk and fails with 2.2.x FWIW.

Thanks!  -- justin

Re: Test suite on Darwin was Re: Pre-release test tarballs of httpd 1.3.40, 2.0.62 and 2.2.7 available

[Justin Erenkrantz]

On Jan 6, 2008 11:41 PM, Sander Temme <[EMAIL PROTECTED]> wrote:
> I actually built my own perl 5.10.0, put its bin in front of my PATH
> and installed all the requisite modules. I did this mainly because I
> don't want to mess with the system installation, but prefer to hose
> one I can easily replace. (:

That's what I thought.

> Perl  5.10.0
> Crypt::SSLeay 0.57
> LWP   5.808
> MIME::Base64  3.07_01
> Digest::MD5   2.36_01
>
> > I'm kind of concerned if we diverge on Intel vs. PowerPC...  -- justin
>
> Your failure pattern actually smacks of broken Crypt::SSLeay,
> Digest::MD5 or even MIME::Base64 fu: try forcing those modules to
> (re)install.

For the 2.2.8 go-'round, I'll try again.  But, at least I'll have
'known good versions' this time.  Thanks.  -- justin

Test suite on Darwin was Re: Pre-release test tarballs of httpd 1.3.40, 2.0.62 and 2.2.7 available

[Justin Erenkrantz]

On Jan 6, 2008 10:13 AM, Sander Temme <[EMAIL PROTECTED]> wrote:
> Mac OS X 10.5 (Leopard) on PowerPC:
>
> [-1] 1.3.40 (CVE-2007-6388 not fixed)
> [+1] 2.0.62 (with SSL toolkit detection patch, and sendfile disabled)
> [+1] 2.2.7
> Darwin Graymalkin.local 9.1.0 Darwin Kernel Version 9.1.0: Wed Oct 31
> 17:48:21 PDT 2007; root:xnu-1228.0.2~1/RELEASE_PPC Power Macintosh
>
> 2.2.7 Prefork:
>
> Failed Test Stat Wstat Total Fail  List of Failed
> ---
> t/php/dirname.t11  1
> t/ssl/v2.t 11  1
> 12 tests and 2 subtests skipped.
> Failed 2/138 test scripts. 2/3007 subtests failed.
> Files=138, Tests=3007, 700 wallclock secs (132.38 cusr + 30.72 csys =
> 163.10 CPU)
> Failed 2/138 test programs. 2/3007 subtests failed.
>
> No regressions.

Interesting in that this differs from my tests on 10.5.1/Intel (see my
earlier post).  What perl / LWP were you using?

I'm kind of concerned if we diverge on Intel vs. PowerPC...  -- justin

Re: Pre-release test tarballs of httpd 1.3.40, 2.0.62 and 2.2.7 available

[Justin Erenkrantz]

On Jan 4, 2008 12:00 PM, Jim Jagielski <[EMAIL PROTECTED]> wrote:
>  http://httpd.apache.org/dev/dist/

+1 for 2.2.7.

Tested on Mac OS 10.5.1 (aka 9.1.0) with APR/APR-util 1.2.12.

Some caveats though on Mac OS - I have a hunch they are more about the
test suite than anything else.

Configure options:

./configure --enable-modules=most --enable-ssl --enable-cache
--enable-proxy --enable-disk-cache

With httpd-test, the built-in Perl distro yields these failures:

Failed TestStat Wstat Total Fail  Failed  List of Failed
---
t/security/CVE-2005-2700.t21  50.00%  1
t/ssl/basicauth.t 32  66.67%  2-3
t/ssl/env.t  30   23  76.67%  1-8 16-30
t/ssl/extlookup.t 22 100.00%  1-2
t/ssl/fakeauth.t  32  66.67%  2-3
t/ssl/headers.t   33 100.00%  1-3
t/ssl/pr12355.t  108  80.00%  1-8
t/ssl/pr43738.t   44 100.00%  1-4
t/ssl/proxy.t   172  113  65.70%  60-172
t/ssl/require.t   52  40.00%  2 5
t/ssl/v2.t11 100.00%  1
t/ssl/varlookup.t72   72 100.00%  1-72
t/ssl/verify.t31  33.33%  2
8 tests and 18 subtests skipped.
Failed 13/80 test scripts, 83.75% okay. 234/2824 subtests failed, 91.71% okay.
[  error] error running tests (please examine t/logs/error_log)

Looking deeper, I replaced Perl with current MacPort perl install and I now get:

(The key bit seems it upgrades libwww-perl/5.805 to libwww-perl/5.808.)

Failed Test   Stat Wstat Total Fail  Failed  List of Failed
---
t/ssl/basicauth.t32  66.67%  2-3
t/ssl/env.t 30   15  50.00%  16-30
t/ssl/extlookup.t22 100.00%  1-2
t/ssl/fakeauth.t 32  66.67%  2-3
t/ssl/pr12355.t 108  80.00%  1-8
t/ssl/pr43738.t  44 100.00%  1-4
t/ssl/require.t  52  40.00%  2 5
t/ssl/varlookup.t   72   72 100.00%  1-72
t/ssl/verify.t   31  33.33%  2
8 tests and 18 subtests skipped.
Failed 9/80 test scripts, 88.75% okay. 108/2824 subtests failed, 96.18% okay.
[  error] error running tests (please examine t/logs/error_log)

Looking at the error and access logs, httpd is returning success and
failures where appropriate, but LWP is somehow dying.  So, I'll cast
my +1 even with these SSL failures as merely changing the LWP version
made some of these go away.  If someone wants to dig more and see
what's up, that'd be appreciated too...but I wouldn't block the
release on this.  -- justin

Re: mod_wombat build help

[Justin Erenkrantz]

On Dec 31, 2007 5:12 PM, Brian McCallister <[EMAIL PROTECTED]> wrote:
> If anyone is familiar with autconf and modules, I would *love* if that
> person could take a look at helping me clean up mod_wombat's build,
> which is presently a mess.
>
> I, to be honest, don't understand enough automake/autoconf + apxs to
> know what needs doing, so will just say... please help?

See below.  If you'd like, I can just commit it.  (It's not perfect,
but it'll build out-of-the-box on Leopard.)

Enjoy.  -- justin

Property changes on: .
___
Modified: svn:ignore
   - wombat.tmproj
cycle
*.loT
Makefile
config
configure
config.log
config.status
*.slo
.libs
ref

   + wombat.tmproj
cycle
*.loT
Makefile
config
configure
config.log
config.status
*.slo
.libs
ref
mod_wombat_config.h.in
mod_wombat_config.h


Index: Makefile.in
===
--- Makefile.in (revision 608009)
+++ Makefile.in (working copy)
@@ -1,26 +1,33 @@
-APXS_CFLAGS = -Wc,-Wall -Wc,-Werror
+# Makefile for mod_wombat

-COMPILE_APACHE_MOD = @APXS@ -c [EMAIL PROTECTED]@ -q INCLUDEDIR`/apreq2
[EMAIL PROTECTED]@/include/apreq2 [EMAIL PROTECTED]@/lib -lapreq2 @LUA_LIBS@
@LUA_CFLAGS@ $(APXS_CFLAGS)
[EMAIL PROTECTED]@

-INSTALL_MOD_SHARED = @APXS@ -i
+EXTRA_CFLAGS = -Wc,-Wall -Wc,-Werror

+APREQ2_DIR = @APREQ2_DIR@
+APREQ2_INCLUDES = -I$(APREQ2_DIR)/include/apreq2
+APREQ2_LIBS = -L$(APREQ2_DIR)/lib -Wl,-R$(APREQ2_DIR)/lib -lapreq2
+
+LUA_CFLAGS = @LUA_CFLAGS@
+LUA_LIBS = @LUA_LIBS@
+
+CPPFLAGS=$(APREQ2_INCLUDES) $(LUA_CFLAGS) $(EXTRA_CFLAGS)
+LIBS=$(APREQ2_LIBS) $(LUA_LIBS)
+LDFLAGS=
+
+COMPILE_APACHE_MOD = $(APXS) -c $(CPPFLAGS) $(LDFLAGS) $(LIBS)
+
+INSTALL_MOD_SHARED = $(APXS) -i
+
 all: mod_wombat.la

 install: install-mods

 clean:
-   @rm -rf *.la
-   @rm -rf *.lo
-   @rm -rf *.o
-   @rm -rf *.slo
-   @rm -rf .libs
+   @rm -rf *.la *.lo *.o *.slo .libs

 maintainer-clean: clean
-   @rm -rf Makefile
-   @rm -rf config
-   @rm -rf config.log
-   @rm -rf config.status
-   @rm -rf configure
+   @rm -rf Makefile config config.log config.status configure

 dist-clean: maintainer-clean

Index: configure.in
===
--- configure.in(revision 608009)
+++ configure.in(working copy)
@@ -1,11 +1,15 @@
+AC_PREREQ(2.50)
+
 AC_INIT
-AC_CONFIG_AUX_DIR(config)
+AC_CONFIG_HEADER(mod_wombat_config.h)
+AC_CONFIG_AUX_DIR(build)

 sinclude(build/ac-macros/lua.m4)

 AC_MSG_NOTICE(looking for Apache module support via DSO through APXS)
 AC_ARG_WITH(apxs,
-[[  --with-apxs[=FILE]  FILE is the path to apxs; defaults to "apxs".]],
+[AS_HELP_STRING([[--with-apxs=[=FILE]]],
+[FILE is the path to apxs; defaults to "apxs".])],
 [
   if test "$withval" = "yes"; then
 APXS=apxs
@@ -14,13 +18,17 @@
   fi
 ])

-if test "$BINNAME" = "" -a "$APXS" = ""; then
+if test -z "$APXS"; then
   for i in /usr/sbin \
/usr/local/apache/bin \
/usr/local/apache2/bin \
/usr/local/sbin ; do
-if test -f "$i/apxs"; then
+if test -f "$i/apxs2"; then
+  APXS="$i/apxs2"
+  break
+elif test -f "$i/apxs"; then
   APXS="$i/apxs"
+  break
 fi
   done
 fi
@@ -61,13 +69,18 @@
   fi
 ])

-if test -n "$APREQ2_DIR" -a "$APREQ2_DIR" != "no"; then
-  AC_MSG_NOTICE(found apreq2 at $APREQ2_DIR)
-else
-  AC_MSG_ERROR(couldn't find apreq2)
-fi
+save_CPPFLAGS=$CPPFLAGS
+APXS_INCLUDES="`$APXS -q EXTRA_INCLUDES`"
+CPPFLAGS="$APXS_INCLUDES -I$APREQ2_DIR/include/apreq2 $CPPFLAGS"
+AC_CHECK_HEADERS([apreq2/apreq_module_apache2.h apreq.h],
+ [],
+ [AC_MSG_ERROR([APREQ headers not found.  Try --with-apreq2])])
+CPPFLAGS=$save_CPPFLAGS

 AC_SUBST(APXS)
+
 AC_SUBST(APREQ2_DIR)

-AC_OUTPUT(Makefile)
+AC_CONFIG_FILES([Makefile])
+
+AC_OUTPUT
Index: vmprep.c
===
--- vmprep.c(revision 608009)
+++ vmprep.c(working copy)
@@ -121,8 +121,8 @@

 lua_pushinteger(L, DONE);
 lua_setfield(L, -2, "DONE");
-
-lua_pushstring(L, ap_get_server_version());
+
+lua_pushstring(L, ap_get_server_banner());
 lua_setfield(L, -2, "version");

 lua_pushinteger(L, HTTP_MOVED_TEMPORARILY);
Index: build/install.sh
===
--- build/install.sh(revision 0)
+++ build/install.sh(revision 0)
@@ -0,0 +1,112 @@
+#!/bin/sh
+##
+##  install.sh -- install a program, script or datafile
+##
+##  Based on `install-sh' from the X Consortium's X11R5 distribution
+##  as of 89/12/18 which is freely available.
+##  Cleaned up for Apache's Autoconf-style Interface (APACI)
+##  by Ralf S. Engelschall <[EMAIL PROTECTED]>
+##
+#
+# This script falls under the Apache License.
+# See http://www.apache.org/docs/LICENSE
+
+
+#
+#   put in absolute 

Re: Apache memory usage

[Justin Erenkrantz]

On Dec 10, 2007 10:42 PM, Stefan Fritsch <[EMAIL PROTECTED]> wrote:
> Yes, that works as well. I wanted to avoid calling APR_BRIGADE_PREPEND
> when the temporary brigade is empty. But I don't really know whether
> APR_BRIGADE_PREPEND is so expensive that this makes sense. An
> alternative would be to check this using APR_BRIGADE_EMPTY.

FWIW, as the brigade is a ring (read: dual-linked list), prepend or
append are both constant-time operations - on the order of maybe 2-3
instructions.

IOW, it's cheap.  -- justin

Re: Memory consumption of mod_substitute

[Justin Erenkrantz]

On Dec 5, 2007 8:36 AM, Plüm, Rüdiger, VF-Group
<[EMAIL PROTECTED]> wrote:
> * My test case lead to the exceptional situation of a very large passbb 
> bucket brigade
>   (about 1,000,000 buckets) as a result of processing 4 MB of the file. So I 
> add
>   a flush bucket once I have more than MAX_BUCKET (1000) buckets in the 
> brigade and pass it
>   down the chain to get it send and the passbb bucket brigade cleaned up and 
> its memory
>   reusable again.

Ha!  Is there a way we could be more aggressive in minimizing the
number of buckets mod_substitute creates?  Perhaps using
apr_bucket_copy to create ref-counted versions of the replacement
string?

> Comments, thoughts?

Your patch looks good on a quick cursory review.  -- justin

Re: [PATCH] Case insensitive username matching for WIN32 and BS2000 (and OS2?)

[Justin Erenkrantz]

On Dec 4, 2007 5:15 AM, Martin Kraemer <[EMAIL PROTECTED]> wrote:
> The usernames in WIN32 are, IIRC , case insensitive (and they are in
> BS2000, and perhaps in OS2?).

I don't get it - why should usernames be case-insensitive?  There's no
involvement of the Win32 (OS/2, z/OS, Netware, etc.) API here -
everything is through our htpasswd/htdigest file format.

Why should we diverge based on the server's platform?  -- justin

Re: Appologies: httpd/httpd/vendor/ SNAFU

[Justin Erenkrantz]

On Nov 26, 2007 8:46 PM, Roy T. Fielding <[EMAIL PROTECTED]> wrote:
> Okay with me.  All we need now is a volunteer to figure out what
> (if any) changes are needed to use a separately installed PCRE.

All hail Guido's time machine than has been hijacked by Joe.  =)  -- justin

% ./configure --help | grep pcre
  --with-pcre=PATHUse external PCRE library


r153400 | jorton | 2005-02-11 06:08:24 -0800 (Fri, 11 Feb 2005) | 12 lines

Support use of an external copy of the PCRE library:

* configure.in: Set abs_{builddir,srcdir} higher.  Add --with-pcre
flag; build against external PCRE library if used.

* Makefile.in (install-include): Don't install pcre headers any more.

* srclib/Makefile.in (SUBDIRS): Remove.

PR: 27550 (part two)
Submitted by: Andres Salomon , Joe Orton

Re: Appologies: httpd/httpd/vendor/ SNAFU

[Justin Erenkrantz]

On Nov 26, 2007 8:01 PM, William A. Rowe, Jr. <[EMAIL PROTECTED]> wrote:
> Justin Erenkrantz wrote:
> >
> > Once we switched our code to supporting external PCREs, in my opinion,
> > we should have just dropped the whole vendor branch concept as it
> > serves no legitimate purpose any more.  If the PCRE guys are doing
> > releases now (it seems someone is home now), then we should just get
> > our changes merged upstream and stop having private copies of it.  --
>
> To Trunk (/future release)?  ++1

Yup, we should unbundle PCRE for trunk/2.4/3.0/whatever-comes-next.

Obviously, we need to keep bundling it for 2.2 and prior; but going
forward?  Eh.  We only had a PCRE in-tree because we were diverging
from upstream and no one on the PCRE side was home for years.  So, if
someone is maintaining PCRE these days, then we don't need to and just
get our folks to download and install PCRE separately.  -- justin

Re: Appologies: httpd/httpd/vendor/ SNAFU

[Justin Erenkrantz]

On Nov 26, 2007 4:28 PM, Roy T. Fielding <[EMAIL PROTECTED]> wrote:
> Generally speaking, if someone tells you to do something in IRC
> then it is almost certainly the wrong thing to do -- just like
> decisions made in boring meetings.

Philip said he never intended to commit it.

> The right thing to do, assuming you actually want this change to
> be done at some point in the near future, is just to apologize for
> the accident and *ask* if anyone objects to the change *here*.

I did indicate sending the email to [EMAIL PROTECTED] was the priority.  *shrug*

> In any case, if you have the itch to update trunk to the latest
> version of PCRE in workable form, then by all means go for it.
> That is, assuming you have time to test it with httpd first and
> make sure that it works on your system before committing.

Once we switched our code to supporting external PCREs, in my opinion,
we should have just dropped the whole vendor branch concept as it
serves no legitimate purpose any more.  If the PCRE guys are doing
releases now (it seems someone is home now), then we should just get
our changes merged upstream and stop having private copies of it.  --
justin