You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Konstantin Kolinko <kn...@gmail.com> on 2011/11/02 16:35:36 UTC

Redeploy on context.xml changes (Was: Tagging 7.0.23)

2011/11/2  <ma...@apache.org>:
> Konstantin Kolinko <kn...@gmail.com> wrote:
>
>>2011/11/2 Mark Thomas <ma...@apache.org>:
>>> On 01/11/2011 22:39, Mark Thomas wrote:
>
>>> My first stab at a patch for this is at [1]. Comments welcome.
>
>>>http://people.apache.org/~markt/patches/2011-11-01-redeploy-trunk-v1.patch
>
>
>>Quick review - several typos:
>>
>>mbeans-descriptors.xml:
>>- typo "findRedployResources"
>>- "Path to the resource, relative to docBase" - It is "either absolute
>>or relative to docBase".
>>
>>docs: s/if is is updated/if it is updated/
>
> I'll get those fixed for the next iteration.

Also s/redploy/redeploy/ in some Javadoc.

The removeWatchedResource() method in the patch is NOOP instead of
calling renamed method.

The following in StandardContext.java does not look backwards-compatible:
-        fireContainerEvent("addWatchedResource", name);
+        fireContainerEvent("addReloadResource", name);
and
-         fireContainerEvent("removeWatchedResource", name);
+        fireContainerEvent("removeReloadResource", name);

Better keep the old name of "watched" and just add the new one of "redeploy".
In all places the "redeploy" list is new and parallel to the old "watched" one.

The only place where meaning changes is
HostConfig#addWatchedResources() that now processes both types.

> The one thing I don't like is that a change to conf/context.xml that
> breaks the file effectively kills the instance until the problem is
> fixed and the instance re-started.

Looking at HostConfig.checkResources(DeployedApplication) note several
ExpandWar.delete() calls. The order is important there. If redeploy
resource #i is missing or updated it deletes all following redeploy
resources starting with #(i+1).

With this behaviour it is better not to let a user to control this
"redeploy" list.

BTW, checkResources(DeployedApplication) deletes reload resources on
auto-undeploy as well, but first checks their path.


Regarding the original task of what happens when context.xml is
touched or edited:  redeploying deletes the work dir, along with all
persisted sessions and all compiled JSPs. (ContextConfig.destroy()
triggered by Host.removeChild() does the deletion)

It is an enormous price to pay for editing an AccessLogValve
configuration or whatever is important in context.xml. I'd prefer a
solution that does not delete the work dir.

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Redeploy on context.xml changes (Was: Tagging 7.0.23)

Posted by Mark Thomas <ma...@apache.org>.
On 14/11/2011 05:54, Konstantin Kolinko wrote:

Note: "Fixed" below means I have fixed the issue locally and will
include it in the commit.

> 1) s/Redploy/Redeploy/ in several comments.
Fixed.

> 2) In HostConfig.checkResources() there are three occurrences of the
> following code:
Fixed.

> 3) Testing redeployment.
Thanks for this. It is a useful recipe.

> Tests:
> 1. touch context.xml  -> OK, all redeployed
> 2. touch context.xml.default -> OK, all redeployed
> 3. touch examples.xml -> FAIL, redeployed examples app, but it deleted
> "context.xml.default" (causing redeployment of all other webapps)
> Now shutdown Tomcat, restore "context.xml.default" and start Tomcat
Fixed as a result of the fix for 2) above.

> 4. delete examples.xml -> Examples app is undeployed, but is instantly
> deployed again as "web application directory". It is strange. Is it
> expected?
On first inspection, not what I expected. On reflection, I think it is.
Consider the sequence (when context.xml files are copied to
conf/Catalina/localhost
- new dir with META-INF/context.xml is copied to appBase
- context.xml is copied to conf/Catalina/localhost during deploy
- conf/Catalina/localhost/newapp.xml is edited (redploy)
- conf/Catalina/localhost/newapp.xml is deleted (redploy)
- At this point the original context.xml is copied from META-INF

I think an argument can be made for this to work either way. Given the
current approach has been like this since the early days of 5.5.x, I
plan on leaving it as is.

> 5. rename "context.xml.default" -> OK, all redeployed
> 6. rename "context.xml" -> OK, all redeployed
> 
> I have not tested what happens if I deploy a war file.

I repeated the above with examples -> examples.war and all behaved the
same way.

> 4) There is StandardContext#setDefaultContextXml() method.
> I do not see it being called by Tomcat code, nor I see it documented
> in config/context.html.

