Hi! Agree with Maxim.
It seems to me quite normal to return a proxy for a local instance in the case when the user has explicitly enabled statistics collection in the service settings. Those. by default, we do not change the behavior and if the collection of metrics is not needed, a local instance will be returned. And I also think the javadoc should be changed to reflect the new behavior. So, I'm for 1 + 3. вт, 28 дек. 2021 г. в 10:51, Maksim Timonin <timoninma...@apache.org>: > > Hi! > > I agree that users shouldn't expect a non-proxy when invoking the > `IgniteServices#serviceProxy()` method. I think it's up to Ignite to return > a non-proxy instance here as possible optimisation. But users have to use > interfaces in any case. There is the `IgniteServices#service()` method for > explicit return of local instances. > > With enabling of metrics we can break users that explicitly > use `#serviceProxy` (proxy!), and then explicitly cast it to an > implementation class. In this case such users will get a runtime exception. > I think we can write a good javadoc for > `ServiceConfiguration#setEnableMetrics()`, it should mention that it works > only with proxy, and it doesn't collect metrics with non-proxy usages with > `IgniteService#service()`. > > So, I propose to proceed with two solutions - 1 and 3: fix docs for > `#serviceProxy()` and provide detailed javadocs > for `ServiceConfiguration#setEnableMetrics()`. > > If some users will enable metrics (even with such docs!) and will be using > casting proxy(!) to an implementation, then they will get a runtime > exception. But I believe that it is an obvious failure, and it should be > fixed on the user side. > > > > > > On Mon, Dec 27, 2021 at 10:26 PM Vladimir Steshin <vlads...@gmail.com> > wrote: > > > Hi, Igniters. > > > > > > I'd like to suggest modifying `/IgniteServices.serviceProxy(String name, > > Class<? super T> svcItf, boolean sticky)/` so that it may return proxy > > even for local service. Motivation: service metrics [1]. To measure > > method call we need to wrap service somehow. Also, the method name says > > `proxy`. For local one we return direct instance. Misleading. > > > > > > Solutions: > > > > 1) Let’s return a proxy (`/GridServiceProxy/`) once we need. Let’s > > change the javadoc to say `@return Proxy over service’. Simply works. > > > > Pros: invocation like `/MyServiceImpl svc = serviceProxy(«myservice», > > MyService.class)`/ would fail. Wierd usage to me. But possible. > > > > 2) Introduce a new method with forced-proxy flag > > like`IgniteSerives.serviceProxy(…, boolead focedProxy)`. And add a > > warning to other service-obtain methods like: «`/You enabled service > > metrics but it doesn’t work for local service instanse. Use forced-proxy > > serviceProxy(…, boolean forcedProxy)/` Pros: we’ve already have about > > 5-6 methods to get services in `/IgniteServices/`. I doubt we need one > > more. > > > > 3) Fix the documentation so that it tells the service metrics would work > > only with proxy. Pros: service metrics just won’t work for local > > services. Suddenly. > > > > > > My vote is for #1: let's use a proxy. WDYT? > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-12464 > > > >
