You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Bojan Smojver <bo...@rexursive.com> on 2002/10/17 05:25:30 UTC

[PATCH]: Counting I/O with FLUSH

OK, this is my first "official" attempt at this. The basic idea is to replace
the EOS bucket at the end of the brigade with a FLUSH bucket, so that each
request is flushed down the pipe. I'm guessing this will completely break
pipelining if mod_logio is enabled, but, as someone mentioned, this is a
workaround, not a solution (hopefully 2.2 will have that :-)

The patch is otherwise the same as before (my first version) with one
difference: there is now an output filter in mod_logio.c. It's sole purpose is
to do the above. It runs just before the network filters.

Let me know what you think.

Bojan

Re: [PATCH]: Counting I/O with FLUSH

Posted by Bojan Smojver <bo...@rexursive.com>.
Quoting Bojan Smojver <bo...@rexursive.com>:

> I'm using Mozilla 1.0.1, which support pipelining

Damn Mozilla... It didn't actually pipeline the requests :-(

So, I switched to telnet and discovered that there is a MAJOR difference in
results with and without the FLUSH filter (which is kinda the point ;-)

Basically, without the FLUSH filter all GIF output bytes are reported as zero.
With the filter, they are all reported correctly and are identical. As the
matter of fact, the last 22 images were transferred in one packet, giving it
size of 6512 (each was 296), even with the flush enabled. And yet, logging was
correct...

I also tested again with a big file (23 MB). I have stopped the download at
various points and mod_logio logged different numbers.

I think we'll be cooking with gas :-)

Bojan

Re: [PATCH]: Counting I/O with FLUSH

Posted by Bojan Smojver <bo...@rexursive.com>.
I'm doing some testing with and without this new filter that replaces EOS with
FLUSH.

Here is the test HTML file:

---------------------------------
<html>
<img src="1.gif" />
<img src="2.gif" />
<img src="3.gif" />
...
<img src="23.gif" />
<img src="24.gif" />
<img src="25.gif" />
</html>
---------------------------------

First, the bad news - I'm using Mozilla 1.0.1, which support pipelining, but I
can't make Apache send more then one GIF per packet even when this filter is
disabled (using tcpdump to monitor the traffic). Each GIF is only 66 bytes, with
headers it turns out 183 bytes. So, it would make sense to put them all in one
packet. Unless, of course, the pipelining is not used at all when there is a
FILE bucket (I've seen something like this in core_output_filter). So, what do I
do to get this test under way? How does one "activate" pipelining in Apache 2?

Second, the good news - the payload of each packet reported by tcpdump is
identical to what mod_logio reports. Hurray!

Bojan

Re: [PATCH]: Counting I/O with FLUSH

Posted by Bojan Smojver <bo...@rexursive.com>.
Thanks Brian. I appreciate your input.

Bojan

On Tue, 2002-10-22 at 04:11, Brian Pane wrote:

> >I'll rework the patch to include the structure structure in core.c rather then
> >one in mod_logio.c. Does that sound OK?
> >
> 
> Sounds good to me
> 
> >The core would also have to know about logio_config_t, which doesn't also
> >doesn't make sense. I would rather have it the other way around.
> >
> 
> I agree--better to add a struct in the core and make mod_logio
> depend on that than to require the core to know about mod_logio.


Re: [PATCH]: Counting I/O with FLUSH

Posted by Bojan Smojver <bo...@rexursive.com>.
Quoting Brian Pane <br...@cnet.com>:

> >The core would also have to know about logio_config_t, which doesn't also
> >doesn't make sense. I would rather have it the other way around.
> >
> 
> I agree--better to add a struct in the core and make mod_logio
> depend on that than to require the core to know about mod_logio.

OOPS, I think we'll have to come up with a compromise solution here. The problem
is that the core is already stuffing its own configuration (a socket) into
c->conn_config and I doubt we can put two configuration records in it by the
same module.

Which means I have to do this:

--------------------------------------------------------------
/* Structure for passing on the socket and logging of input/output bytes per
 * connection and eventually per request. The logging stuff is used in both
 * core.c and mod_logio.c */
typedef struct core_conn_conf {
    apr_sock_t csd;
    apr_off_t  bytes_in;
    apr_off_t  bytes_out;
} core_conn_conf;
--------------------------------------------------------------

I'll fix all code in Apache that uses the socket directly after this kind of call:

--------------------------------------------------------------
ap_get_module_config(something->conn_config, &core_module);
--------------------------------------------------------------

to get the socket from the structure instead:

--------------------------------------------------------------
ccc->esd
--------------------------------------------------------------

At least two problems with this:

- does this require an MMN bump by itself
- is this likely to break something in the 3rd party module world that expects
core conn_config to be a socket

As usual, any input appreciated...

Bojan

Re: [PATCH]: Counting I/O with FLUSH

Posted by Brian Pane <br...@cnet.com>.
Bojan Smojver wrote:

>Quoting Brian Pane <br...@cnet.com>:
>
>  
>
>>As for the MMN bump, IMHO it would be better to code the
>>patch in a way that doesn't require it (if I remember correctly,
>>you had an alternate solution involving a module-specific
>>structure instead of new fields in conn_rec).  That would
>>make it easier to get the improved bytes-sent accounting
>>into the hands of users who need it, without having to wait
>>until 2.1 or 2.2.
>>    
>>
>
>Yep, there was such a structure in mod_logio. However, given that the counting
>of output is now done in core_output_filter, such a structure would have to be
>introduced in the core, rather then in the mod_logio itself (i.e. core should
>not rely on mod_logio being present during compilation, dynamic linking or
>execution).
>
>I'll rework the patch to include the structure structure in core.c rather then
>one in mod_logio.c. Does that sound OK?
>

