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

[Jim Ma]

Hi, Please see my comments inline.
On 05/22/2013 02:01 AM, Alessio Soldano wrote:
> Hi Jim,
> CC-ing ws dev list, as this is possibly of wider interest.
>
>
> On 05/21/2013 09:29 AM, Jim Ma wrote:
>> Hi Alessio,
>>
>> 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 .
> 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.
> 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.

>   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.
>
>
>> 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.

>
> 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.
>
> WDYT?
> Cheers
> Alessio
>
>