I assume this is for the embedded case.

> 5) FailedContext is used in HostConfig class only.
> 
> Actually HostConfig#digester just preparses the context.xml file: it
> processes only the root <Context> element and its attributes.

This is by design. If the Context element and attributes are OK then a
valid context object can be created. Whether or not it can start is a
different problem. FailedContext is intended for use when the config is
so badly broken that Tomcat can't determine what class it should be
using for the Context object or there is some issue configuring it.


Is there scope for further improvement, clarification around deployment?
Sure. But I don't think this issue is place to make those changes.

I'll be committing the amended patch shortly.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Redeploy on context.xml changes (Was: Tagging 7.0.23)

Posted by Konstantin Kolinko <kn...@gmail.com>.
2011/11/10 Mark Thomas <ma...@apache.org>:
> On 08/11/2011 22:49, Mark Thomas wrote:
>> I found some time today to look at this. Summarising the issues so far:
>>
>> 1. I have separately applied a couple of minor fixes that were included
>> in the larger patch.
>>
>> 2. Renaming watchedResources -> reloadResoucres
>> Agree with reverting this change
>>
>> 3. Adding JavaDoc to FailedContext
>> Agree with doing this. I tweaked the wording a little bit as it might
>> not be a StandardContext instance that we failed to create
>>
>> 3. Formatting of FailedContext
>> Given that the majority of the code is NO-OP and the rest is pretty much
>> getters/setters the current formatting is deliberate to reduce the
>> length of the file. I did something similar for WebXml.
>>
>> 4. s/absolue/absolute/
>> Drat. Missed this first time around. Fixed now.
>>
>> 5. redeployResource has been implemented for FailedContext
>>
>> 6. Unexpected deletion of resources due to adding conf/context.xml to
>> redeployResources
>> - removed redeployResources from Context
>> - ensure global resources are never deleted
>> - add global resources at end of redeploy list
>>
>>
>> Particularly with the changes for 6, the patch should be easier to read.
>>
>> The patch is here:
>> http://people.apache.org/~markt/patches/2011-11-08-redeploy-trunk-v4.patch
>>
>> There are currently a few open TC7 bugs so fixing those should provide
>> enough time to review this patch before the 7.0.23 tag.
>
> It has been a couple of days and no objections so I plan to apply this
> patch in the next day or so and tag 7.0.23 early next week. That should
> give me time to fix the remaining open bug and any unit test and/or TCK
> failures that appear.
>

1) s/Redploy/Redeploy/ in several comments.
2) In HostConfig.checkResources() there are three occurrences of the
following code:

                            if (...
                                    || (current.getAbsolutePath().startsWith(
                                            configBase().getAbsolutePath()))) {
                                if (log.isDebugEnabled())
                                    log.debug("Delete " + current);
                                ExpandWar.delete(current);
                            }
Thus the following two files will be deleted:
- CATALINA_BASE/conf/<service>/<host>/<appname>.xml
- CATALINA_BASE/conf/<service>/<host>/context.xml.default

Actually there is the following code block that protects
"context.xml.default", but it is only in 2 places out of 3:

                        // Never delete per host context.xml defaults
                        if (Constants.HostContextXml.equals(
                                current.getName())) {
                            continue;
                        }

It is easy to fix.

3) Testing redeployment.
Using the following configuration:
- Default tomcat build, plus make two copies of the default context.xml file:
cp  context.xml  Catalina/localhost/context.xml.default
cp  context.xml  Catalina/localhost/examples.xml

Tests:
1. touch context.xml  -> OK, all redeployed
2. touch context.xml.default -> OK, all redeployed
3. touch examples.xml -> FAIL, redeployed examples app, but it deleted
"context.xml.default" (causing redeployment of all other webapps)
Now shutdown Tomcat, restore "context.xml.default" and start Tomcat

4. delete examples.xml -> Examples app is undeployed, but is instantly
deployed again as "web application directory". It is strange. Is it
expected?

