You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ryan Bloom <rb...@covalent.net> on 2002/05/30 23:43:05 UTC
RE: cvs commit: httpd-2.0/modules/test mod_bucketeer.c
> From: jerenkrantz@apache.org [mailto:jerenkrantz@apache.org]
>
> jerenkrantz 02/05/30 14:10:19
>
> Modified: modules/test mod_bucketeer.c
> Log:
> mod_bucketeer needs to preserve error buckets if it sees them rather
> than silently discarding them.
This doesn't fix the underlying problem. Mod_bucketeer needs to be
smart enough to copy buckets if it doesn't recognize their type. If the
bucket can't be copied, then the filter should remove the bucket from
one brigade and add it to the other.
Any filter that silently removes buckets is broken.
Ryan
>
> Revision Changes Path
> 1.11 +8 -0 httpd-2.0/modules/test/mod_bucketeer.c
>
> Index: mod_bucketeer.c
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/modules/test/mod_bucketeer.c,v
> retrieving revision 1.10
> retrieving revision 1.11
> diff -u -r1.10 -r1.11
> --- mod_bucketeer.c 29 May 2002 06:19:26 -0000 1.10
> +++ mod_bucketeer.c 30 May 2002 21:10:19 -0000 1.11
> @@ -67,6 +67,7 @@
> #include "util_filter.h"
> #include "apr_buckets.h"
> #include "http_request.h"
> +#include "http_protocol.h"
>
> static const char bucketeerFilterName[] = "BUCKETEER";
> module AP_MODULE_DECLARE_DATA bucketeer_module;
> @@ -137,6 +138,13 @@
> * Ignore flush buckets for the moment..
> * we decide what to stream
> */
> + continue;
> + }
> +
> + if (AP_BUCKET_IS_ERROR(e)) {
> + apr_bucket *err_copy;
> + apr_bucket_copy(e, &err_copy);
> + APR_BRIGADE_INSERT_TAIL(ctx->bb, err_copy);
> continue;
> }
>
>
>
>
RE: cvs commit: httpd-2.0/modules/test mod_bucketeer.c
Posted by Ryan Bloom <rb...@covalent.net>.
> From: Justin Erenkrantz [mailto:jerenkrantz@apache.org]
>
> On Thu, May 30, 2002 at 05:50:45PM -0400, Cliff Woolley wrote:
> > On Thu, 30 May 2002, Justin Erenkrantz wrote:
> >
> > > The fact here is that not all buckets can be "read" such as EOS,
> > > flush, and error.
> >
> > Sure they can. They just contain zero data bytes.
>
> Right, but the problem is how are these "metadata" buckets identified
> if their types aren't checked for explicitly? It'd be nice if we
> came up with a way to prevent every module for checking for EOS,
> flush, and error each time. -- Justin
All of the metadata buckets have a common interface. They all have a 0
length. So, filters should be written so that any 0 length bucket is
just moved from one brigade to the next. If 0 length buckets are
removed from the brigade, then the filter is most likely doing something
wrong. The only time this isn't true, is when the bucket is a 0 length
heap/mmap/file/transient bucket, but those don't really matter, because
the lowest level filters will remove those cleanly. Since the lowest
level filters know which are the metadata buckets, they also know that
any 0 length bucket that isn't a metadata bucket can be removed.
Ryan
Re: cvs commit: httpd-2.0/modules/test mod_bucketeer.c
Posted by Justin Erenkrantz <je...@apache.org>.
On Thu, May 30, 2002 at 05:50:45PM -0400, Cliff Woolley wrote:
> On Thu, 30 May 2002, Justin Erenkrantz wrote:
>
> > The fact here is that not all buckets can be "read" such as EOS,
> > flush, and error.
>
> Sure they can. They just contain zero data bytes.
Right, but the problem is how are these "metadata" buckets identified
if their types aren't checked for explicitly? It'd be nice if we
came up with a way to prevent every module for checking for EOS,
flush, and error each time. -- justin
RE: cvs commit: httpd-2.0/modules/test mod_bucketeer.c
Posted by Cliff Woolley <jw...@virginia.edu>.
On Thu, 30 May 2002, Ryan Bloom wrote:
> Agreed, but I can think of one case. (a bit of hand-waving, but a
> possibility, nonetheless) :-) If I write two filters that work in
> tandem to solve a problem, I may want to use a special bucket type to
> facilitate them working together.
Fair enough. I was thinking of a similar situation a few minutes ago.
Though I think in practice it's likely that such a bucket type would hang
on to the buffer it copied the data into in addition to the "native"
format and would therefore meet my earlier assertion by virtue of having
the data in both formats (binary buffer and "native" format) after the
first call to apr_bucket_read(). It wouldn't *have* to, but it probably
would anyway. :)
--Cliff
RE: cvs commit: httpd-2.0/modules/test mod_bucketeer.c
Posted by Ryan Bloom <rb...@covalent.net>.
> From: Cliff Woolley [mailto:jwoolley@virginia.edu]
>
> On Thu, 30 May 2002, Ryan Bloom wrote:
>
> > I didn't think it _had_ to auto-morph. My understanding is that the
> > default buckets do, because we assume the performance will be better
if
> > they do. That makes sense, because a file_bucket is likely to be
read
> > multiple times, so it makes sense to morph the bucket on the first
read,
> > so that we don't have to go to the disk twice.
>
> If it didn't, how could you possibly pass back a buffer containing the
> data? In other words, I guess there's no hard and fast rule that it
*has*
> to actually morph, but since it's doing all the work of copying that
data
> into an in-memory buffer anyway, it doesn't make much sense *not* to
morph
> to a bucket type that handles in-memory buffers. Sure it could
allocate a
> new buffer and redo the copy every single time it was read from, but
why
> would it want to? <shrug>
Agreed, but I can think of one case. (a bit of hand-waving, but a
possibility, nonetheless) :-) If I write two filters that work in
tandem to solve a problem, I may want to use a special bucket type to
facilitate them working together. For example, if I have a bucket type
that encodes graphics in lists of RGB values, then the read function
MUST return a binary buffer of data, but later filters will most likely
find it MUCH easier to deal with the RGB values directly. So, in this
case, I could have a handler that served graphics files by filling out
this special bucket, and a filter later on that added a watermark. Any
filter in between the two will still have to be able to read from the
bucket, but the watermark filter will be able to operate much better on
the RGB values.
That is the only type of case that would make sense to keep the data in
a different structure. :-)
Ryan
RE: cvs commit: httpd-2.0/modules/test mod_bucketeer.c
Posted by Cliff Woolley <jw...@virginia.edu>.
On Thu, 30 May 2002, Ryan Bloom wrote:
> I didn't think it _had_ to auto-morph. My understanding is that the
> default buckets do, because we assume the performance will be better if
> they do. That makes sense, because a file_bucket is likely to be read
> multiple times, so it makes sense to morph the bucket on the first read,
> so that we don't have to go to the disk twice.
If it didn't, how could you possibly pass back a buffer containing the
data? In other words, I guess there's no hard and fast rule that it *has*
to actually morph, but since it's doing all the work of copying that data
into an in-memory buffer anyway, it doesn't make much sense *not* to morph
to a bucket type that handles in-memory buffers. Sure it could allocate a
new buffer and redo the copy every single time it was read from, but why
would it want to? <shrug>
--Cliff
RE: cvs commit: httpd-2.0/modules/test mod_bucketeer.c
Posted by Ryan Bloom <rb...@covalent.net>.
> > Every bucket can be read. That is a mandatory function that all
buckets
> > must implement. If the back-end bucket-type isn't know, the filter
can
> > ALWAYS do a read/apr_bucket_make_heap.
>
> True. Though we've specified the additional semantics that
> apr_bucket_read() causes the bucket to auto-morph into one that is
> in-memory. Either way.
I didn't think it _had_ to auto-morph. My understanding is that the
default buckets do, because we assume the performance will be better if
they do. That makes sense, because a file_bucket is likely to be read
multiple times, so it makes sense to morph the bucket on the first read,
so that we don't have to go to the disk twice.
Ryan
RE: cvs commit: httpd-2.0/modules/test mod_bucketeer.c
Posted by Cliff Woolley <jw...@virginia.edu>.
On Thu, 30 May 2002, Ryan Bloom wrote:
> > True, but I thought part of the idea behind buckets is so that the
> > backing type doesn't have to be known.
>
> Exactly!
Exactly. All you really need to test here is length. If the bucket is
zero bytes, that's all you need to know. That gets you all metadata
buckets handled in one fell swoop. If there are certain kinds of metadata
buckets a filter *does* know about and want to handle specially (eg EOS or
FLUSH), it can make the choice to special case those.
> Every bucket can be read. That is a mandatory function that all buckets
> must implement. If the back-end bucket-type isn't know, the filter can
> ALWAYS do a read/apr_bucket_make_heap.
True. Though we've specified the additional semantics that
apr_bucket_read() causes the bucket to auto-morph into one that is
in-memory. Either way.
--Cliff
RE: cvs commit: httpd-2.0/modules/test mod_bucketeer.c
Posted by Ryan Bloom <rb...@covalent.net>.
> From: Justin Erenkrantz [mailto:jerenkrantz@apache.org]
>
> On Thu, May 30, 2002 at 02:43:05PM -0700, Ryan Bloom wrote:
> > This doesn't fix the underlying problem. Mod_bucketeer needs to be
> > smart enough to copy buckets if it doesn't recognize their type. If
the
> > bucket can't be copied, then the filter should remove the bucket
from
> > one brigade and add it to the other.
> >
> > Any filter that silently removes buckets is broken.
>
> True, but I thought part of the idea behind buckets is so that the
> backing type doesn't have to be known.
Exactly!
> The fact here is that not all buckets can be "read" such as EOS,
> flush, and error. Perhaps these buckets should return an error
> if they are read? Then, if a filter sees APR_ENOTIMPL, it can
> copy them over.
Every bucket can be read. That is a mandatory function that all buckets
must implement. If the back-end bucket-type isn't know, the filter can
ALWAYS do a read/apr_bucket_make_heap.
Ryan
Re: cvs commit: httpd-2.0/modules/test mod_bucketeer.c
Posted by Cliff Woolley <jw...@virginia.edu>.
On Thu, 30 May 2002, Justin Erenkrantz wrote:
> The fact here is that not all buckets can be "read" such as EOS,
> flush, and error.
Sure they can. They just contain zero data bytes.
> Perhaps these buckets should return an error
> if they are read? Then, if a filter sees APR_ENOTIMPL, it can
> copy them over. -- justin
No way. *All* buckets can be read from, that's an important part of the
design. Having to test for APR_ENOTIMPL all over the place would be a
major drag.
--Cliff
Re: cvs commit: httpd-2.0/modules/test mod_bucketeer.c
Posted by Justin Erenkrantz <je...@apache.org>.
On Thu, May 30, 2002 at 02:43:05PM -0700, Ryan Bloom wrote:
> This doesn't fix the underlying problem. Mod_bucketeer needs to be
> smart enough to copy buckets if it doesn't recognize their type. If the
> bucket can't be copied, then the filter should remove the bucket from
> one brigade and add it to the other.
>
> Any filter that silently removes buckets is broken.
True, but I thought part of the idea behind buckets is so that the
backing type doesn't have to be known.
The fact here is that not all buckets can be "read" such as EOS,
flush, and error. Perhaps these buckets should return an error
if they are read? Then, if a filter sees APR_ENOTIMPL, it can
copy them over. -- justin