[jira] [Commented] (FELIX-3422) Improve framework state change checking in ResolverImpl.deploy()

["Denis Rossi (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/FELIX-3422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14052422#comment-14052422 ] 

Denis Rossi commented on FELIX-3422:
------------------------------------

I found that LocalRepositoryImpl is registering itself as AllServiceListener.
In the serviceChanged method the m_snapshotTimeStamp, that is used by getLastModified() method,  is updated and the bundle is removed and re-added:

 public void serviceChanged(ServiceEvent event)
    {
        Bundle bundle = event.getServiceReference().getBundle();
        if (bundle.getState() == Bundle.ACTIVE && event.getType() != ServiceEvent.MODIFIED)
        {
            synchronized (this)
            {
                removeBundle(bundle, m_logger);
                addBundle(bundle, m_logger);
                m_snapshotTimeStamp = System.currentTimeMillis();
            }
        }
    }


> Improve framework state change checking in ResolverImpl.deploy()
> ----------------------------------------------------------------
>
>                 Key: FELIX-3422
>                 URL: https://issues.apache.org/jira/browse/FELIX-3422
>             Project: Felix
>          Issue Type: Improvement
>          Components: Bundle Repository (OBR)
>    Affects Versions: bundlerepository-1.6.6
>         Environment: All platforms
>            Reporter: Declan Cox
>            Priority: Minor
>              Labels: patch
>
> Currently the ResolverImpl deploy method checks to see if the last modified time of a repository instance is greater than the last updated resolution start timestamp, indicating a possible inconsistent state between the repository resources and the set of resolved resources. However it appears that the last modified time on a repository instance can be updated by the framework without it affecting the resolve state of the contained resources. The following comment indicates that the original implementer is aware of this, viz:
>         // Check to make sure that our local state cache is up-to-date
>         // and error if it is not. This is not completely safe, because
>         // the state can still change during the operation, but we will
>         // be optimistic. This could also be made smarter so that it checks
>         // to see if the local state changes overlap with the resolver.
>         for (int repoIdx = 0; (m_repositories != null) && (repoIdx < m_repositories.length); repoIdx++)
>         {
>             if (m_repositories[repoIdx].getLastModified() > m_resolveTimeStamp)
>             {
>                 throw new IllegalStateException("Framework state has changed, must resolve again.");
>             }
>         }
> I have encountered this error on an upgrade from version 1.0.3 to 1.6.6 recently (we have a provisioning agent which delegates to the bundle repository ). What we have seen is that between a call to resolve() and deploy() the local repository last modified timestamp is updated, however on investigation all contained resources have been resolved but we get the IllegalStateException nonetheless. 
> We are not sure why this update happens and where it comes from (this we will investigate further), however we could avoid the unnecessary exception by checking the overlap with the resolved set as suggested above. 
> To this end I added the following private method to ResolverImpl in a local copy, which seems to do the trick:
>       /**
>      * Check to see if the repository state has changed and whether
>      * this state change requires re-resolving
>      *
>      * @param repository - repository instance
>      * @return true if state has changed requiring re-resolving, otherwise false
>      */
>     private boolean hasStateChanged(final Repository repository) {
>         if (repository.getLastModified() > m_resolveTimeStamp.get()) {
>             // check overlap with resolver
>             final Resource[] resources = repository.getResources();
>             for (int i = 0; i < resources.length; i++) {
>                 if (!m_resolveSet.contains(resources[i])) {
>                     return true;
>                 }
>             }
>         }
>         return false;
>     }
> The corresponding change to the deploy method is as follows:
>         // Check to make sure that our local state cache is up-to-date
>         // and error if it is not. This is not completely safe, because
>         // the state can still change during the operation, but we will
>         // be optimistic. This could also be made smarter so that it checks
>         // to see if the local state changes overlap with the resolver.
>         for (int repoIdx = 0; (m_repositories != null) && (repoIdx < m_repositories.length); repoIdx++)
>         {
>             final Repository repository = m_repositories[repoIdx];
>             if (hasStateChanged(repository))
>             {
>                 throw new IllegalStateException("Framework state has changed, must resolve again.");
>             }
>         }
> I am not sure if this is the most efficient way to do this and if all considerations about state change have been taken into account (I am new to Felix) but I thought would submit it for consideration as a potential patch. 
> Thanks,
> Declan



--
This message was sent by Atlassian JIRA
(v6.2#6252)