Tomcat 3.3 contextAdmin issues

[Paulo Gaspar <pa...@krankikom.de>]

Hi,


I have been trying to improve a bit on the "admin" 
application, especially on the "contextAdmin" bit,
tweaking its web pages/JSPs in order to add functionality
and ease of use.

I am especially interested on making it easier to restart
individual applications, deploy or redeploy new 
applications and restart the whole container trough this
web interface.

The first two things that are really confusing are:
- the existence of 2 lines of very similar classes 
  (e.g.: TomcatAdmin and ContextAdmin) in the "tadm"
  package at "webapps\admin\WEB-INF\classes\tadm";
- and the fact that "restart.jsp" does not work as 
  expected producing duplicate entries in the list 
  presented by "contextList.jsp".
  
Looking at "org.apache.tomcat.core.ContextManager" did 
not help a lot since its comments are not very clear
either, as is the case of its "shutdown()" method where
comments make me doubt about how cleanup should be done.


Can someone help me here?

I am willing to help improving the admin application and
I have a lot of selfish motives to do it, but I need 
some pointers in order to understand how it should work.

This restart thing probably has some relation with the
work Costin is doing on "EmbededTomcat" - maybe the 
information missing is the same.


Thanks and have fun,
Paulo Gaspar

RE: Tomcat 3.3 contextAdmin issues

[Costin Manolache <cm...@yahoo.com>]

On 20 Aug 2001 02:01:12 +0200, Paulo Gaspar wrote:

> Your explanation sure helps understanding what functionality is intended
> for each tag. I can take a look at that too. It is easier for me to 
> understand the taglibs than the rest of Tomcat.
> =;o)

Well, I hope understanding the rest of tomcat is not that difficult, but
the goal of the tags was to hide tomcat implementation details ( or
tomcat itself ). A "context" tag can have different implementations,
maybe specific to other servlet containers - the admin interface will be
the same, just a different taglib code will be used.

BTW, nothing requires you to use the tags or jsp - but whatever you use,
please keep the implementation behind an interface similar with the
tags. ( i.e. similar name and behaviors ).


> In this case I am talking about the comments in the method
>     org.apache.tomcat.core.ContextManager.shutdown()
> 
> In this method's source code there are 2 blocks of cleanup code that 
> were commented out. The fact that they were not just removed and the 
> nature of a comment:
>   "remove the modules ( XXX do we need that ? )"
> before one of those blocks makes me wonder how sure it is that they
> are correct.

The code that is commented out used to be part of the method, including
the one with XXX comment ( and the answer is - we don't need to remove
the modules ).

The idea is quite simple - shutdown() is symetrical with init(). If you
add any context manually ( for example EmbededTomcat.addContext() ), you
should also remove it when you stop ( if you want to ). If a module is
adding contexts - it should also remove them ( or leave them in and
don't add them back ).

That's probably where the bug is, I need to review ContextXmlReader and
AutoWebApp to make sure they remove the contexts on shutdown.

shutdown() followed by init() should bring you back to the same state as
you were before ( if no configuration change happened ). 

Now, that's the theory - or what seems to be the 'correct' behavior for
the core. 3.3 is mostly about making sure the core behaves in a well
defined way - better modules will follow, and we can fix modules easily
without all the headache of a major release. 

If you have any doubts about the ContextManager behavior - please speak
now, we may still be able to fix it. 

Regarding the module removal - again, whoever adds modules should also
remove them back, shutdown shouldn't mess with that ( since init doesn't
either ).

In future, some modules will be enhanced to deal 'nicely' with restarts,
and I plan to add support module reloading ( via a module, of course
:-). As I mentioned so many times, after 3.3 we shouldn't have to change
anything in the core, so new modules can rely on some known and well
defined behavior ( well defined doesn't mean perfect, but good enough
:-). 


