[MDSAL-392] Clean up BindingRuntimeContext instantiation Created: 14/Nov/18  Updated: 28/Apr/20  Resolved: 29/Feb/20

Status: Resolved
Project: mdsal
Component/s: Binding runtime
Affects Version/s: None
Fix Version/s: 6.0.0

Type: Epic Priority: Medium
Reporter: Robert Varga Assignee: Robert Varga
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Blocks
blocks CONTROLLER-1882 Inject DataBroker only when all shard... Resolved
blocks MDSAL-525 Eliminate blueprint from mdsal-bindin... Resolved
Duplicate
is duplicated by MDSAL-250 Eliminate duplicate YangTextSchemaCon... Resolved
Relates
relates to MDSAL-235 [INFO] Failed to fully assemble schem... Resolved
relates to MDSAL-520 Replace blueprint containers with OSG... Confirmed

 Description   

DOM OSGi SchemaContext services and the interaction between binding codec, adapter are quite a bit perplexing. The wiring should really be simplified, so that we can arrive at a place where the codec tree is completely predictable and does not rely on TCCL.

One key ingredient we seem to be missing is that the set of ClassLoaders from which a particular SchemaContext should be carried with it.



 Comments   
Comment by Robert Varga [ 14/Nov/18 ]

Key points that need to be investigated:

  • BindingRuntimeContextListener and BindingRuntimeContextService need to be disregarded, as they are not used anywhere
  • ModuleInfoBackedContext is registered as the ClassLoaderStrategy, but otherwise should not be needed (aside from tests)
  • BindingCodecTreeFactory.(SchemaContext, Class<?>...) seems to be unused
  • ModuleInfoBackedContext.loadClass() works by locating ClassLoader corresponding to the package name of requested class – and this is populated from the YangModuleInfo
  • ModuleInfoBackedContext has the following fixme:
        // TODO finish schema parsing and expose as SchemaService
        // Unite with current SchemaService
        // Implement remove ModuleInfo to update SchemaContext
    
Comment by Robert Varga [ 14/Nov/18 ]

Note: in order to get the ClassLoader associated with a bundle, we need to load a class which is contained therein, so we can do BundleContext.getBundle().loadClass(foo).getClassLoader().

Current solution revolves around YangModelBindingProvider being a ServiceLoader service, hence we acquire implementation names from a Bundle's META-INF/services/.

This effectively means that DOM layer needs to have some service it can rely on, or at least a well-known class, which would sit in a private package solely for the purpose of being loaded. Otherwise DOM layer has no skin in the class loading game and it is only logical it cannot acquire the class loader.

An (ugly) alternative is to proxy Bundle.loadClass() as a service, but that is tricky, as it can fail if the bundle is uninstalled – which may be fine if "uninstalled" means that the class loader no longer exists, i.e. when the Bundle's contents are otherwise unreachable. If they are still reachable, we need the ClassLoader, as we need to continue to have access to the classes for as long as the target SchemaContext exists (or not, if we are fine with throwing weird exceptions).

 

Comment by Robert Varga [ 19/Nov/18 ]

Since OSGi is the only multi-classloader environment we are supporting, we really can get away with making mdsal-dom-schema-service-osgi depend on yang-binding, which defines the class we need, effectively moving tracking to a single place.

We then need to refactor bundletracker so we get explicit control over sources which go into a SchemaContext, thus we know which bundles are participating in it and can extract their classloaders – thus we can form a ClassLoading strategy.

Comment by Robert Varga [ 19/Nov/18 ]