5. rename "context.xml.default" -> OK, all redeployed
6. rename "context.xml" -> OK, all redeployed

I have not tested what happens if I deploy a war file.


4) There is StandardContext#setDefaultContextXml() method.
I do not see it being called by Tomcat code, nor I see it documented
in config/context.html.

The code itself is old, starting with the following commit (Sept 2004,
7 years ago).
http://svn.apache.org/viewvc?view=revision&revision=303172

Do not know why it is needed. Maybe for some embedding scenarios.
It provides an alternative for conf/context.xml.

I mean that HostConfig#addGlobalRedeployResources() could use that
file, but it is unclear how to prevent its deletion on undeploy. It is
a minor and rare issue though (and it is not so important what file
you are going to touch to cause redeployment. Let's allow to touch
conf/context.xml regardless of this configuration).


5) FailedContext is used in HostConfig class only.

Actually HostConfig#digester just preparses the context.xml file: it
processes only the root <Context> element and its attributes.

Main parsing is done in ContextConfig#init() ->
#contextConfig(Digester) -> calls #processContextConfig(Digester, URL)
x 3 times.

There is ContextConfig#ok flag that is reset to "false" if parsing fails.

At end of ContextConfig#configureStart() it does:
        if (ok) {
            context.setConfigured(true);
        } else {
            log.error(sm.getString("contextConfig.unavailable"));
            context.setConfigured(false);
        }

I mean:
- FailedContext class solves a very limited task.
- I think that if a StandardContext instance were used instead of
FailedContext, it will result in a failure when parsing one of 3
context files.  It covers more wide range of failures.


Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Redeploy on context.xml changes (Was: Tagging 7.0.23)

Posted by Mark Thomas <ma...@apache.org>.
On 08/11/2011 22:49, Mark Thomas wrote:
> I found some time today to look at this. Summarising the issues so far:
> 
> 1. I have separately applied a couple of minor fixes that were included
> in the larger patch.
> 
> 2. Renaming watchedResources -> reloadResoucres
> Agree with reverting this change
> 
> 3. Adding JavaDoc to FailedContext
> Agree with doing this. I tweaked the wording a little bit as it might
> not be a StandardContext instance that we failed to create
> 
> 3. Formatting of FailedContext
> Given that the majority of the code is NO-OP and the rest is pretty much
> getters/setters the current formatting is deliberate to reduce the
> length of the file. I did something similar for WebXml.
> 
> 4. s/absolue/absolute/
> Drat. Missed this first time around. Fixed now.
> 
> 5. redeployResource has been implemented for FailedContext
> 
> 6. Unexpected deletion of resources due to adding conf/context.xml to
> redeployResources
> - removed redeployResources from Context
> - ensure global resources are never deleted
> - add global resources at end of redeploy list
> 
> 
> Particularly with the changes for 6, the patch should be easier to read.
> 
> The patch is here:
> http://people.apache.org/~markt/patches/2011-11-08-redeploy-trunk-v4.patch
> 
> There are currently a few open TC7 bugs so fixing those should provide
> enough time to review this patch before the 7.0.23 tag.

It has been a couple of days and no objections so I plan to apply this
patch in the next day or so and tag 7.0.23 early next week. That should
give me time to fix the remaining open bug and any unit test and/or TCK
failures that appear.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Redeploy on context.xml changes (Was: Tagging 7.0.23)

Posted by Mark Thomas <ma...@apache.org>.
I found some time today to look at this. Summarising the issues so far:

1. I have separately applied a couple of minor fixes that were included
in the larger patch.

2. Renaming watchedResources -> reloadResoucres
Agree with reverting this change

3. Adding JavaDoc to FailedContext
Agree with doing this. I tweaked the wording a little bit as it might
not be a StandardContext instance that we failed to create

3. Formatting of FailedContext
Given that the majority of the code is NO-OP and the rest is pretty much
getters/setters the current formatting is deliberate to reduce the
length of the file. I did something similar for WebXml.

4. s/absolue/absolute/
Drat. Missed this first time around. Fixed now.

5. redeployResource has been implemented for FailedContext

6. Unexpected deletion of resources due to adding conf/context.xml to
redeployResources
- removed redeployResources from Context
- ensure global resources are never deleted
- add global resources at end of redeploy list


