filters (was: Re: [PATCH] mod_proxy Keepalives)

[Greg Stein <gs...@lyra.org>]

On Mon, Apr 02, 2001 at 06:25:41AM -0700, rbb@covalent.net wrote:
> On Mon, 2 Apr 2001, Greg Stein wrote:
>...
> > The filters should not be tied to request_rec or conn_rec. Unfortunately,
> > they are.
> >
> > I'd be most interested in understanding how the proxy-outbound filters you
> > are using (I forgot the list) are tied to the conn_rec. If we can discover
> > how they are tied, then we can get them fixed.
> >
> > Filters are arranged as a simple chain. Somewhere, you record the head of
> > that chain, and you organize how items are inserted into a current/active
> > chain. I don't believe that conn_rec.output_filters is any benefit to you;
> > the filter system itself: yes. I believe you can use the filters, and that
> > you can use them without needing a conn_rec.
> >
> > Summary: for expediency, I can see how you may want to use a conn_rec. But
> > please recognize and (I hope) make it a long-term goal to remove them. As a
> > precondition, that means the (builtin) filters that you want to use need to
> > fixed. I'd totally agree to those fixes on two reasons: make the filters
> > more isolated/independent/less-coupled, and enable mod_proxy to stop using
> > conn_rec for outbound connections.
> >
> > [ consider: any filter referencing a conn_rec or a request_rec becomes
> >   tightly coupled with the Apache system. And ideal filter is just massaging
> >   data that flows through it. ]
> 
> I seriously disagree with modifying filters.  Our filters are designed for
> Apache, to de-couple them from Apache would remove a lot of what makes
> them perfect for us.  There is a lot of information that is retrieved from
> the conn and request rec's, and making all of that inaccessible to filters
> is a bad idea IMHO.

Well, the DECHUNK is the filter that we're talking about. Why should that be
tied to request_rec or to conn_rec? Shouldn't it be possible to DECHUNK a
stream?

> Having said that, I was involved on the initial proxy development, where
> we decided to use filters for both incoming and outbound connections.  To
> be honest, the design is rather slick.  Yes, there are some fields that do
> not apply to outbound connections.  The solution for that, isn't to remove
> conn and request recs from filters, but rather to remove those fields from
> the conn and request recs.  The fields can be moved into some non-central
> location.

Oh, using the filters *is* slick. No question at all!

But having filters reference the conn_rec and the request_rec couples them
to Apache. The DECHUNK should be able to operate on arbitrary streams; there
isn't a reason it needs to be tied to Apache's conn_rec or request, wouldn't
you agree?

I'm not suggesting that *all* of our filters become independent. The header
filter is tied to the request_rec, and there is very little we can do about
that. Just that we look at whether proxy's filters can be independent.

And lastly: the conn_rec is about inbound connections. We should not be
moving fields out of it simply because they don't also apply to outbound
connections or to the use of the DECHUNK filter. We can talk about this at
the Hackathon, but really... the DECHUNK can apply to inbound/outbound if we
can make it operate on a stream, independently of request_rec or conn_rec.

Okay, I just looked at the ap_dechunk_filter in http_protocol.c. It has the
following references:

*) apr_pcalloc(f->r->pool, ...)
*) ap_getline(line, sizeof(line), f->r, 0)
*) f->c->remain = ...

The pool can be solved by allocating the context when we insert the filter.
(an interesting question here, though: what would the general policy be, for
a pool for filters to allocate mem?)

The last two are dealing with how input is read. I think we can cleanly
solve that one, so that you can read lines from an arbitrary filter.
Currently, ap_getline can only read a line from the top of the connection's
filter chain. It would be nice to be able to read a line from any filter,
whether it is a connection filter, the top of the connection filters, or a
request filter.

Anyway... if those three references were removed from DECHUNK, then we'd be
able to dechunk any(!) stream. That would be kinda cool :-)

Hmm. I just realized something. *Because* ap_getline only works on the top
of the connection filter stack, it means that DECHUNK will ignore filters
between itself at the connection filters. It should be using f->next, but it
doesn't... urg.

Ryan: you mentioned that you recalled there may have been a reason why "read
one line" isn't part of ap_input_mode_t. Can you remember why?

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: filters (was: Re: [PATCH] mod_proxy Keepalives)

[Graham Leggett <mi...@sharp.fm>]