> Specific questions, besides the above "ContextManager.shutdown()" 
> issue:
>  - Why is it possible to add 2 or more contexts with the same name
>    and base path? It is a cleanup issue that this happens with the 
>    "restart.jsp" code, but shouldn't this kind of duplication also 
>    be prevented?

Contexts should be identified by hostname and base path. If we don't
check for that - it's a bug. BTW, the right way to fix the bug is to add
the check - not in the core, but in a module ( that can do other checks
during the addContext/contextInit phases ). Again, this can wait until
after 3.3, I don't think it's a major issue. 

>  - To make a hot restart, it looks like modules should be restarted
>    too. Is this correct?

Modules should deal themself with the init/shutdown events, and restart.
Most existing modules do not need to be reloaded - but that can be done
too. I'm still investigating how to implement module restart, and "hot"
module add/remove. I don't think this can be done in 3.3, but in a set
of modules that can be released after 3.3 ( I have some code, but now I
want to focus on 3.3 and few other important pieces ).


>  - When using "restart.jsp", previously removed contexts (using the
>    admin pages) were not added back. Why?

Bug probably :-) Again, the right behavior is that whoever adds
something should take it back. 

>  - Where are existing contexts detected and loaded? Is it on a 
>    module? And if yes, then which?

Contexts ( and modules ) are added in 2 ways:
- in applications embedding tomcat, by the application via calls to EmbededTomcat 
or ContextManager. 

- in standalone tomcat, by various configuration modules. Right now the list is:
ContextXmlReader
AutoWebApp.

The first will add modules declared in apps-XXX.xml and server.xml ( which 
shouldn't be used for contexts, that's only for backward compat ).

The second deals with webapps-like dirs ( you can define additional
dirs, very usefull for multiple virtual hosts ).

Thats' where you should check what happens on shutdown.

BTW, this review will help a lot - thank you very much. I think it's very
important to make sure the behavior is right in the first place, we can fix
the modules later ( even after beta2, and even after release 
 - I suspect most of the errors are minor and shouldn't brake anything that works )

Costin 

RE: Tomcat 3.3 contextAdmin issues

[Paulo Gaspar <pa...@krankikom.de>]

Answer inline:

> -----Original Message-----
> From: Costin Manolache [mailto:cmanolache@yahoo.com]
> Sent: Saturday, August 18, 2001 8:29 PM
> 
> On 18 Aug 2001 19:56:33 +0200, Paulo Gaspar wrote:
>
> .......
>
> > The first two things that are really confusing are:
> > - the existence of 2 lines of very similar classes 
> >   (e.g.: TomcatAdmin and ContextAdmin) in the "tadm"
> >   package at "webapps\admin\WEB-INF\classes\tadm";
> 
> Well, TomcatAdmin is the first tag I wrote, and it did a lot of things.
> Too many, actually, so I started to split it. ContextAdmin will focus on
> context tasks, TomcatAdmin for generic tomcat.
> 
> Also, TomcatAdmin declares the "ContextManager" scripting variable, and
> that's a problem to be fixed - you can have only one <tadm:admin> in a
> page right now. 
 
Yes, I was thinking the same about that variable. I had to go around that
to improvise a context restart (by using "<% { %>" and "<% } %>").

Your explanation sure helps understanding what functionality is intended
for each tag. I can take a look at that too. It is easier for me to 
understand the taglibs than the rest of Tomcat.
=;o)
 
> > - and the fact that "restart.jsp" does not work as 
> >   expected producing duplicate entries in the list 
> >   presented by "contextList.jsp".
> 
> That's a bug. I'll take a look, I wrote restart.jsp mostly as a test -
> to make sure all modules are cleaning up after themself ( i.e. you do
> restart.jsp few times and check the thread count, memory use, etc - it
> should stay constant ). If some contexts are not removed - that must be
> fixed ( please add a bug so I'll remember ).

I will try (adding the bug).
     
> > Looking at "org.apache.tomcat.core.ContextManager" did 
> > not help a lot since its comments are not very clear
> > either, as is the case of its "shutdown()" method where
> > comments make me doubt about how cleanup should be done.
> 
> Ok, what's not clear :-) ? As you know, I'm not very good at docs, but
> if you ask specific questions I may be able to answer ( and fix the
> comments along the way ).  

No one seems to be very good at docs on Tomcat. =;o)