Particularly with the changes for 6, the patch should be easier to read.

The patch is here:
http://people.apache.org/~markt/patches/2011-11-08-redeploy-trunk-v4.patch

There are currently a few open TC7 bugs so fixing those should provide
enough time to review this patch before the 7.0.23 tag.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Redeploy on context.xml changes (Was: Tagging 7.0.23)

Posted by Konstantin Kolinko <kn...@gmail.com>.
2011/11/6 Konstantin Kolinko <kn...@gmail.com>:
> 2011/11/6 Mark Thomas <ma...@apache.org>:
>>
>> I'm in two minds whether to apply this patch before or after the 7.0.23
>> tag. There are clearly issues here that need fixing but the patch is
>> quite invasive. I'm leaning towards tagging after the patch is applied.
>> Thoughts?
>>
>
> In short: I think there are two issues
> a) what FailedContext solves
> b) touching context.xml causing redeploy instead of reload
>
> I think a) needs to be solved asap and likely to be backported.

I mean, handling deployment failures.

Being stumbled here I agree that looks like it is better to cut 7.0.x
as is and take several days after that to though of a better solution.

> b) can be postponed.
>
> Regarding b) - remember that order of entries in redeploy list used by
> autodeploy check is very important (because files ##(n+1)..last are
> deleted if file #n is touched).  I am not yet sure that the patch is
> safe adding context.xml there.

There is StandardContext#setDefaultContextXml() method.
Maybe implement something like that in FailedContext.

In those places where you are replacing addWatchedResource() with
addRedeployResource() - maybe do not do such replacement.

Instead of that in HostConfig#addWatchedResources() or elsewhere use
the value of StandardContext#getDefaultContextXml() and add that file
to redeploy resources explicitly. Thusly we will have more precise
control on their order.

Maybe in HostConfig$DeployedApplication.redeployResources do store
explicit flag whether the resource is supposed to be deleted on
undeploy. (Instead of checking file paths before deleting the resource
in HostConfig#checkResources()).
Maybe I am just afraid of those path checks in #checkResources() and
adding a comment there can resolve it.


> ====================
> Regarding RequestFilterValve:

Question is solved. Updated the valve implementation earlier today and
merged to TC7.

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Redeploy on context.xml changes (Was: Tagging 7.0.23)

Posted by Konstantin Kolinko <kn...@gmail.com>.
2011/11/6 Mark Thomas <ma...@apache.org>:
>
> I'm in two minds whether to apply this patch before or after the 7.0.23
> tag. There are clearly issues here that need fixing but the patch is
> quite invasive. I'm leaning towards tagging after the patch is applied.
> Thoughts?
>

In short: I think there are two issues
a) what FailedContext solves
b) touching context.xml causing redeploy instead of reload

I think a) needs to be solved asap and likely to be backported.

b) can be postponed.