Sounds good to me

>Alternative is to let core.c know that there is such a thing as mod_logio.c for
>the purposes of fetching the structure. In other words, something like this
>would appear in core.c:
>
>--------------------------------------------------
>logio_config_t *cf = ap_get_module_config(c->conn_config,                      
>                       
>                                          &logio_module);
>--------------------------------------------------
>
>Which means that something like this would also have to appear in core.c:
>
>--------------------------------------------------
>extern module logio_module;
>--------------------------------------------------
>
>The core would also have to know about logio_config_t, which doesn't also
>doesn't make sense. I would rather have it the other way around.
>

I agree--better to add a struct in the core and make mod_logio
depend on that than to require the core to know about mod_logio.

Brian




Re: [PATCH]: Counting I/O with FLUSH

Posted by Bojan Smojver <bo...@rexursive.com>.
Quoting Brian Pane <br...@cnet.com>:

> As for the MMN bump, IMHO it would be better to code the
> patch in a way that doesn't require it (if I remember correctly,
> you had an alternate solution involving a module-specific
> structure instead of new fields in conn_rec).  That would
> make it easier to get the improved bytes-sent accounting
> into the hands of users who need it, without having to wait
> until 2.1 or 2.2.

Yep, there was such a structure in mod_logio. However, given that the counting
of output is now done in core_output_filter, such a structure would have to be
introduced in the core, rather then in the mod_logio itself (i.e. core should
not rely on mod_logio being present during compilation, dynamic linking or
execution).

I'll rework the patch to include the structure structure in core.c rather then
one in mod_logio.c. Does that sound OK?

Alternative is to let core.c know that there is such a thing as mod_logio.c for
the purposes of fetching the structure. In other words, something like this
would appear in core.c:

--------------------------------------------------
logio_config_t *cf = ap_get_module_config(c->conn_config,                      
                       
                                          &logio_module);
--------------------------------------------------

Which means that something like this would also have to appear in core.c:

--------------------------------------------------
extern module logio_module;
--------------------------------------------------

The core would also have to know about logio_config_t, which doesn't also
doesn't make sense. I would rather have it the other way around.

Bojan

Re: [PATCH]: Counting I/O with FLUSH

Posted by Brian Pane <br...@cnet.com>.
On Sun, 2002-10-20 at 14:24, Bojan Smojver wrote:
> Sorry to be a pest (no, not really :-)...
> 
> Would this patch have more chance if it didn't involve an MMN bump? If
> so, let me know and I'll rework it...

>From your description of the patch, I'm +1 on the concept
(though I haven't had time to read the code itself).

As for the MMN bump, IMHO it would be better to code the
patch in a way that doesn't require it (if I remember correctly,
you had an alternate solution involving a module-specific
structure instead of new fields in conn_rec).  That would
make it easier to get the improved bytes-sent accounting
into the hands of users who need it, without having to wait
until 2.1 or 2.2.

Brian



Re: [PATCH]: Counting I/O with FLUSH

Posted by Bojan Smojver <bo...@rexursive.com>.
Sorry to be a pest (no, not really :-)...

Would this patch have more chance if it didn't involve an MMN bump? If
so, let me know and I'll rework it...

Bojan