In this case I am talking about the comments in the method
    org.apache.tomcat.core.ContextManager.shutdown()

In this method's source code there are 2 blocks of cleanup code that 
were commented out. The fact that they were not just removed and the 
nature of a comment:
  "remove the modules ( XXX do we need that ? )"
before one of those blocks makes me wonder how sure it is that they
are correct.


> > This restart thing probably has some relation with the
> > work Costin is doing on "EmbededTomcat" - maybe the 
> > information missing is the same.
> 
> It has some relation, in the sense EmbeddedTomcat must be able to
> restart ( and it's using the same calls as restart.jsp ).

Yes, the cleanup issues are related.


Specific questions, besides the above "ContextManager.shutdown()" 
issue:
 - Why is it possible to add 2 or more contexts with the same name
   and base path? It is a cleanup issue that this happens with the 
   "restart.jsp" code, but shouldn't this kind of duplication also 
   be prevented?
 - To make a hot restart, it looks like modules should be restarted
   too. Is this correct?
 - When using "restart.jsp", previously removed contexts (using the
   admin pages) were not added back. Why?
 - Where are existing contexts detected and loaded? Is it on a 
   module? And if yes, then which?

As you see from the above questions, I still ignore a lot.


Thanks a lot for your attention on this, Costin.


Have fun,
Paulo Gaspar

Re: Tomcat 3.3 contextAdmin issues

[Costin Manolache <cm...@yahoo.com>]

On 18 Aug 2001 19:56:33 +0200, Paulo Gaspar wrote:

> I have been trying to improve a bit on the "admin" 
> application, especially on the "contextAdmin" bit,
> tweaking its web pages/JSPs in order to add functionality
> and ease of use.

Great :-)

> I am especially interested on making it easier to restart
> individual applications, deploy or redeploy new 
> applications and restart the whole container trough this
> web interface.



> The first two things that are really confusing are:
> - the existence of 2 lines of very similar classes 
>   (e.g.: TomcatAdmin and ContextAdmin) in the "tadm"
>   package at "webapps\admin\WEB-INF\classes\tadm";

Well, TomcatAdmin is the first tag I wrote, and it did a lot of things.
Too many, actually, so I started to split it. ContextAdmin will focus on
context tasks, TomcatAdmin for generic tomcat.

Also, TomcatAdmin declares the "ContextManager" scripting variable, and
that's a problem to be fixed - you can have only one <tadm:admin> in a
page right now. 


> - and the fact that "restart.jsp" does not work as 
>   expected producing duplicate entries in the list 
>   presented by "contextList.jsp".

That's a bug. I'll take a look, I wrote restart.jsp mostly as a test -
to make sure all modules are cleaning up after themself ( i.e. you do
restart.jsp few times and check the thread count, memory use, etc - it
should stay constant ). If some contexts are not removed - that must be
fixed ( please add a bug so I'll remember ).

    
> Looking at "org.apache.tomcat.core.ContextManager" did 
> not help a lot since its comments are not very clear
> either, as is the case of its "shutdown()" method where
> comments make me doubt about how cleanup should be done.

Ok, what's not clear :-) ? As you know, I'm not very good at docs, but
if you ask specific questions I may be able to answer ( and fix the
comments allong the way ).  

> This restart thing probably has some relation with the
> work Costin is doing on "EmbededTomcat" - maybe the 
> information missing is the same.

It has some relation, in the sense EmbeddedTomcat must be able to
restart ( and it's using the same calls as restart.jsp ).

Costin