rbb@covalent.net wrote:

> Make sure that you are resetting all of your fields correctly.

I reinserted the filters into the conn_rec (I was keeping them over to
the next connection before)...

That was it - problem seems to be fixed :)

Regards,
Graham
-- 
-----------------------------------------
minfrin@sharp.fm		"There's a moon
					over Bourbon Street
						tonight..."

Re: filters (was: Re: [PATCH] mod_proxy Keepalives)

[Graham Leggett <mi...@sharp.fm>]

rbb@covalent.net wrote:

> It would be cool, but it would also be incorrect IMO.  Filters are Apache
> based things.  Chunking is an HTTP based thing.  To try to separate
> filters from Apache will cause more problems than it solves.

Would it not be ideal to remove the dependancy on request_rec from the
dechunk filter?

> Make sure that you are resetting all of your fields correctly.
> 
> > What could be happening is that ap_getline() is reading directly from
> > the CORE_IN filter after the HTTP_IN filter had swallowed up the lines -
> > causing the "missing response"...
> 
> Can't happen.  CORE_IN doesn't actually do any reading from the socket.

Hmmm - ok. I will take a closer look at this and see what I can uncover.
  
Regards,
Graham
-- 
-----------------------------------------
minfrin@sharp.fm		"There's a moon
					over Bourbon Street
						tonight..."

Re: filters (was: Re: [PATCH] mod_proxy Keepalives)

[rb...@covalent.net]

On Mon, 2 Apr 2001, Graham Leggett wrote:

> Greg Stein wrote:
>
> > Anyway... if those three references were removed from DECHUNK, then we'd be
> > able to dechunk any(!) stream. That would be kinda cool :-)
>
> Agreed :)

It would be cool, but it would also be incorrect IMO.  Filters are Apache
based things.  Chunking is an HTTP based thing.  To try to separate
filters from Apache will cause more problems than it solves.

> > Hmm. I just realized something. *Because* ap_getline only works on the top
> > of the connection filter stack, it means that DECHUNK will ignore filters
> > between itself at the connection filters. It should be using f->next, but it
> > doesn't... urg.
> >
> > Ryan: you mentioned that you recalled there may have been a reason why "read
> > one line" isn't part of ap_input_mode_t. Can you remember why?

Yes.  It doesn't logically make sense.  As soon as you say "read one line"
as a parameter, then it also makes sense to say "read 512 bytes" as a
parameter.  Then you need to return how many bytes were actually read.
You also need to deal with how many bytes are being returned, because a
filter could modify how much data is returned.

Once you start really looking at this, it becomes easier to just use a
field in the conn_rec.

> This might explain a problem I've had with the keepalive patch - after a
> request is proxied that involves a header only being returned (and no
> body) such as a HEAD request, the request directly after that fails with
> a an error that the first response line cannot be read (string length
> read is zero).

Make sure that you are resetting all of your fields correctly.

> What could be happening is that ap_getline() is reading directly from
> the CORE_IN filter after the HTTP_IN filter had swallowed up the lines -
> causing the "missing response"...

Can't happen.  CORE_IN doesn't actually do any reading from the socket.

Ryan

_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------

Re: filters (was: Re: [PATCH] mod_proxy Keepalives)

[Graham Leggett <mi...@sharp.fm>]

Greg Stein wrote:

> Anyway... if those three references were removed from DECHUNK, then we'd be
> able to dechunk any(!) stream. That would be kinda cool :-)

Agreed :)

> Hmm. I just realized something. *Because* ap_getline only works on the top
> of the connection filter stack, it means that DECHUNK will ignore filters
> between itself at the connection filters. It should be using f->next, but it
> doesn't... urg.
> 
> Ryan: you mentioned that you recalled there may have been a reason why "read
> one line" isn't part of ap_input_mode_t. Can you remember why?

This might explain a problem I've had with the keepalive patch - after a
request is proxied that involves a header only being returned (and no
body) such as a HEAD request, the request directly after that fails with
a an error that the first response line cannot be read (string length
read is zero).

What could be happening is that ap_getline() is reading directly from
the CORE_IN filter after the HTTP_IN filter had swallowed up the lines -
causing the "missing response"...

Regards,
Graham
-- 
-----------------------------------------
minfrin@sharp.fm		"There's a moon
					over Bourbon Street
						tonight..."