Bug 336941

Comment 1 Thomas Watson 2011-03-14 10:56:21 EDT

I will take a look at persisting the digraph.  My initial motivation for this was to persist the digraph for runtime purposes so you don't have to keep reconstructing the digraph each time you restart the framework from a persistently cached state.  Right virgo does not restart from a warm cached state so this will not help until virgo decides to do that.

If you want to load a digraph from storage for the purpose of offline analysis, then we may need a factory of some sorts that can load the digraph.  Should a new service be used for this?  or should we stick a method on RegionDigraph for storing and loading:

// saves the current state of the digraph to out
save(OutputStream out)

// loads the saved state of a digraph from in into a new digraph object
Region Digraph load(InputStream in)

Comment 2 Glyn Normington 2011-03-14 10:59:45 EDT

I think it would be cleaner to add a new service for those two methods. Something with a name like RegionDigraphPersistence.

Comment 3 Thomas Watson 2011-03-16 10:27:45 EDT

Created attachment 191303 [details]
Possible solution for persisting digraph

Here is a patch that I have been working on for persisting the digraph.  A new method is added to RegionDigraph:

RegionDigraphPersistence getRegionDigraphPersistence();

A RegionDigraphPersistence object can then be used to save and load RegionDigraph objects.  Note that an RegionDigraph object loaded from persistence will be disconnected from the running OSGi Framework.  Any changes to the loaded RegionDigraph will have no affect on the running system.

Note that I did not integrate persistence into the RegionManager which manages the RegionDigraph for the running framework.  Glyn and I have discussed this and decided to put off that work until we extract the digraph code into a separate bundle that eventually will be moved to equinox.

Comment 4 Glyn Normington 2011-03-16 10:52:56 EDT

Thanks for the contribution Tom. Please would you confirm that you wrote 100% of the code, that you have your employer's permission to donate it, and that any new files have suitable license headers.

Comment 5 Thomas Watson 2011-03-16 11:24:24 EDT

