[xwiki-devs] [Proposal] Introduce new @DisposePriority annotation or new Disposable interface to fix XWiki shutdown
Hi devs, Right now there’s a problem when XWiki is shutdown. This is what currently happens: 2014-09-23 13:54:05,563 [Thread-1] DEBUG o.x.shutdown - Starting XWiki shutdown... 2014-09-23 13:54:05,563 [Thread-1] DEBUG o.x.shutdown - Stopping Database Connection Pool... 2014-09-23 13:54:05,735 [Thread-1] DEBUG o.x.shutdown - Database Connection Pool has been stopped 2014-09-23 13:54:10,513 [Thread-1] DEBUG o.x.shutdown - Stopping component [org.xwiki.localization.wiki.internal.DocumentTranslationBundleFactory]… 2014-09-23 13:54:10,513 [Thread-1] DEBUG o.x.shutdown - Component [org.xwiki.localization.wiki.internal.DocumentTranslationBundleFactory] has been stopped … 2014-09-23 13:54:10,514 [Thread-1] DEBUG o.x.shutdown - Stopping component [org.xwiki.search.solr.internal.DefaultSolrIndexer]... 2014-09-23 13:54:10,514 [Thread-1] DEBUG o.x.shutdown - Component [org.xwiki.search.solr.internal.DefaultSolrIndexer] has been stopped … 2014-09-23 13:54:10,539 [Thread-1] INFO o.x.shutdown - XWiki shutdown has been completed. As you can see the problem is that the DB is stopped first. So any code accessing the DB will get an error once the connection pool is stopped. This is why we get a lot stack traces when you ctrl-c XWiki quickly after it starts (i.e. when the init is not finished or when SOLR is still indexing). You can also get stack traces if a watchlist is being sent or some scheduler job is executing for example. There are 2 ways to fix this: - solution 1: all code that accesses the DB could check if an XWiki shutdown is in progress and don’t generate stack traces if this is the case - solution 2: stop the DB as the last thing, after everything else has stopped Obviously solution 2 is much better. I’ve started implementing it by doing this: - Remove the current HibernateShutdownEventListener since this is executed first during shutdown, before component disposal - Instead have XWikiHibernateStore implement Disposable and move the content of HibernateShutdownEventListener in that method This works already better but it’s not enough since ECM.dispose() works out the component dependencies but this works only for explicit dependencies and doesn’t work if a component uses the ComponentManager to dynamically find implementations (as this is the case for example for DefaultSolrIndexer). Thus in order to help the component disposal process, I propose to introduce a new annotation: @DisposePriority(int). Note that an alternative would be to introduce a new Disposable interface with a new getPriority() method in it (we cannot modify the current Disposable since that would break backward compat). The idea is then to use this information in ECM.dispose() to first sort all components based on this and then only to order them with their dependencies. WDYT? Thanks -Vincent
On Tue, Sep 23, 2014 at 11:48 PM, [email protected] <[email protected]> wrote:
Hi devs,
Right now there’s a problem when XWiki is shutdown. This is what currently happens:
2014-09-23 13:54:05,563 [Thread-1] DEBUG o.x.shutdown - Starting XWiki shutdown... 2014-09-23 13:54:05,563 [Thread-1] DEBUG o.x.shutdown - Stopping Database Connection Pool... 2014-09-23 13:54:05,735 [Thread-1] DEBUG o.x.shutdown - Database Connection Pool has been stopped 2014-09-23 13:54:10,513 [Thread-1] DEBUG o.x.shutdown - Stopping component [org.xwiki.localization.wiki.internal.DocumentTranslationBundleFactory]… 2014-09-23 13:54:10,513 [Thread-1] DEBUG o.x.shutdown - Component [org.xwiki.localization.wiki.internal.DocumentTranslationBundleFactory] has been stopped … 2014-09-23 13:54:10,514 [Thread-1] DEBUG o.x.shutdown - Stopping component [org.xwiki.search.solr.internal.DefaultSolrIndexer]... 2014-09-23 13:54:10,514 [Thread-1] DEBUG o.x.shutdown - Component [org.xwiki.search.solr.internal.DefaultSolrIndexer] has been stopped … 2014-09-23 13:54:10,539 [Thread-1] INFO o.x.shutdown - XWiki shutdown has been completed.
As you can see the problem is that the DB is stopped first. So any code accessing the DB will get an error once the connection pool is stopped. This is why we get a lot stack traces when you ctrl-c XWiki quickly after it starts (i.e. when the init is not finished or when SOLR is still indexing). You can also get stack traces if a watchlist is being sent or some scheduler job is executing for example.
There are 2 ways to fix this: - solution 1: all code that accesses the DB could check if an XWiki shutdown is in progress and don’t generate stack traces if this is the case - solution 2: stop the DB as the last thing, after everything else has stopped
Obviously solution 2 is much better.
I’ve started implementing it by doing this: - Remove the current HibernateShutdownEventListener since this is executed first during shutdown, before component disposal - Instead have XWikiHibernateStore implement Disposable and move the content of HibernateShutdownEventListener in that method
This works already better but it’s not enough since ECM.dispose() works out the component dependencies but this works only for explicit dependencies and doesn’t work if a component uses the ComponentManager to dynamically find implementations (as this is the case for example for DefaultSolrIndexer).
Thus in order to help the component disposal process, I propose to introduce a new annotation: @DisposePriority(int).
Note that an alternative would be to introduce a new Disposable interface with a new getPriority() method in it (we cannot modify the current Disposable since that would break backward compat).
It's always hard to know what priority to put exactly so this is last resort solution for me. A few other alternatives: 1) stop the DB after disposing components (I would not vote for it I think but it should work for this use case) 2) introduce something like a @ComponentDependency annotation to let component declare indirect dependencies (ECM#dispose would simply add @ComponentDependency annotations to its current dependencies based ordering) and: 2.1) even when a component does not directly use XWikiStoreInterface (pretty much all of them are using XWiki methods and don't even know about it officially) we could still ask them to declare something like @ComponentDependency(XWikiStoreInterface.class) to indicate that they want to be disposed after any component of this role. 2.2) make XWiki instance a component instead of storing it in the servlet context and inject it instead of accessing it through XWikiContext (XWiki itself would inject the component it want). XWiki itself would declare a @ComponentDependency(XWikiStoreInterface.class). 2.3) move all storage related methods from XWiki (saveDocument, getDocument, etc) to another component (and remove XWikiContext parameter to all these methods) which would be injected by components directly instead of searching for the XWikiContext (that's how new model will work but we can have another one before that still expose olcore APIs like XWikiDocument). Then this component would take care of declaring the @ComponentDependency(XWikiStoreInterface.class) (it's not really possible to directly inject the right XWikiStoreInterface since it's configuration based)
The idea is then to use this information in ECM.dispose() to first sort all components based on this and then only to order them with their dependencies.
WDYT?
Thanks -Vincent
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
I prefer any of the 2.x over priority based ordering and I think the nicest for components is 2.3. -- Thomas Mortagne
Hi Thomas, On 24 Sep 2014 at 08:58:56, Thomas Mortagne ([email protected](mailto:[email protected])) wrote:
On Tue, Sep 23, 2014 at 11:48 PM, [email protected] wrote:
Hi devs,
Right now there’s a problem when XWiki is shutdown. This is what currently happens:
2014-09-23 13:54:05,563 [Thread-1] DEBUG o.x.shutdown - Starting XWiki shutdown... 2014-09-23 13:54:05,563 [Thread-1] DEBUG o.x.shutdown - Stopping Database Connection Pool... 2014-09-23 13:54:05,735 [Thread-1] DEBUG o.x.shutdown - Database Connection Pool has been stopped 2014-09-23 13:54:10,513 [Thread-1] DEBUG o.x.shutdown - Stopping component [org.xwiki.localization.wiki.internal.DocumentTranslationBundleFactory]… 2014-09-23 13:54:10,513 [Thread-1] DEBUG o.x.shutdown - Component [org.xwiki.localization.wiki.internal.DocumentTranslationBundleFactory] has been stopped … 2014-09-23 13:54:10,514 [Thread-1] DEBUG o.x.shutdown - Stopping component [org.xwiki.search.solr.internal.DefaultSolrIndexer]... 2014-09-23 13:54:10,514 [Thread-1] DEBUG o.x.shutdown - Component [org.xwiki.search.solr.internal.DefaultSolrIndexer] has been stopped … 2014-09-23 13:54:10,539 [Thread-1] INFO o.x.shutdown - XWiki shutdown has been completed.
As you can see the problem is that the DB is stopped first. So any code accessing the DB will get an error once the connection pool is stopped. This is why we get a lot stack traces when you ctrl-c XWiki quickly after it starts (i.e. when the init is not finished or when SOLR is still indexing). You can also get stack traces if a watchlist is being sent or some scheduler job is executing for example.
There are 2 ways to fix this: - solution 1: all code that accesses the DB could check if an XWiki shutdown is in progress and don’t generate stack traces if this is the case - solution 2: stop the DB as the last thing, after everything else has stopped
Obviously solution 2 is much better.
I’ve started implementing it by doing this: - Remove the current HibernateShutdownEventListener since this is executed first during shutdown, before component disposal - Instead have XWikiHibernateStore implement Disposable and move the content of HibernateShutdownEventListener in that method
This works already better but it’s not enough since ECM.dispose() works out the component dependencies but this works only for explicit dependencies and doesn’t work if a component uses the ComponentManager to dynamically find implementations (as this is the case for example for DefaultSolrIndexer).
Thus in order to help the component disposal process, I propose to introduce a new annotation: @DisposePriority(int).
Note that an alternative would be to introduce a new Disposable interface with a new getPriority() method in it (we cannot modify the current Disposable since that would break backward compat).
It's always hard to know what priority to put exactly so this is last resort solution for me.
The idea is to use this only for special cases (like DB shutdown) since in the majority of cases the component dependency order would be enough. So in these few cases it’s quite easy and similar to Macro priority (which hasn’t proved to be a problem so far and is working well).
A few other alternatives: 1) stop the DB after disposing components (I would not vote for it I think but it should work for this use case)
Yes I thought about this but I wanted something a bit more generic. See at the end of this email though.
2) introduce something like a @ComponentDependency annotation to let component declare indirect dependencies (ECM#dispose would simply add @ComponentDependency annotations to its current dependencies based ordering) and: 2.1) even when a component does not directly use XWikiStoreInterface (pretty much all of them are using XWiki methods and don't even know about it officially) we could still ask them to declare something like @ComponentDependency(XWikiStoreInterface.class) to indicate that they want to be disposed after any component of this role.
Yes I had also thought about this one but discarded because it means finding all components using indirectly XWikiStoreInterface and modifying them all. This means a lot of places (since a lot of places use the XWiki class for example) and a lot of work. I also find it a lot more complex to implement than a priority order which seems really the most simple solution.
2.2) make XWiki instance a component instead of storing it in the servlet context and inject it instead of accessing it through XWikiContext (XWiki itself would inject the component it want). XWiki itself would declare a @ComponentDependency(XWikiStoreInterface.class). 2.3) move all storage related methods from XWiki (saveDocument, getDocument, etc) to another component (and remove XWikiContext parameter to all these methods) which would be injected by components directly instead of searching for the XWikiContext (that's how new model will work but we can have another one before that still expose olcore APIs like XWikiDocument). Then this component would take care of declaring the @ComponentDependency(XWikiStoreInterface.class) (it's not really possible to directly inject the right XWikiStoreInterface since it's configuration based)
These 2 alternatives are ok but they are a lot of work and could be done in the future. I was trying to find a solution that can be applied in 1/2 day right now. We need to fix our bug now and I fear we’ll just not do anything if we wait for 2.2 and 2.3 (which we can do on the longer run). Since you don’t agree about priority and since I’d like to fix the shutdown “bug” for 6.2.x and on 6.3M1 now, I’m proposing the following alternative for now: * Introduce a new constructor in the ApplicationStoppedEvent class: ApplicationStoppedEvent(boolean afterDispose) * Modify XWikiServletContextListener.contextDestroyed() so that it sends the ApplicationStoppedEvent(false) before calling cm.dispose() and it sends the ApplicationStoppedEvent(true) after calling cm.dispose() * Keep HibernateShutdownEventListener but make it listen to ApplicationStoppedEvent(true) This will ensure it’s called after all components have been disposed. WDYT? Acceptable? Thanks -Vincent
The idea is then to use this information in ECM.dispose() to first sort all components based on this and then only to order them with their dependencies.
WDYT?
Thanks -Vincent
On Wed, Sep 24, 2014 at 9:23 AM, [email protected] <[email protected]> wrote:
Hi Thomas,
On 24 Sep 2014 at 08:58:56, Thomas Mortagne ([email protected](mailto:[email protected])) wrote:
On Tue, Sep 23, 2014 at 11:48 PM, [email protected] wrote:
Hi devs,
Right now there’s a problem when XWiki is shutdown. This is what currently happens:
2014-09-23 13:54:05,563 [Thread-1] DEBUG o.x.shutdown - Starting XWiki shutdown... 2014-09-23 13:54:05,563 [Thread-1] DEBUG o.x.shutdown - Stopping Database Connection Pool... 2014-09-23 13:54:05,735 [Thread-1] DEBUG o.x.shutdown - Database Connection Pool has been stopped 2014-09-23 13:54:10,513 [Thread-1] DEBUG o.x.shutdown - Stopping component [org.xwiki.localization.wiki.internal.DocumentTranslationBundleFactory]… 2014-09-23 13:54:10,513 [Thread-1] DEBUG o.x.shutdown - Component [org.xwiki.localization.wiki.internal.DocumentTranslationBundleFactory] has been stopped … 2014-09-23 13:54:10,514 [Thread-1] DEBUG o.x.shutdown - Stopping component [org.xwiki.search.solr.internal.DefaultSolrIndexer]... 2014-09-23 13:54:10,514 [Thread-1] DEBUG o.x.shutdown - Component [org.xwiki.search.solr.internal.DefaultSolrIndexer] has been stopped … 2014-09-23 13:54:10,539 [Thread-1] INFO o.x.shutdown - XWiki shutdown has been completed.
As you can see the problem is that the DB is stopped first. So any code accessing the DB will get an error once the connection pool is stopped. This is why we get a lot stack traces when you ctrl-c XWiki quickly after it starts (i.e. when the init is not finished or when SOLR is still indexing). You can also get stack traces if a watchlist is being sent or some scheduler job is executing for example.
There are 2 ways to fix this: - solution 1: all code that accesses the DB could check if an XWiki shutdown is in progress and don’t generate stack traces if this is the case - solution 2: stop the DB as the last thing, after everything else has stopped
Obviously solution 2 is much better.
I’ve started implementing it by doing this: - Remove the current HibernateShutdownEventListener since this is executed first during shutdown, before component disposal - Instead have XWikiHibernateStore implement Disposable and move the content of HibernateShutdownEventListener in that method
This works already better but it’s not enough since ECM.dispose() works out the component dependencies but this works only for explicit dependencies and doesn’t work if a component uses the ComponentManager to dynamically find implementations (as this is the case for example for DefaultSolrIndexer).
Thus in order to help the component disposal process, I propose to introduce a new annotation: @DisposePriority(int).
Note that an alternative would be to introduce a new Disposable interface with a new getPriority() method in it (we cannot modify the current Disposable since that would break backward compat).
It's always hard to know what priority to put exactly so this is last resort solution for me.
The idea is to use this only for special cases (like DB shutdown) since in the majority of cases the component dependency order would be enough. So in these few cases it’s quite easy and similar to Macro priority (which hasn’t proved to be a problem so far and is working well).
A few other alternatives: 1) stop the DB after disposing components (I would not vote for it I think but it should work for this use case)
Yes I thought about this but I wanted something a bit more generic. See at the end of this email though.
2) introduce something like a @ComponentDependency annotation to let component declare indirect dependencies (ECM#dispose would simply add @ComponentDependency annotations to its current dependencies based ordering) and: 2.1) even when a component does not directly use XWikiStoreInterface (pretty much all of them are using XWiki methods and don't even know about it officially) we could still ask them to declare something like @ComponentDependency(XWikiStoreInterface.class) to indicate that they want to be disposed after any component of this role.
Yes I had also thought about this one but discarded because it means finding all components using indirectly XWikiStoreInterface and modifying them all. This means a lot of places (since a lot of places use the XWiki class for example) and a lot of work.
I also find it a lot more complex to implement than a priority order which seems really the most simple solution.
The dependencies based ordering is already what we have and 2) is just about adding more depdencies. The first need is to add the annotation in the few places where we have the issue (only SOLR right now AFAIK) so no it does not really mean "finding all components using indirectly XWikiStoreInterface and modifying them all" right away.
2.2) make XWiki instance a component instead of storing it in the servlet context and inject it instead of accessing it through XWikiContext (XWiki itself would inject the component it want). XWiki itself would declare a @ComponentDependency(XWikiStoreInterface.class). 2.3) move all storage related methods from XWiki (saveDocument, getDocument, etc) to another component (and remove XWikiContext parameter to all these methods) which would be injected by components directly instead of searching for the XWikiContext (that's how new model will work but we can have another one before that still expose olcore APIs like XWikiDocument). Then this component would take care of declaring the @ComponentDependency(XWikiStoreInterface.class) (it's not really possible to directly inject the right XWikiStoreInterface since it's configuration based)
These 2 alternatives are ok but they are a lot of work and could be done in the future. I was trying to find a solution that can be applied in 1/2 day right now. We need to fix our bug now and I fear we’ll just not do anything if we wait for 2.2 and 2.3 (which we can do on the longer run).
2.3 is mostly copy pasting and start using the new component in the few places that have ordering issue (pretty much only SOLR right now). Changing other places is not a priority, it's not like we were refactoring the whole code each time we introduce a new API.
Since you don’t agree about priority and since I’d like to fix the shutdown “bug” for 6.2.x and on 6.3M1 now, I’m proposing the following alternative for now:
* Introduce a new constructor in the ApplicationStoppedEvent class: ApplicationStoppedEvent(boolean afterDispose) * Modify XWikiServletContextListener.contextDestroyed() so that it sends the ApplicationStoppedEvent(false) before calling cm.dispose() and it sends the ApplicationStoppedEvent(true) after calling cm.dispose() * Keep HibernateShutdownEventListener but make it listen to ApplicationStoppedEvent(true)
This will ensure it’s called after all components have been disposed.
I doubt that would work. If the CM is disposed HibernateShutdownEventListener won't exist anymore (and I'm not talking about using the observation manager which is going to be disposed officially, it's not doing anything special when disposed right now but it might).
WDYT? Acceptable?
Thanks -Vincent
The idea is then to use this information in ECM.dispose() to first sort all components based on this and then only to order them with their dependencies.
WDYT?
Thanks -Vincent
devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne
Ok, so after some discussion with Thomas: * I agree that solution 2.3 is nice and we should probably go towards it in the future * I need a quick solution that can be committed for 6.2.1 As a conclusion I’ll implement the @DisposePriority annotation for now (it’s simple to do, I have it done actually). Then in the future we’ll be able to either keep it or deprecate it in favor of a better solution. Thanks -Vincent On 24 Sep 2014 at 09:46:38, Thomas Mortagne ([email protected](mailto:[email protected])) wrote:
On Wed, Sep 24, 2014 at 9:23 AM, [email protected] wrote:
Hi Thomas,
On 24 Sep 2014 at 08:58:56, Thomas Mortagne ([email protected](mailto:[email protected])) wrote:
On Tue, Sep 23, 2014 at 11:48 PM, [email protected] wrote:
Hi devs,
Right now there’s a problem when XWiki is shutdown. This is what currently happens:
2014-09-23 13:54:05,563 [Thread-1] DEBUG o.x.shutdown - Starting XWiki shutdown... 2014-09-23 13:54:05,563 [Thread-1] DEBUG o.x.shutdown - Stopping Database Connection Pool... 2014-09-23 13:54:05,735 [Thread-1] DEBUG o.x.shutdown - Database Connection Pool has been stopped 2014-09-23 13:54:10,513 [Thread-1] DEBUG o.x.shutdown - Stopping component [org.xwiki.localization.wiki.internal.DocumentTranslationBundleFactory]… 2014-09-23 13:54:10,513 [Thread-1] DEBUG o.x.shutdown - Component [org.xwiki.localization.wiki.internal.DocumentTranslationBundleFactory] has been stopped … 2014-09-23 13:54:10,514 [Thread-1] DEBUG o.x.shutdown - Stopping component [org.xwiki.search.solr.internal.DefaultSolrIndexer]... 2014-09-23 13:54:10,514 [Thread-1] DEBUG o.x.shutdown - Component [org.xwiki.search.solr.internal.DefaultSolrIndexer] has been stopped … 2014-09-23 13:54:10,539 [Thread-1] INFO o.x.shutdown - XWiki shutdown has been completed.
As you can see the problem is that the DB is stopped first. So any code accessing the DB will get an error once the connection pool is stopped. This is why we get a lot stack traces when you ctrl-c XWiki quickly after it starts (i.e. when the init is not finished or when SOLR is still indexing). You can also get stack traces if a watchlist is being sent or some scheduler job is executing for example.
There are 2 ways to fix this: - solution 1: all code that accesses the DB could check if an XWiki shutdown is in progress and don’t generate stack traces if this is the case - solution 2: stop the DB as the last thing, after everything else has stopped
Obviously solution 2 is much better.
I’ve started implementing it by doing this: - Remove the current HibernateShutdownEventListener since this is executed first during shutdown, before component disposal - Instead have XWikiHibernateStore implement Disposable and move the content of HibernateShutdownEventListener in that method
This works already better but it’s not enough since ECM.dispose() works out the component dependencies but this works only for explicit dependencies and doesn’t work if a component uses the ComponentManager to dynamically find implementations (as this is the case for example for DefaultSolrIndexer).
Thus in order to help the component disposal process, I propose to introduce a new annotation: @DisposePriority(int).
Note that an alternative would be to introduce a new Disposable interface with a new getPriority() method in it (we cannot modify the current Disposable since that would break backward compat).
It's always hard to know what priority to put exactly so this is last resort solution for me.
The idea is to use this only for special cases (like DB shutdown) since in the majority of cases the component dependency order would be enough. So in these few cases it’s quite easy and similar to Macro priority (which hasn’t proved to be a problem so far and is working well).
A few other alternatives: 1) stop the DB after disposing components (I would not vote for it I think but it should work for this use case)
Yes I thought about this but I wanted something a bit more generic. See at the end of this email though.
2) introduce something like a @ComponentDependency annotation to let component declare indirect dependencies (ECM#dispose would simply add @ComponentDependency annotations to its current dependencies based ordering) and: 2.1) even when a component does not directly use XWikiStoreInterface (pretty much all of them are using XWiki methods and don't even know about it officially) we could still ask them to declare something like @ComponentDependency(XWikiStoreInterface.class) to indicate that they want to be disposed after any component of this role.
Yes I had also thought about this one but discarded because it means finding all components using indirectly XWikiStoreInterface and modifying them all. This means a lot of places (since a lot of places use the XWiki class for example) and a lot of work.
I also find it a lot more complex to implement than a priority order which seems really the most simple solution.
The dependencies based ordering is already what we have and 2) is just about adding more depdencies.
The first need is to add the annotation in the few places where we have the issue (only SOLR right now AFAIK) so no it does not really mean "finding all components using indirectly XWikiStoreInterface and modifying them all" right away.
2.2) make XWiki instance a component instead of storing it in the servlet context and inject it instead of accessing it through XWikiContext (XWiki itself would inject the component it want). XWiki itself would declare a @ComponentDependency(XWikiStoreInterface.class). 2.3) move all storage related methods from XWiki (saveDocument, getDocument, etc) to another component (and remove XWikiContext parameter to all these methods) which would be injected by components directly instead of searching for the XWikiContext (that's how new model will work but we can have another one before that still expose olcore APIs like XWikiDocument). Then this component would take care of declaring the @ComponentDependency(XWikiStoreInterface.class) (it's not really possible to directly inject the right XWikiStoreInterface since it's configuration based)
These 2 alternatives are ok but they are a lot of work and could be done in the future. I was trying to find a solution that can be applied in 1/2 day right now. We need to fix our bug now and I fear we’ll just not do anything if we wait for 2.2 and 2.3 (which we can do on the longer run).
2.3 is mostly copy pasting and start using the new component in the few places that have ordering issue (pretty much only SOLR right now). Changing other places is not a priority, it's not like we were refactoring the whole code each time we introduce a new API.
Since you don’t agree about priority and since I’d like to fix the shutdown “bug” for 6.2.x and on 6.3M1 now, I’m proposing the following alternative for now:
* Introduce a new constructor in the ApplicationStoppedEvent class: ApplicationStoppedEvent(boolean afterDispose) * Modify XWikiServletContextListener.contextDestroyed() so that it sends the ApplicationStoppedEvent(false) before calling cm.dispose() and it sends the ApplicationStoppedEvent(true) after calling cm.dispose() * Keep HibernateShutdownEventListener but make it listen to ApplicationStoppedEvent(true)
This will ensure it’s called after all components have been disposed.
I doubt that would work. If the CM is disposed HibernateShutdownEventListener won't exist anymore (and I'm not talking about using the observation manager which is going to be disposed officially, it's not doing anything special when disposed right now but it might).
WDYT? Acceptable?
Thanks -Vincent
The idea is then to use this information in ECM.dispose() to first sort all components based on this and then only to order them with their dependencies.
WDYT?
Thanks -Vincent
devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
@ComponentDependency is bad. We already declare dependencies with @Inject, and that should be enough for the component manager to know (transitively) which components it needs to instantiate. The CM is already capable of creating the components in the right order, it should be able to do the same thing in reverse, making sure that all the dependencies of a component are destroyed before that component. If some class needs @ComponentDependency, then there's another problem in the code. We should not have hidden dependencies that must somehow made explicit. Same thing about @DisposePriority, the components form a directed acyclic graph, it should be easy to find the leafs and dispose of components starting from those. The real problem is that some components use oldcore APIs that bypass the component manager, and we should fix that instead of using workarounds. If you add code knowing that pretty soon you'll have to deprecate it, you're just preparing more headache for yourself and the rest of the community. We don't really get rid of old APIs, we kind of have to maintain them for ever, and it sucks that even the "new, clean" components are littered with deprecated code. On 09/24/2014 07:40 AM, [email protected] wrote:
Ok, so after some discussion with Thomas:
* I agree that solution 2.3 is nice and we should probably go towards it in the future * I need a quick solution that can be committed for 6.2.1
As a conclusion I’ll implement the @DisposePriority annotation for now (it’s simple to do, I have it done actually).
Then in the future we’ll be able to either keep it or deprecate it in favor of a better solution.
Thanks -Vincent
On 24 Sep 2014 at 09:46:38, Thomas Mortagne ([email protected](mailto:[email protected])) wrote:
On Wed, Sep 24, 2014 at 9:23 AM, [email protected] wrote:
Hi Thomas,
On 24 Sep 2014 at 08:58:56, Thomas Mortagne ([email protected](mailto:[email protected])) wrote:
On Tue, Sep 23, 2014 at 11:48 PM, [email protected] wrote:
Hi devs,
Right now there’s a problem when XWiki is shutdown. This is what currently happens:
2014-09-23 13:54:05,563 [Thread-1] DEBUG o.x.shutdown - Starting XWiki shutdown... 2014-09-23 13:54:05,563 [Thread-1] DEBUG o.x.shutdown - Stopping Database Connection Pool... 2014-09-23 13:54:05,735 [Thread-1] DEBUG o.x.shutdown - Database Connection Pool has been stopped 2014-09-23 13:54:10,513 [Thread-1] DEBUG o.x.shutdown - Stopping component [org.xwiki.localization.wiki.internal.DocumentTranslationBundleFactory]… 2014-09-23 13:54:10,513 [Thread-1] DEBUG o.x.shutdown - Component [org.xwiki.localization.wiki.internal.DocumentTranslationBundleFactory] has been stopped … 2014-09-23 13:54:10,514 [Thread-1] DEBUG o.x.shutdown - Stopping component [org.xwiki.search.solr.internal.DefaultSolrIndexer]... 2014-09-23 13:54:10,514 [Thread-1] DEBUG o.x.shutdown - Component [org.xwiki.search.solr.internal.DefaultSolrIndexer] has been stopped … 2014-09-23 13:54:10,539 [Thread-1] INFO o.x.shutdown - XWiki shutdown has been completed.
As you can see the problem is that the DB is stopped first. So any code accessing the DB will get an error once the connection pool is stopped. This is why we get a lot stack traces when you ctrl-c XWiki quickly after it starts (i.e. when the init is not finished or when SOLR is still indexing). You can also get stack traces if a watchlist is being sent or some scheduler job is executing for example.
There are 2 ways to fix this: - solution 1: all code that accesses the DB could check if an XWiki shutdown is in progress and don’t generate stack traces if this is the case - solution 2: stop the DB as the last thing, after everything else has stopped
Obviously solution 2 is much better.
I’ve started implementing it by doing this: - Remove the current HibernateShutdownEventListener since this is executed first during shutdown, before component disposal - Instead have XWikiHibernateStore implement Disposable and move the content of HibernateShutdownEventListener in that method
This works already better but it’s not enough since ECM.dispose() works out the component dependencies but this works only for explicit dependencies and doesn’t work if a component uses the ComponentManager to dynamically find implementations (as this is the case for example for DefaultSolrIndexer).
Thus in order to help the component disposal process, I propose to introduce a new annotation: @DisposePriority(int).
Note that an alternative would be to introduce a new Disposable interface with a new getPriority() method in it (we cannot modify the current Disposable since that would break backward compat).
It's always hard to know what priority to put exactly so this is last resort solution for me.
The idea is to use this only for special cases (like DB shutdown) since in the majority of cases the component dependency order would be enough. So in these few cases it’s quite easy and similar to Macro priority (which hasn’t proved to be a problem so far and is working well).
A few other alternatives: 1) stop the DB after disposing components (I would not vote for it I think but it should work for this use case)
Yes I thought about this but I wanted something a bit more generic. See at the end of this email though.
2) introduce something like a @ComponentDependency annotation to let component declare indirect dependencies (ECM#dispose would simply add @ComponentDependency annotations to its current dependencies based ordering) and: 2.1) even when a component does not directly use XWikiStoreInterface (pretty much all of them are using XWiki methods and don't even know about it officially) we could still ask them to declare something like @ComponentDependency(XWikiStoreInterface.class) to indicate that they want to be disposed after any component of this role.
Yes I had also thought about this one but discarded because it means finding all components using indirectly XWikiStoreInterface and modifying them all. This means a lot of places (since a lot of places use the XWiki class for example) and a lot of work.
I also find it a lot more complex to implement than a priority order which seems really the most simple solution.
The dependencies based ordering is already what we have and 2) is just about adding more depdencies.
The first need is to add the annotation in the few places where we have the issue (only SOLR right now AFAIK) so no it does not really mean "finding all components using indirectly XWikiStoreInterface and modifying them all" right away.
2.2) make XWiki instance a component instead of storing it in the servlet context and inject it instead of accessing it through XWikiContext (XWiki itself would inject the component it want). XWiki itself would declare a @ComponentDependency(XWikiStoreInterface.class). 2.3) move all storage related methods from XWiki (saveDocument, getDocument, etc) to another component (and remove XWikiContext parameter to all these methods) which would be injected by components directly instead of searching for the XWikiContext (that's how new model will work but we can have another one before that still expose olcore APIs like XWikiDocument). Then this component would take care of declaring the @ComponentDependency(XWikiStoreInterface.class) (it's not really possible to directly inject the right XWikiStoreInterface since it's configuration based)
These 2 alternatives are ok but they are a lot of work and could be done in the future. I was trying to find a solution that can be applied in 1/2 day right now. We need to fix our bug now and I fear we’ll just not do anything if we wait for 2.2 and 2.3 (which we can do on the longer run).
2.3 is mostly copy pasting and start using the new component in the few places that have ordering issue (pretty much only SOLR right now). Changing other places is not a priority, it's not like we were refactoring the whole code each time we introduce a new API.
Since you don’t agree about priority and since I’d like to fix the shutdown “bug” for 6.2.x and on 6.3M1 now, I’m proposing the following alternative for now:
* Introduce a new constructor in the ApplicationStoppedEvent class: ApplicationStoppedEvent(boolean afterDispose) * Modify XWikiServletContextListener.contextDestroyed() so that it sends the ApplicationStoppedEvent(false) before calling cm.dispose() and it sends the ApplicationStoppedEvent(true) after calling cm.dispose() * Keep HibernateShutdownEventListener but make it listen to ApplicationStoppedEvent(true)
This will ensure it’s called after all components have been disposed.
I doubt that would work. If the CM is disposed HibernateShutdownEventListener won't exist anymore (and I'm not talking about using the observation manager which is going to be disposed officially, it's not doing anything special when disposed right now but it might).
WDYT? Acceptable?
Thanks -Vincent
The idea is then to use this information in ECM.dispose() to first sort all components based on this and then only to order them with their dependencies.
WDYT?
Thanks -Vincent
-- Sergiu Dumitriu http://purl.org/net/sergiu
On 24 Sep 2014 at 16:24:16, Sergiu Dumitriu ([email protected](mailto:[email protected])) wrote:
@ComponentDependency is bad. We already declare dependencies with @Inject, and that should be enough for the component manager to know (transitively) which components it needs to instantiate. The CM is already capable of creating the components in the right order, it should be able to do the same thing in reverse, making sure that all the dependencies of a component are destroyed before that component.
Not quite as I mentioned in my first email. A lot of code uses @Inject ComponentManager and dynamically do lookups. And that’s not counting with all the code that don’t use components such as the XWiki class which itself uses dependencies but since it’s not a component itself it breaks the chain… Thanks -Vincent
If some class needs @ComponentDependency, then there's another problem in the code. We should not have hidden dependencies that must somehow made explicit.
Same thing about @DisposePriority, the components form a directed acyclic graph, it should be easy to find the leafs and dispose of components starting from those.
The real problem is that some components use oldcore APIs that bypass the component manager, and we should fix that instead of using workarounds.
If you add code knowing that pretty soon you'll have to deprecate it, you're just preparing more headache for yourself and the rest of the community. We don't really get rid of old APIs, we kind of have to maintain them for ever, and it sucks that even the "new, clean" components are littered with deprecated code.
On 09/24/2014 07:40 AM, [email protected] wrote:
Ok, so after some discussion with Thomas:
* I agree that solution 2.3 is nice and we should probably go towards it in the future * I need a quick solution that can be committed for 6.2.1
As a conclusion I’ll implement the @DisposePriority annotation for now (it’s simple to do, I have it done actually).
Then in the future we’ll be able to either keep it or deprecate it in favor of a better solution.
Thanks -Vincent
On 24 Sep 2014 at 09:46:38, Thomas Mortagne ([email protected](mailto:[email protected])) wrote:
On Wed, Sep 24, 2014 at 9:23 AM, [email protected] wrote:
Hi Thomas,
On 24 Sep 2014 at 08:58:56, Thomas Mortagne ([email protected](mailto:[email protected])) wrote:
On Tue, Sep 23, 2014 at 11:48 PM, [email protected] wrote:
Hi devs,
Right now there’s a problem when XWiki is shutdown. This is what currently happens:
2014-09-23 13:54:05,563 [Thread-1] DEBUG o.x.shutdown - Starting XWiki shutdown... 2014-09-23 13:54:05,563 [Thread-1] DEBUG o.x.shutdown - Stopping Database Connection Pool... 2014-09-23 13:54:05,735 [Thread-1] DEBUG o.x.shutdown - Database Connection Pool has been stopped 2014-09-23 13:54:10,513 [Thread-1] DEBUG o.x.shutdown - Stopping component [org.xwiki.localization.wiki.internal.DocumentTranslationBundleFactory]… 2014-09-23 13:54:10,513 [Thread-1] DEBUG o.x.shutdown - Component [org.xwiki.localization.wiki.internal.DocumentTranslationBundleFactory] has been stopped … 2014-09-23 13:54:10,514 [Thread-1] DEBUG o.x.shutdown - Stopping component [org.xwiki.search.solr.internal.DefaultSolrIndexer]... 2014-09-23 13:54:10,514 [Thread-1] DEBUG o.x.shutdown - Component [org.xwiki.search.solr.internal.DefaultSolrIndexer] has been stopped … 2014-09-23 13:54:10,539 [Thread-1] INFO o.x.shutdown - XWiki shutdown has been completed.
As you can see the problem is that the DB is stopped first. So any code accessing the DB will get an error once the connection pool is stopped. This is why we get a lot stack traces when you ctrl-c XWiki quickly after it starts (i.e. when the init is not finished or when SOLR is still indexing). You can also get stack traces if a watchlist is being sent or some scheduler job is executing for example.
There are 2 ways to fix this: - solution 1: all code that accesses the DB could check if an XWiki shutdown is in progress and don’t generate stack traces if this is the case - solution 2: stop the DB as the last thing, after everything else has stopped
Obviously solution 2 is much better.
I’ve started implementing it by doing this: - Remove the current HibernateShutdownEventListener since this is executed first during shutdown, before component disposal - Instead have XWikiHibernateStore implement Disposable and move the content of HibernateShutdownEventListener in that method
This works already better but it’s not enough since ECM.dispose() works out the component dependencies but this works only for explicit dependencies and doesn’t work if a component uses the ComponentManager to dynamically find implementations (as this is the case for example for DefaultSolrIndexer).
Thus in order to help the component disposal process, I propose to introduce a new annotation: @DisposePriority(int).
Note that an alternative would be to introduce a new Disposable interface with a new getPriority() method in it (we cannot modify the current Disposable since that would break backward compat).
It's always hard to know what priority to put exactly so this is last resort solution for me.
The idea is to use this only for special cases (like DB shutdown) since in the majority of cases the component dependency order would be enough. So in these few cases it’s quite easy and similar to Macro priority (which hasn’t proved to be a problem so far and is working well).
A few other alternatives: 1) stop the DB after disposing components (I would not vote for it I think but it should work for this use case)
Yes I thought about this but I wanted something a bit more generic. See at the end of this email though.
2) introduce something like a @ComponentDependency annotation to let component declare indirect dependencies (ECM#dispose would simply add @ComponentDependency annotations to its current dependencies based ordering) and: 2.1) even when a component does not directly use XWikiStoreInterface (pretty much all of them are using XWiki methods and don't even know about it officially) we could still ask them to declare something like @ComponentDependency(XWikiStoreInterface.class) to indicate that they want to be disposed after any component of this role.
Yes I had also thought about this one but discarded because it means finding all components using indirectly XWikiStoreInterface and modifying them all. This means a lot of places (since a lot of places use the XWiki class for example) and a lot of work.
I also find it a lot more complex to implement than a priority order which seems really the most simple solution.
The dependencies based ordering is already what we have and 2) is just about adding more depdencies.
The first need is to add the annotation in the few places where we have the issue (only SOLR right now AFAIK) so no it does not really mean "finding all components using indirectly XWikiStoreInterface and modifying them all" right away.
2.2) make XWiki instance a component instead of storing it in the servlet context and inject it instead of accessing it through XWikiContext (XWiki itself would inject the component it want). XWiki itself would declare a @ComponentDependency(XWikiStoreInterface.class). 2.3) move all storage related methods from XWiki (saveDocument, getDocument, etc) to another component (and remove XWikiContext parameter to all these methods) which would be injected by components directly instead of searching for the XWikiContext (that's how new model will work but we can have another one before that still expose olcore APIs like XWikiDocument). Then this component would take care of declaring the @ComponentDependency(XWikiStoreInterface.class) (it's not really possible to directly inject the right XWikiStoreInterface since it's configuration based)
These 2 alternatives are ok but they are a lot of work and could be done in the future. I was trying to find a solution that can be applied in 1/2 day right now. We need to fix our bug now and I fear we’ll just not do anything if we wait for 2.2 and 2.3 (which we can do on the longer run).
2.3 is mostly copy pasting and start using the new component in the few places that have ordering issue (pretty much only SOLR right now). Changing other places is not a priority, it's not like we were refactoring the whole code each time we introduce a new API.
Since you don’t agree about priority and since I’d like to fix the shutdown “bug” for 6.2.x and on 6.3M1 now, I’m proposing the following alternative for now:
* Introduce a new constructor in the ApplicationStoppedEvent class: ApplicationStoppedEvent(boolean afterDispose) * Modify XWikiServletContextListener.contextDestroyed() so that it sends the ApplicationStoppedEvent(false) before calling cm.dispose() and it sends the ApplicationStoppedEvent(true) after calling cm.dispose() * Keep HibernateShutdownEventListener but make it listen to ApplicationStoppedEvent(true)
This will ensure it’s called after all components have been disposed.
I doubt that would work. If the CM is disposed HibernateShutdownEventListener won't exist anymore (and I'm not talking about using the observation manager which is going to be disposed officially, it's not doing anything special when disposed right now but it might).
WDYT? Acceptable?
Thanks -Vincent
The idea is then to use this information in ECM.dispose() to first sort all components based on this and then only to order them with their dependencies.
WDYT?
Thanks -Vincent
-- Sergiu Dumitriu http://purl.org/net/sergiu _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
participants (3)
-
Sergiu Dumitriu -
Thomas Mortagne -
vincent＠massol.net