[jbossws-dev] Eliminate the EndpointRegistry and consolidate the management

[Alessio Soldano]

Hi Jim,

On 05/27/2013 06:16 AM, Jim Ma wrote:
>>> As we discussed yesterday, these are my two proposals we might can think
>>> about and refactor a bit in this  or future release after complete
>>> jax-rpc removal.
>>>
>>> 1) Eliminate the EndpointRegistry, use EndpointService to get
>>> Endpoint info
>>>     ATM, each deployed endpoint will be created with an EndpointService,
>>> and each EndpointService will be also registered into serviceContainer.
>>>     With MSC's getServiceNames() and getService() we can get all the
>>> deployed endpoint information. ServiceContainer is a registry , so we
>>> don't need
>>>     create our own registry to maintain the endpoint map.
>>>     To resolve the stack/container depends issue ,  we can create
>>> similar
>>> spi api and factory class in integration layer, and return the endpoint
>>> list to cxf stack .
>> I've been thinking a bit about this proposal; the way I would possibly
>> see it implemented is in having a new implementation of the existing
>> EndpointRegistry SPI interface that internally relies on the container
>> services (through ServiceContainer). That would be created in the
>> existing EndpointRegistryService instead of the default impl that's
>> created now (we'd also need a managed version of it btw, implementing
>> ManagedEndpointRegistry).
>> There's no need for a new abstraction interface, given we already have
>> the EndpointRegistry one (the interface is fine and keeping it would
>> allow us to avoid touching the stack).
>  Agree with delegate EndpointRegistry to ServiceContainer to get the
> Endpoint .

OK

>> If we did this new impl, we'd not need to explicitly register/unregister
>> endpoints in the registry in start/stop methods of EndpointService. So
>> far so good.
>   Yes. This will bring us two pros: 1) as you said , no need to
> register/unregister endpoints.
>   2) we  can also simplify or remove the EndpointLifeCycleHandler: we do
> this job when the
>   EndpointService is started/stopped.

Would you simply remove the EndpointLifecycleDeploymentAspect and move
the LifeCycleHandlers call into the EndpointService, or have anything
more in mind?


>> The problem is that in change of saving the memory of a map instance,
>> with the new impl we need to process the whole set of AS services each
>> time and filter out the WS endpoints ones through name matching, which
>> is possibly not very good performance-wise.
>  This is the cons of change we can conclude.  But at the moment our
> EndpointRegistry is used in
>  these two places : WSEndpointMetrics and ServletHelper. I looked at the
> code and
>  it looks it can be all refactored to get the Endpoint with the
> ServiceContainer.getService(serivceName)
>  if we name the ServiceName with ContextRoot and endpointName without
> deployment archive name.
>  The as7 console retrieves the endpoint info from the ModelNode which is
> created
> in the deployment phase, and it won't need call the serviceConintainer
> and filter out the WS endpoint.
> That said, unless we have other requirement to retrieve all the ws
> endpoints from the serviceContainer,
> it seems won't be a problem at the current deployment and management
> architecture.

OK, so you're basically suggesting to change the convention on the
service endpoint name (removing the deployment unit archive name part)
and rely on the new one to make a direct getService call in the
ServiceContainer. That might work, I can't remember why we have the
deployment unit name in there, but it's not required for a specific
reason, this is definitely doable.


>>   Moreover, we actually lose
>> control over the actual time an endpoint is registered/unregistered and
>> is hence available/unavailable for invocation.
>> So we have pros and cons, I'd say this needs to be properly evaluated...
>   IMHO, the state of EndpointService should be represented the state of
> the Endpoint.
>   We don't need to ask the consumer to check the state of Endpoint.With
> this concept,
>   we can evaluate how can we place the order of
> EndpointServiceDeploymentAspect and
>   make it reflect the real state of an endpoint is ready for invocation.

ok, agreed (also considering comments above). Created
https://issues.jboss.org/browse/JBWS-3643


>>> 2) Consolidate the jbossws-cxf and cxf management
>>>     We now have ManagedEndpoint MBean to expose the management info to
>>> monitor the processTime, requestCount etc. If the cxf management is
>>> enabled in jboss-webservices.xml,
>>>     some of the info are duplicate.  We'd think about how to consolidate
>>> these two kind of management bean. Enable the cxf management by default
>>> is the way to go ?
>> The problem I see now is that cxf management is jmx / managed bean
>> based, and I think I remember the AS might run without the mbean
>> server on.
>> To be better analysed, also considering that the stats needs to be
>> available through the Endpoint (they're exposed in the AS managemen, see
>> org.jboss.as.webservices.dmr.WSEndpointMetrics).
>  The problem I see here is if cxf's management is enabled we exposed
> duplicate
>  manament info though JMX. And customer will probably get the different
> value
>  from CXF's JMX Bean and  our JMX bean exposed from WSEndpointMetrics 
> although
>  it represents the same thing (for example our ProssTiime in a certain
> Endpoint and
> CXF's response time). Whether enrich our management and finally remove the
> cxf's enabled flag or we make our management internally use the cxf's
> management stuff,
> it will be OK as long as we provide the unified interface to user. Just
> my 2 cents.

I'd say let's keep on thinking possible practical solutions for this and
target 4.3 series.


>> This said, speaking of cleanups, there's actually something I thought
>> about I'd love to simply and just remembered while looking at your
>> registry proposal. That's the jbossws SPI resolution mechanism.
>> Currently we have a double resolution, so e.g. to get Foo we do:
>>
>> SPIProvider spiProvider =
>> SPIProviderResolver.getInstance().getProvider();
>> Foo foo = spiProvider.getSPI(Foo.class);
>>
>> or (when a specific classloader is to be used):
>>
>> SPIProvider spiProvider =
>> SPIProviderResolver.getInstance(classLoader).getProvider();
>> Foo foo = spiProvider.getSPI(Foo.class, classLoader);
>>
>> both SPIProviderResolver#getInstance and SPIProvider#getSPI internally
>> rely on the jbossws ServiceLoader, which goes through the usual 3
>> resolution mechanism (service loader, propfile and sysprop). Moreover,
>> the default SPIResolver impl in jbws-common (which is what is actually
>> always used) allows using default values if nothing is found.
>>
>> I'd like to simplify this; we could remove the SPIProviderResolver
>> indirection level by having a singleton in SPIProvider returned by a
>> static getInstance() and resolved once using the jbossws-spi
>> classloader. No need to try resolve the resolver each time; no need for
>> such a verbose api.
>> Something like:
>>
>> public abstract class SPIProvider
>> {
>>     private static SPIProvider me;
>>
>>     public static SPIProvider getInstance() {
>>        if (me == null) {
>>            me = SPIProviderResolver.getInstance().getProvider();
>>        }
>>        return me;
>>     }
>>
>>     public <T> T getSPI(Class<T> spiType)
>>     {
>>        return getSPI(spiType, SecurityActions.getContextClassLoader());
>>     }
>>
>>     public abstract <T> T getSPI(Class<T> spiType, ClassLoader loader);
>> }
>>
>>
>> and the use it as follows:
>>
>> Foo foo = SPIProvider.getInstance().getSPI(Foo.class, classLoader);
> 
>  I totally agree with this suggestion. It's very nice.

OK, just created https://issues.jboss.org/browse/JBWS-3644

Thanks for the feedback

Alessio

-- 
Alessio Soldano
Web Service Lead, JBoss