[
https://issues.apache.org/jira/browse/FELIX-3329?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13211547#comment-13211547
]
Bram de Kruijff commented on FELIX-3329:
----------------------------------------
Hi, Marcel a couple of comments and findings on this code
1) If found the ambiguous use of "resource" throughout the code is rather
confusing. First there is the "deployment resources" which are effectively
representations of metadata files. Second there are the "autoconf resources"
which are effectively representations of designates. So, for example the
DropResource and DeleteResource tasks are entirely different beasts. I think
some renaming could help future adventurers. I'll use the aforementioned terms
throughout this comment.
2) When an existing deployment resource gets updated with a new version
prepare() will fail throwing a NPE on the m_configAdmin which will always be
null. I think it even tries to delete all previous autoconf resources
potentially also deleting the once that were in the update as well. I've
attached a patch that should illustrate and fix this issue.
3) You have implemented the defer only to the end of a DeploymentSession. After
that all outstanding configuration tasks are simply discarded. This does not
cover our Amdatu use case and IMHO has some fundamental issues. Let me
eleborate a little on both:
The use case is a Felix Fileinstall Autoconf extension. Whenever fileinstall
detects a new file in the deployement directory is calls our artifactinstaller.
Our approach is to simply translate such an event into one small deployment
session. It only holds one file at a time. Now the deployment of such a
configuration file can trigger the registration of a new ConfigurationAdmin. A
second configuration file can then be deployed that contains a filter
(FELIX-3330) to match the newly created ConfigurationAdmin and via the same
route the AutoConfResourceProcessor will deliver it. This works well, but fails
when the second configuration file is delivered first. Each deployment
translates into a separate session, as the filter wont match the first
ConfigurationAdmin and when the deployment session ends
AutoConfResourceProcessor will simply discard it. As fileinstall will deliver
only one file at a time and there is no defined delivery order the means we can
not use it in this way. Either we need to rework the fileinstall in ways I don
care to think about right now or we need to defer to extend beyond deployment
sessions.
The fundamental issue with discarding updates at the end of a deployment
session if rather arbitrary. It covers the use case in the description of this
issue where DeploymentAdmin stops the ConfigurationAdmin bundle during
deployment, then starts it again AND it synchronously registers its service
during start. If for whatever reason the ConfigurationAdmin is deferred
asynchronously this mechanism will fail in a non deterministic way. It couples
bundles and service lifecycle and places requirements on the implementation of
the ConfigurationAdmin bundle.
Previously the AutoConfResourceProcessor would simply throw a
ResourceProcessorException when no ConfigurationAdmin was available which, as
explained in this issue, has severe limitations. Thus deferring IMHO is a good
option changing the AutoConfResourceProcessor contract from "yes, the
configuration is valid and I have delivered the it" to "yes, the configuration
is valid and I will deliver it when the (appropriate) ConfigurationAdmin shows
up". To be honest I am not sure this is within the intent of the specification
which specifies the processor must create/delete configurations in the
commit(), but as we have use cases for this behavior I am inclined to support
defer when needed.
Thus, my concern is about the "before the end of the deployment session" part.
Effectively the contract is now "yes, the configuration is valid and I will
deliver the configuration when the (appropriate) ConfigurationAdmin shows up
before the end of the deployment session OR ELSE I WILL SILENTLY DROP THEM".
Why and why not defer indefinitely? It is true that deferring longer does not
guarantee delivery, but as long as AutoConfResourceProcessor honors its
contract maintaining configuration update order I think it's much more robust.
You can hardly blame AutoConfResourceProcessor for a ConfigurationAdmin not
showing up.
Concluding, IMHO defer is a good option but it should not be limited to
deployment session lifetime because that is not very robust and does not cover
use cases that exceed the deployment session. Obviously, to support that
AutoConfResourceProcessor would also have to persist its state to survive
restarts itself.
> Allow the autoconf resource processor to defer the setting of the
> configuration until ConfigurationAdmin is available
> ---------------------------------------------------------------------------------------------------------------------
>
> Key: FELIX-3329
> URL: https://issues.apache.org/jira/browse/FELIX-3329
> Project: Felix
> Issue Type: Task
> Reporter: Marcel Offermans
> Assignee: Marcel Offermans
> Attachments: FELIX-3329_ResourceDeleteNPE.patch
>
>
> Right now, autoconf will always fail if you start out with an empty container
> and install a deployment package with all your bundles, including
> configuration data and ConfigurationAdmin, because at the time it tries to
> find the ConfigurationAdmin service, that won't be started yet. A solution
> would be to (in that case) defer setting the configuration until the service
> becomes available. The downside of this is that we never know for sure if
> this will work.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