Regarding b) - remember that order of entries in redeploy list used by
autodeploy check is very important (because files ##(n+1)..last are
deleted if file #n is touched).  I am not yet sure that the patch is
safe adding context.xml there.

====================
Regarding RequestFilterValve:
It sounds like you have tested it with JMX.

If it throws LifecycleException from initInternal() are you able to
fix it with JMX later?

(Maybe "throw new LifecycleException" should be removed from
initInternal(), keeping in only in startInternal() ?  I do not mind to
keep the code as is now, but though maybe there is a reason to remove
the first throw).


Regarding RequestFilterValve (one more):

I thought that there could be a public method callable through JMX to
check whether allowed address is allowed or denied.
Something like "test(String)" or "isAllowed(String)".

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Redeploy on context.xml changes (Was: Tagging 7.0.23)

Posted by Mark Thomas <ma...@apache.org>.
On 05/11/2011 15:24, Konstantin Kolinko wrote:
> 2011/11/4 Mark Thomas <ma...@apache.org>:
>> On 03/11/2011 17:08, Mark Thomas wrote:
>>> I'm still looking at:
>>> - recovery after fixing the broken file
>>> - not deploying an expanded dir if the context.xml in
>>> conf/Catalina/localhost is broken.
>>
>> OK. Here is a patch for review. There are so many combinations
>> deployment and ways to create a dodgy configuration that I have probably
>> missed a couple of edge cases.
>>
>> The main idea with this patch is that a Context always gets added even
>> if deployment fails. That allows fixes via JMX and prevents
>> WAR/directory deployment after a failed descriptor deployment. If the
>> context.xml can't be read, a new FailedContext object is used. This
>> allows redeploy resources to be tracked but the Context cannot be started.
>>
>> A number of clean-up issues I spotted along the way have already been fixed.
>>
>> http://people.apache.org/~markt/patches/2011-11-03-redeploy-trunk-v2.patch
>>
> 
> http://people.apache.org/~kkolinko/patches/2011-11-05_tc8_redeploy-v3.patch
> 
> It is just your patch, but
> - reverted renames of "watched resource" -> "reload resource". (*)
> - formatted FailedContext.java and added javadoc at the top of it
> - corrected a typo s/absolue/absolute/
> 
> (*) Renaming does not add much, and so it is easier to backport. Let's
> do not rename WatchedResource -> ReloadResource.

Fair enough. Now we aren't exposing redeployResoucres to users there is
no real need for the renaming.

> As of now:
> 1) addReloadResource() is not implemented in FailedContext. So there
> is no way to add dependency on the global conf/context.xml there.
> 
> Possible solutions, but need to think about them
>  a) store reload resources elsewhere outside of Context
Possible since we no longer allow user configuration of this.

>  b) extract some code from StandardContext into a new class (something
> like StandardContextBase?) and share it with FailedContext?  I see
> that FailedContext does not inherit from ContainerBase, so maybe there
> is some trouble with that.
I wanted to ensure that FailedContext did the absolute minimum. Any of
those approaches could be made to work. I selected the implement the
interfaces directly approach as it is the simplest to review to ensure
it does what you want it to and no more.

>  c) Copy some code into FailedContext?
This would be my preferred choice.

>  d) Use StandardContext, but overwrite some methods?
See response to b)

> 2) It is odd that FailedContext has
>     @SuppressWarnings("unused")
>     public synchronized void addValve(Valve valve)
It needs to support addValve() (I forget what calls this) but it doesn't
need to keep a record of the Valves.

> 3) deployer-howto.xml still mentions "RedeployResource". I'd see where
> it goes and fix later.
> 
> PS: I had troubles applying your patch with svn and started a thread
> on users@subversion referring to it as an example. It does not handle
> the way git denotes added files. Not a big deal when there is only one
> added file - it us easy to handle that.
Thanks for the review so far. I'll update the patch.

I'm in two minds whether to apply this patch before or after the 7.0.23
tag. There are clearly issues here that need fixing but the patch is
quite invasive. I'm leaning towards tagging after the patch is applied.
Thoughts?

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Redeploy on context.xml changes (Was: Tagging 7.0.23)

Posted by Konstantin Kolinko <kn...@gmail.com>.
2011/11/4 Mark Thomas <ma...@apache.org>:
> On 03/11/2011 17:08, Mark Thomas wrote:
>> I'm still looking at:
>> - recovery after fixing the broken file
>> - not deploying an expanded dir if the context.xml in
>> conf/Catalina/localhost is broken.
>
> OK. Here is a patch for review. There are so many combinations
> deployment and ways to create a dodgy configuration that I have probably
> missed a couple of edge cases.
>
> The main idea with this patch is that a Context always gets added even
> if deployment fails. That allows fixes via JMX and prevents
> WAR/directory deployment after a failed descriptor deployment. If the
> context.xml can't be read, a new FailedContext object is used. This
> allows redeploy resources to be tracked but the Context cannot be started.
>
> A number of clean-up issues I spotted along the way have already been fixed.
>
> http://people.apache.org/~markt/patches/2011-11-03-redeploy-trunk-v2.patch
>

http://people.apache.org/~kkolinko/patches/2011-11-05_tc8_redeploy-v3.patch

It is just your patch, but
- reverted renames of "watched resource" -> "reload resource". (*)
- formatted FailedContext.java and added javadoc at the top of it
- corrected a typo s/absolue/absolute/

(*) Renaming does not add much, and so it is easier to backport. Let's
do not rename WatchedResource -> ReloadResource.


