-
1. Re: Deployer chain cleanup
dmlloyd Nov 10, 2010 5:41 PM (in response to jason.greene)In an ideal world we'd be able to use an interceptor-chain kind of pattern where each processor calls invokeNext() or whatever, and any error cleanup could be done in a finally block. However the grim reality is that we could face stack exhaustion if we had a truly large number of extensions involved. I think the next-best option is what you suggest: a cleanup() method which is called if a later processor fails.
So to restate:
- Add a cleanup() method to DeploymentUnitProcessor
- Each implementation of DUP should handle failure cleanly, so processDeployment is essentially atomic: either everything gets done or nothing (no partial state when an exception is thrown).
- Then the implementation of cleanup() can perform a full cleanup safely, because it will only ever be called if processDeployment() had completed successfully and a later processDeployment() failed.
- This implies that if a given processDeployment() fails, its cleanup() is not called, but the cleanup() methods on all the DUPs before the one that failed are called.
Make sense?
-
2. Re: Deployer chain cleanup
dmlloyd Nov 10, 2010 5:48 PM (in response to dmlloyd)David Lloyd wrote:
I should elaborate here: the cleanup() methods should be called in reverse order, just like recursive catch/finally blocks would be.
-
3. Re: Deployer chain cleanup
brian.stansberry Nov 10, 2010 6:37 PM (in response to dmlloyd)I like the approach of each DUP being responsible for cleaning itself up if it fails, with the cleanup call being reserved for those that had previously completed successfully. That's the way the handling of lists of model/runtime updates work.
Putting the logic to do the cleanup work inside DeploymentChain.processDeployment() seems easiest -- no need to do any kind of external bookkeeping of what DUPs have been invoked and need cleanup. The contract for that method can guarantee that all cleanup will be done before any exception is thrown.
-
4. Re: Deployer chain cleanup
aloubyansky Nov 11, 2010 2:56 AM (in response to brian.stansberry)Adding a cleanup method to DUP is probably the most natural thing to do. The reason I didn't do it was I didn't want to go modify each DUP implementation.
Also I had an idea of doing something like this
{code}// this is the interface I'm currently using
public interface DeploymentRollbackAction {
void rollback(DeploymentUnitContext ctx);
}
public abstract class DeploymentUnitProcessorWithRollback implements DeploymentUnitProcessor, DeploymentRollbackAction {
public final void processDeployment(DeploymentUnitContext context) throws DeploymentUnitProcessingException {context.pushRollback(this);
doProcess(context);
}
protected abstract void doProcess(DeploymentUnitContext context) throws DeploymentUnitProcessingException;}{code}
So then DUP that may need cleaning up can extend this processor.
-
5. Re: Deployer chain cleanup
thomas.diesler Nov 12, 2010 3:48 PM (in response to dmlloyd)David, how many extensions do you think would be needed to reach stack exhaustion? Its only a relativly small frame you would need per DUP, no?
-
6. Re: Deployer chain cleanup
aloubyansky Nov 22, 2010 10:35 AM (in response to aloubyansky)Actually, having looked at the processor implementations, there are just a few that need to do clean up. For example, in case of managed beans it's 1 out of 12. So, apparently, it's not that common.
Another thought on this subject is that if we are to add some clean-up method then perhaps it makes sense to invoke it when the unit is undeployed. I.e. go through the same deployment chain and invoke clean-up on the processors.
-
7. Re: Deployer chain cleanup
dmlloyd Nov 22, 2010 6:12 PM (in response to thomas.diesler)Thomas Diesler wrote:
David, how many extensions do you think would be needed to reach stack exhaustion? Its only a relativly small frame you would need per DUP, no?
Yes, theoretically it's a small frame, but if we have hundreds of processors it can add up. If someone told me that they can successfully recurse, say, 2000 times with, say, 10 local reference-type variables per frame and they tested it on Windows, Mac, Linux (x86 and others) and Solaris with default VM options, then I might be persuaded though.
-
8. Re: Deployer chain cleanup
dmlloyd Nov 22, 2010 6:15 PM (in response to aloubyansky)Alexey Loubyansky wrote:
Actually, having looked at the processor implementations, there are just a few that need to do clean up. For example, in case of managed beans it's 1 out of 12. So, apparently, it's not that common.
Another thought on this subject is that if we are to add some clean-up method then perhaps it makes sense to invoke it when the unit is undeployed. I.e. go through the same deployment chain and invoke clean-up on the processors.
No, undeploy will be a different situation: the batch builders will be gone, for example, and so will much of the deploy-time metainformation. In addition, certain things like the deployment mount(s) will be associated with a service rather than the deployment unit processor context. Undeploy should consist of nothing more than iterating all services belonging to a deployment unit and setting their mode to "REMOVE", and then waiting for completion.
-
9. Re: Deployer chain cleanup
dmlloyd Dec 2, 2010 12:00 PM (in response to dmlloyd)David Lloyd wrote:
No, undeploy will be a different situation....
OK turns out I'm probably completely wrong about this. I'll follow up later with an email to the as7-dev list to explain...
-
10. Re: Deployer chain cleanup
dmlloyd Dec 2, 2010 3:24 PM (in response to dmlloyd)Further discussion is here: http://lists.jboss.org/pipermail/jboss-as7-dev/2010-December/000075.html