For OSGi this should look something like:

  1. mdsal-dom-schema-service-osgi scans bundles, creating a SchemaContext and a QName->ClassLoader mapping (based on SchemaContext results). This combination (ClassLoadingSchemaContext?) is published to Service Registry.
  2. mdsal-binding-dom-adapter(-osgi) tracks ClassLoadingSchemaContext and for each publish it performs
    • locate all YangModuleInfos based on ServiceLoader on specified ClassLoader, filtering by QName
    • verify consistency of YMI with QName/ClassLoader mapping
    • construct a ClassLoadingStrategy (almost like ModuleInfoBackedContext, but without ModuleInfoRegistry)
    • construct a BindingRuntimeContext
    • publish BindingRuntimeContext to ServiceRegistry (or whatever)

Except we cannot make it work this way, as dom-adapter is tied with dom-codec and AbstractStreamWriterGenerator is spilling classes into the input ClassLoaders – hence we need to execute synchronously and not do roundtrips through Service Registry At any rate, that allows BindingRuntimeContext to be completely resolvable, without the need for TCCL trickeries.

The update of AbstractStreamWriterGenerator is tracked in MDSAL-401, at which point it feels that we should not be publishing the SchemaContext until we have an updated codec – hence our default Karaf wiring should entail binding-aware stack and we should provide an explicit feature for DOM-only deployments, if there is even such a thing. Non-feature.xml-based deployments are of course free to wire things the way they like.

 

Comment by Robert Varga [ 21/Dec/18 ]

It is worth noting that ModuleInfoBackedContext is the only binding-generator-impl class which requires yang-parser-impl. I think that directly implies ModuleInfoBackedContext should really be factored out of that package somewhere else.

Its design is really registry-like, where YangModuleInfos are registered and there is a baseline backing ClassLoadingStrategy, which is responsible for last-ditch lookups. It really is only used for Class-based instantiation of BindingCodecTree in BindingNormalizedNodeCodecRegistry – to realize unused  create(SchemaContext, Class<?>...) method.

The alternative uses BindingRuntimeContext, which is a mdsal-binding-generator-util concept. It is not a fully API-disaggregated, but for the purposes of wiring implementations it seems good enough.

Hence we can disregard most of current wiring possibilities and concentrate on getting a BindingRuntimeContext – in either a single-classloader scenario (which points towards ServiceLoader use) or in OSGi (which points towards scanning).

Comment by Michael Vorburger [ 29/Jan/19 ]

> BindingRuntimeContextListener and BindingRuntimeContextService need to be disregarded, as they are not used anywhere

==> https://git.opendaylight.org/gerrit/#/c/79999/

Comment by Robert Varga [ 24/Jul/19 ]

Acquiring a ClassLoader in OSGi requires knowledge of at least one class name, so that the class can be looked up.

In order to do this, mdsal-binding-dom-codec-osgi is examining /META-INF/services/org.opendaylight.yangtools.yang.binding.YangModelBindingProvider, which is a springboard to YangModuleInfo.

There is no DOM-level service available, which would provide the equivalent facility, not at least until we have YANGTOOLS-1009 implemented. Until that is done, mdsal-dom-schema-service-osgi should be bound to the binding packaging in its reliance on YangModelBindingProvider. Note it should not instantiate the provider, just load the class. Most definitely it must not access YangModuleInfo and the dependency information it holds, as that static resolution (and implied inter-revision dependencies) are part of Binding Spec proper.

Comment by Robert Varga [ 22/Feb/20 ]

Okay, so after moving things around and cleaning them up, the picture is quite clear – we are battling lifecycle issues coming from multiple fronts and their common denominator is that the entities involved are not effectively-immutable and are attempting to make in-place updates work.

This will involve multiple steps, each of which will provide incremental improvement to the situation at hand.

Comment by Robert Varga [ 22/Feb/20 ]

The code has been refactored to the point where we have mdsal-binding-runtime-{api,spi}. API is where BindingRuntimeContext landed. SPI is where ModuleInfoBackedContext ended up being (which cleaned the dependency tree a lot). It is used by static utilities (for tests and statically-wired environments), which do not offer any fallback classloader nor support dynamic updates. Its dynamic capabilities are used only by mdsal-binding-dom-codec-osgi.

