Mailing-List: contact tomcat-dev-help@jakarta.apache.org; run by ezmlm
Message-ID: <38E68E38.46341614@eng.sun.com>
Date: Sat, 01 Apr 2000 16:03:05 -0800
From: Costin Manolache <costin@eng.sun.com>
MIME-Version: 1.0
To: tomcat-dev@jakarta.apache.org
Subject: Re: Multi-homing/Virtual named hosts
References: <006401bf9b9d$5159e5c0$4bd90eca@sew.co.nz>
 <38E5950D.6F59B365@eng.sun.com> <00ad01bf9bd2$ce47e980$4bd90eca@sew.co.nz>
 <00dd01bf9bdf$102c6360$4bd90eca@sew.co.nz> <38E6251F.82D166A3@eng.sun.com>
 <001601bf9c2e$0877f090$4bd90eca@sew.co.nz>
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

> > > 2) It appears that ContextManager would need to be changed to handle
> > > anything other than a simple path - was this expected?
> >
> > Context yes, I don't see the problem with ContextManager - it doesn't care
> > about virtual hosts, it just deals with Request/Responses.
>
> But this appears to be where your statement doesn't make sense - the
> SimpleMapper depends _very much_ on how contexts are stored in ContextMapper
> to find the context that is correct for the current request. Take the
> following method (from SimpleMapper)

That's because SimpleMapper is derived from old code, and it's not so simple to
explain how it evolved :-)

ContextManager has a list of all Contexts in order to implement "stop" ( to
shut down all Contexts). The fact that we have getContext ( name ) is a mistake,

that assumes unique names - probably the name should be vhost+path or
we should just remove that method.

If you use the getContext() method in CM, then you'll restrict your code to use
the Hashtable or any ( hard to change ) alghoritm that is build into
ContextManager.
Since the only use of contexts in CM is outside of the critical path, when the
server is shut down - there is no reason to optimize it. It will also make CM
very
complex - it's job is to serve as a control point, not to implement efficient
(host, path ) -> context mappings.

The right ( IMHO ) way is to construct an efficient data structure using the
addContext() notification - and in fact that's what SimpleMapper is doing in
requestMap(). Mea culpa for getContext() - I kept the old ( and simpler )
code that just used ContextManager.getContext( name ) instead of using
the internal structure.

Let me try again:  if you use CM.getContext() you put the "pressure" on
Context manager to implement the mapping, and you have no control.
If you use addContext() to construct your own structure ( and you
can use virtual host or any other property in Context - including
context init params or any other property that might be added to
context - group, etc) - then you will be able to partition and organize
everything in an optimal mode.

You may also customize the mapper based on your environment - you
may have many virtual hosts, each with one or few contexts - and you'll
use a different alghoritm if you have few hosts and many contexts.
( Hashtables are inefficient if you have few objects ). You just can't do
that customization if you hardcode the representation in your object model
( and use methods like CM.getContext( path )).

At least that was my intention when I wrote some of the code.


>     Context getContextByPath(String path) {
> ...
>         lookup:
>  do {
>      ctx = cm.getContext(path);

Bad - one reason for starting with a new mapper.

> This method is called by contextMap to determine the context that is
> required by the request - simply put, it cycles through the path request,
> lopping bits off the end until it finds one that matches a context "path"
> attribute and once found, returns it. This is my interpretation of the code.

And that's very bad too - because the same cycle will be repeated on
request map. And it's an expensive operation. It's also looking up
based on path prefix - Context path can be kept split in components
and you can do the search in the reverse direction -  most of the time
the context is flat, with one or 2 levels.

I think a much better way would be to split the context in components
and create a tree structure, then eat the request path from start to end.

We don't do that now - it's a "Simple" mapper,  but the model allows
that.


>     public Context getContext(String name) {
>  return (Context)contexts.get(name);
>     }
>
> Hmmmmmmmmmm - this means that the context is being indexed on its _path_ -
> which is absolutely no use in the situation of multiple contexts with the
> _same_ path (two virtual hosts each with / for example).

Yes, and this is a bad choice for mapping.


> This has to mean one of two things, either (a) VirtualHosts have to be
> defined OUTSIDE of ContextManagers, or (b) ContextManagers will need to deal
> with a more complex mechanism for determining the path. As I discuss this
> more and more, I am leaning towards (a) - but I have yet to figure out from
> the rather voluminous source code exactly how context managers get set...
> (any points in this regard would be helpful) (a) would also seem to map more
> closely to your comment:

I don't want to ( and can't ) force you to use one or another - I'm just listing
my
arguments and how I would do it.

My point is that ContextManager shouldn't "deal" with determining the path.
The only reason CM needs the Contexts is because it have to shut down.
It is not supposed to deal with mapping or determining the path - you should
be able to iterate and get all contexts.

For (a) - I thing a better representation is ContextGroup ( VHost is just a
particular case). We started to use Container for that, as a grouping of
contexts.

But again - the mapper is supposed to implement the mapping, and you want
to be able to plug specialized alghoritms, since this is the expensive
operation.
Container ( or ContextGroup or Context or VirtualHost) is just a data structure
with the common properties.

( Context and ContextManager are already too complex, adding mapping
alghoritms will make them imposible to read)


> Er, not quite. Again, to quote from the source code in
> Ajp12ConnectionHandler.java, in the AJP12RequestAdapter, one sees the
> following lines (readNextRequest):
>
>       contextPath = ajpin.readString(null);              file://Zone
>       // GS, the following commented line causes the Apache + Jserv + Tomcat
>       // combination to hang with a 404!!!
>       // if("ROOT".equals( contextPath ) ) contextPath="";
>       if("ROOT".equalsIgnoreCase( contextPath ) ) contextPath=null;
>       if( doLog ) log("AJP: CP=" + contextPath);
>
>       if( contextPath!= null )
>    context=contextM.getContext( contextPath );
>
> It is again, just assuming a path. This is fine if there is only one
> "virtual" host, but the host is yet to be determined (it occurs a few lines
> further down), which, if choosing the (a) option above would determine the
> contextManager to actually use in this case. My simple preference would be
> to not attempt to determine the context at all in this location.

Right - just set the informations you received from the web server.


Please not that I don't want to impose a certain solution - but I spent
a lot of time with tomcat ( and few other servers ) and this is the
result :-)

I have nothing against having a VirtualHost object ( I would prefer to
use Container, and a virtual host will be just an instance ), but I think
it's better to make the parsing and to determine the context in the
mapper interceptor, not in ContextManager or Container.

It is a very important decision - based on the object model we choose
we'll be ( or not ) able to implement certain alghoritms. If we put
part of the parsing or searching in ContextManager ( or worse, in
the adapter ) - we'll just disable some options.

Costin