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

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