mdsal-binding-dom-codec-osgi also does the wrong thing, really, as it is using ClassLoaderFactory to effectively enlarge the space of allowed classloaders (in the hope they'll provide the augmentation being saught), hence its BindingRuntimeContext is not really immutable, as the backing factory is playing tricks – which directly contradicts Immutable contract. The dynamic part of ModuleInfoBackedContext needs to be factored out into a Builder pattern, where the dynamic parts of ModuleInfoBackedContext are handled by the OSGi part (i.e. retain a cache/registry/etc.). Once it attempts to create a BindingRuntimeContext, it really wants to capture a snapshot of current state of affairs, including the right set of ClassLoaders and present that as the BindingRuntimeContext. This will disconnect the BindingRuntimeContext from any further actions of the OSGi part.

mdsal-binding-dom-codec-osgi tries to solve this by propagating BindingRuntimeContext updates, but unfortunately no one is acting on them – as evidenced by https://git.opendaylight.org/gerrit/c/mdsal/+/79999 .

The root cause is the use of Blueprint by mdsal-binding-dom-adapter. Blueprint's take on lifecycle is that you cannot retain (i.e. cache) anything the service has given you – simply because if the service goes away, there is no way to find out, because BP will very helpfully actually block the call until the disappeared services comes back (backed by completely different state!) and then resume operation. If the service does not come back by some arbitrary time, it will throw an exception. There is nothing we can use to invalidate anything we cache, hence we cannot cache anything.

This means that while mdsal-binding-dom-codec can be tightly bound to a BindingRuntimeContext (and internally is via BindingCodecContext), this is not exposed to the adapter, but rather covered by the BindingNormalizedNodeCodecRegistry indirection – which actually is the only implementation class the adapter uses!.

OSGi Declarative Services (DS), a.k.a Service Component Runtime, provides a properly-propagated binding lifecycle where we are in control of the services we are actually talking to. Most notably the default static binding completely embraces immutability and guarantees that the binding does not change between invocation of @Activate and @Deactivate methods. This in turn allows us to instantiated immutable services – and those do not need to deal with externalities changing. At the very most there may end up being some inter-component hand-off, but the need for that is not quite clear ATM.

We want to remove BindingRuntimeContextListener, but we also want to leverage DS to perform all the service tracking – hence we can whiteboard easily and not worry about how things are coming and going.

Therefore Blueprint has to be terminated with prejudice and replaced by properly-defined components. The primary component is mdsal-binding-runtime-osgi, a new component, which will do the codec-osgi work and publish BindingRuntimeContext as a component (hence it needs to become a service).

All of these will be naturally immutable, allowing safe derivation of state. This also means that for OSGi binding-runtime-osgi will also provide the DOMSchemaService component – as that is readily derived. Once DOM is ready (i.e. when we have YANGTOOLS-1009), this role will revert to the DOM layer and Binding will just tack BindingRuntimeTypes on top of a ClassLoading strategy.

At the end of all of this, binding-dom-codec needs to provide a factory, which takes a BindingRuntimeContext and produces the public equivalent of BindingCodecContext. That factory is then used by binding-dom-adapter to instantiate its context – and publish services across them.

 

Comment by Robert Varga [ 28/Feb/20 ]

https://git.opendaylight.org/gerrit/c/mdsal/+/88028 deals with the all but the codec/adapter parts. It leaves off with a properly-published BindingRuntimeContext, which needs to be injected into codec/adapter. We already have the CodecTree part of codec ready, but adapter's use of CodecRegistry needs to be audited. We may need a separate interface for a fused CodecContextFactory, BindingCodecTree, DataObjectSerializerRegistry – which is what BindingCodecContext provides.

Generated at Wed Feb 07 20:09:41 UTC 2024 using Jira 8.20.10#820010-sha1:ace47f9899e9ee25d7157d59aa17ab06aee30d3d.