As of now:
1) addReloadResource() is not implemented in FailedContext. So there
is no way to add dependency on the global conf/context.xml there.

Possible solutions, but need to think about them
 a) store reload resources elsewhere outside of Context
 b) extract some code from StandardContext into a new class (something
like StandardContextBase?) and share it with FailedContext?  I see
that FailedContext does not inherit from ContainerBase, so maybe there
is some trouble with that.
 c) Copy some code into FailedContext?
 d) Use StandardContext, but overwrite some methods?

2) It is odd that FailedContext has
    @SuppressWarnings("unused")
    public synchronized void addValve(Valve valve)

3) deployer-howto.xml still mentions "RedeployResource". I'd see where
it goes and fix later.

PS: I had troubles applying your patch with svn and started a thread
on users@subversion referring to it as an example. It does not handle
the way git denotes added files. Not a big deal when there is only one
added file - it us easy to handle that.

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Redeploy on context.xml changes (Was: Tagging 7.0.23)

Posted by Mark Thomas <ma...@apache.org>.
On 03/11/2011 17:08, Mark Thomas wrote:
> I'm still looking at:
> - recovery after fixing the broken file
> - not deploying an expanded dir if the context.xml in
> conf/Catalina/localhost is broken.

OK. Here is a patch for review. There are so many combinations
deployment and ways to create a dodgy configuration that I have probably
missed a couple of edge cases.

The main idea with this patch is that a Context always gets added even
if deployment fails. That allows fixes via JMX and prevents
WAR/directory deployment after a failed descriptor deployment. If the
context.xml can't be read, a new FailedContext object is used. This
allows redeploy resources to be tracked but the Context cannot be started.

A number of clean-up issues I spotted along the way have already been fixed.

http://people.apache.org/~markt/patches/2011-11-03-redeploy-trunk-v2.patch

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Redeploy on context.xml changes (Was: Tagging 7.0.23)

Posted by Mark Thomas <ma...@apache.org>.
On 02/11/2011 15:35, Konstantin Kolinko wrote:
> 2011/11/2  <ma...@apache.org>:

> Also s/redploy/redeploy/ in some Javadoc.
Fixed for next iteration of the patch.

> The removeWatchedResource() method in the patch is NOOP instead of
> calling renamed method.
Also fixed.

> The following in StandardContext.java does not look backwards-compatible:
> -        fireContainerEvent("addWatchedResource", name);
> +        fireContainerEvent("addReloadResource", name);
> and
> -         fireContainerEvent("removeWatchedResource", name);
> +        fireContainerEvent("removeReloadResource", name);
> 
> Better keep the old name of "watched" and just add the new one of "redeploy".
> In all places the "redeploy" list is new and parallel to the old "watched" one.
Fixed too.

>> The one thing I don't like is that a change to conf/context.xml that
>> breaks the file effectively kills the instance until the problem is
>> fixed and the instance re-started.
> 
> Looking at HostConfig.checkResources(DeployedApplication) note several
> ExpandWar.delete() calls. The order is important there. If redeploy
> resource #i is missing or updated it deletes all following redeploy
> resources starting with #(i+1).
> 
> With this behaviour it is better not to let a user to control this
> "redeploy" list.
Fair point. I'll remove them from the docs, the digester and JMX.

> Regarding the original task of what happens when context.xml is
> touched or edited:  redeploying deletes the work dir, along with all
> persisted sessions and all compiled JSPs. (ContextConfig.destroy()
> triggered by Host.removeChild() does the deletion)
> 
> It is an enormous price to pay for editing an AccessLogValve
> configuration or whatever is important in context.xml. I'd prefer a
> solution that does not delete the work dir.

I do see where you are coming from here. There are times when it isn't
necessary to clean out the work dir or the sessions. However, figuring
out which is which is really tricky. I'm not even sure it is completely
possible. We need to use redeploy else the Context configuration won't
get re-read. I think the answer is to use parallel deployment if you
don't want to lose sessions etc although that doesn't help with the work
dir. There is always pre-compilation for that.

I don't think a one-size-fits-all solution is possible here but I think
we do have options that cover the key use cases.

I'm still looking at:
- recovery after fixing the broken file
- not deploying an expanded dir if the context.xml in
conf/Catalina/localhost is broken.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org