(In reply to comment #4)
> Thanks for the contribution Tom. Please would you confirm that you wrote 100%
> of the code, that you have your employer's permission to donate it, and that
> any new files have suitable license headers.

Yes I confirm:

- I authored 100% of the code
- I have my employer's permission to contribute the code
- The new files have a suitable license header

Comment 6 Glyn Normington 2011-03-18 12:43:57 EDT

Tom's changes are pushed to master.

He suggested added a version to the persistent form of a digraph. I added a string identifier and a version and validate these when reading the stream back in.

Comment 7 Glyn Normington 2011-03-18 12:44:32 EDT

It remains to dump out the region digraph using a dump contributor and read this back in when doing offline state analysis.

Comment 8 Glyn Normington 2011-03-30 11:09:23 EDT

This is mostly implemented, tested, and in the process of rippling through.

Note that StandardQuasiFramework now has this method:

private void setResolverHookFactory() {
        /*
         * Create a resolver hook factory for the region digraph. If the region digraph is live, this will create a hook
         * factory equivalent to the live hook factory. If the region digraph is disconnected (a reconstituted copy of a
         * live region digraph), this will produce a hook factory independent of the live hook factory.
         */
        ResolverHookFactory resolverHookFactory = new RegionResolverHookFactory(this.regionDigraph);
        this.state.setResolverHookFactory(resolverHookFactory);
    }

For a quasi-framework representing a "live" state, the state shares a resolver hook factory based on the "live" digraph which was passed to StandardQuasiFramework. For a quasi-framework representing a state dump, the state uses a resolver hook factory based on the disconnected digraph passed to StandardQuasiFramework.

I point this out because it will still be necessary to create a RegionResolverHookFactory after the region digraph code and hooks move to Equinox.

Comment 9 Thomas Watson 2011-03-30 11:20:47 EDT

(In reply to comment #8)
> I point this out because it will still be necessary to create a
> RegionResolverHookFactory after the region digraph code and hooks move to
> Equinox.

This indicates that the hook implementation will need to be an API of some sorts?  I was under the assumption that org.eclipse.virgo.kernel.osgi.region.hook package was going to become internal at some point.

Comment 10 Glyn Normington 2011-03-30 11:43:14 EDT

(In reply to comment #9)
> (In reply to comment #8)
> > I point this out because it will still be necessary to create a
> > RegionResolverHookFactory after the region digraph code and hooks move to
> > Equinox.
> 
> This indicates that the hook implementation will need to be an API of some
> sorts?  I was under the assumption that
> org.eclipse.virgo.kernel.osgi.region.hook package was going to become internal
> at some point.

That's why I brought it up. I would prefer to keep the hook implementation internal to Equinox after the move. Perhaps we need an extra State API to set the digraph instead of the framework hook factory?

Comment 11 Thomas Watson 2011-03-30 11:52:10 EDT

(In reply to comment #10)
> 
> That's why I brought it up. I would prefer to keep the hook implementation
> internal to Equinox after the move. Perhaps we need an extra State API to set
> the digraph instead of the framework hook factory?

My intention is that the RegionDigraph API and impl will be included in a bundle on top of the framework, not actually integrated directly into the equinox framework.  This makes setting the RegionDigraph on the State API a no-go since the State API is in the framework and cannot reference outside types.

Perhaps the RegionDigraph should have the ability to return hook implementations?

Comment 12 Glyn Normington 2011-03-30 12:05:27 EDT

Good point. Then we could add a method on RegionDigraph which sets the resolver hook factory of a State passed as a parameter. We could abstract the description to talk about making the State honour the region digraph. That would avoid exposing a hook on the API.

Comment 13 Thomas Watson 2011-03-30 14:28:26 EDT

(In reply to comment #12)
> Good point. Then we could add a method on RegionDigraph which sets the resolver
> hook factory of a State passed as a parameter. We could abstract the
> description to talk about making the State honour the region digraph. That
> would avoid exposing a hook on the API.

That is a possibility, but I would prefer to keep the state API out of the signature of the Region API.  I don't mind introducing optional dependencies on Equinox framework API so that we can optimize for the equinox case, but embedding Equinox framework API directly in the region API really binds the whole thing to only the equinox framework.  I would like to avoid that if possible.

Another possibility is to have an adapt like method (ala Bundle.adapt) that allows you to adapt the RegionDigraph to one of the hooks:

ResolverHookFactory resolverHook = digraph.adapt(ResolverHookFactory.class);

Comment 14 Glyn Normington 2011-03-31 04:20:45 EDT

(In reply to comment #13)
> (In reply to comment #12)
> > Good point. Then we could add a method on RegionDigraph which sets the resolver
> > hook factory of a State passed as a parameter. We could abstract the
> > description to talk about making the State honour the region digraph. That
> > would avoid exposing a hook on the API.
> 
> That is a possibility, but I would prefer to keep the state API out of the
> signature of the Region API.  I don't mind introducing optional dependencies on
> Equinox framework API so that we can optimize for the equinox case, but
> embedding Equinox framework API directly in the region API really binds the
> whole thing to only the equinox framework.  I would like to avoid that if
> possible.

Ah, I hadn't thought of that. Thanks.

> 
> Another possibility is to have an adapt like method (ala Bundle.adapt) that
> allows you to adapt the RegionDigraph to one of the hooks:
> 
> ResolverHookFactory resolverHook = digraph.adapt(ResolverHookFactory.class);

Adapt is weird and I'd prefer to be more explicit. I guess the digraph method:

ResolverHookFactory getResolverHookFactory();

would be simplest as the return type is then OSGi standard. This begs the question of
whether to have getters for the other hooks of course. I don't see why not for completeness.

Comment 15 Thomas Watson 2011-03-31 09:30:47 EDT

(In reply to comment #14)
> > 
> > Another possibility is to have an adapt like method (ala Bundle.adapt) that
> > allows you to adapt the RegionDigraph to one of the hooks:
> > 
> > ResolverHookFactory resolverHook = digraph.adapt(ResolverHookFactory.class);
> 
> Adapt is weird and I'd prefer to be more explicit. I guess the digraph method:
> 

Agree.

> ResolverHookFactory getResolverHookFactory();
> 
> would be simplest as the return type is then OSGi standard. This begs the
> question of
> whether to have getters for the other hooks of course. I don't see why not for
> completeness.

This is fine with me.  We can wait to do this when we move the code to Equinox. (unless you really want to do it now) :)

Comment 16 Glyn Normington 2011-03-31 09:53:32 EDT

Thanks. I'll wait. :)

Comment 17 Thomas Watson 2011-04-21 11:37:29 EDT

(In reply to comment #16)
> Thanks. I'll wait. :)

I opened equinox bug 343570 to address this.