Hi, folks.
I'd like to advance the final decision. Is it OK to: 1)Make IgniteService.serviceProxy() return proxy for locally deployed service too. 2)Make the proxy collect metrics of service method invocations without any additional conditions, interfaces, options. 3) Deprecate IgniteService.service(). WDYT? 17.03.2020 19:56, Vladimir Steshin пишет: > Andrey, > > >>> Is it possible to return actual class instance instead of > interface from serviceProxy() method? > > No, it is not possible. IgniteServiceImpl doesn't allow that: > > public <T> T serviceProxy(final String name, final Class<? super T> > svcItf, final boolean sticky, final long timeout) { > ... > A.ensure(svcItf.isInterface(), "Service class must be an > interface: " + svcItf); > > ... > > } > > > 17.03.2020 17:34, Andrey Gura пишет: >> Vladimir, >> >>> Why not using IgniteServices.serviceProxy() for that? Since it >>> requires an interface, It could return proxy for local service too and >>> keep backward compatibility at the same time. >> Is it possible to return actual class instance instead of interface >> from serviceProxy() method? E.g. could I get ServiceImpl as result of >> method call instead of ServiceItf? >> >> On Tue, Mar 17, 2020 at 9:50 AM Vladimir Steshin <[hidden email]> >> wrote: >>> Andrey, >>> >>>>>> IgniteServices.service() method could return actual interface >>>>>> implementation instead of interface itself. >>> >>> IgniteServices.service() always return actual local service >>> instance, no proxy, might be without any interface but except Service. >>> >>>>>> If yes then we can add new method IgniteService.serviceLocalProxy(). >>>>>> It will not break backward compatibility and will always return >>>>>> proxy. >>> Why not using IgniteServices.serviceProxy() for that? Since it >>> requires an interface, It could return proxy for local service too and >>> keep backward compatibility at the same time. >>> >>> 16.03.2020 20:21, Andrey Gura пишет: >>>> Vladimir, >>>> >>>>>> We won’t affect existing services >>>>> How exactly will we affect services without special interface? >>>>> Please see >>>>> the benchmarks in previous email. >>>> I talk about backward compatibility, not about performance. But it >>>> doesn't matter because... see below. >>>> >>>> My fault. From discussion I realized that services doesn't require >>>> interface. But indeed it does require. >>>> >>>> If I understand correctly, IgniteServices.service() method could >>>> return actual interface implementation instead of interface itself. >>>> Am I right? >>>> >>>> If yes then we can add new method IgniteService.serviceLocalProxy(). >>>> It will not break backward compatibility and will always return proxy. >>>> >>>> On Thu, Mar 12, 2020 at 2:25 PM Vladimir Steshin >>>> <[hidden email]> wrote: >>>>> Andrey, hi. >>>>> >>>>>>> We won’t affect existing services >>>>> How exactly will we affect services without special interface? >>>>> Please see >>>>> the benchmarks in previous email. >>>>> >>>>> >>>>>>>> what if we generate will generate proxy that collects service’s >>>>>>>> metrics >>>>> only if service will implement some special interface? >>>>> >>>>> >>>>> I don’t like idea enabling/disabling metrics involves code change, >>>>> compilation. I believe it should be an external option, probably >>>>> available >>>>> at runtime through JMX. >>>>> >>>>> >>>>>>> we can impose additional requirement for services that want use >>>>>>> metrics >>>>> out of box. … service must have own interface and only invocations of >>>>> methods of this interface will be taken into account for metrics >>>>> collection. >>>>> >>>>> Why one more interface? To work via proxy, with remote services user >>>>> already has to use an interface additionally to Service. If we >>>>> introduce >>>>> proxy for local services too (as suggested earlier), an interface >>>>> will be >>>>> required. Current IgniteService#serviceProxy() already requires >>>>> interface >>>>> even for local service. I don’t think we need one more special >>>>> interface. >>>>> >>>>> >>>>>>> user always can use own metrics framework. >>>>> Since we do not significantly affect services user can use both or >>>>> disable >>>>> our by an option. >>>>> >>>>> >>>>> With the discussion before and the benchmark I propose: >>>>> >>>>> >>>>> - Let IgniteService#serviceProxy() give GridServiceProxy for local >>>>> services >>>>> too. It already requires to work via interface. So it’s safe for >>>>> user code. >>>>> >>>>> >>>>> - Deprecate IgniteService#service() >>>>> >>>>> >>>>> - Make service metrics enabled by default for all services. >>>>> >>>>> >>>>> - Bring system param which disables metrics by default for all >>>>> services. >>>>> >>>>> >>>>> - Bring parameter/method in MetricsMxBean which allows >>>>> disabling/enabling >>>>> metrics for all services at run time. >>>>> >>>>> Makes sense? >>>>> >>>>> чт, 5 мар. 2020 г., 16:48 Andrey Gura <[hidden email]>: >>>>> >>>>>> Hi there, >>>>>> >>>>>> what if we will generate proxy that collects service's metrics >>>>>> only if >>>>>> service will implement some special interface? In such case: >>>>>> >>>>>> - we won't affect existing services at all. >>>>>> - we can impose additional requirement for services that want use >>>>>> metrics out of box (i.e. service that implements our special >>>>>> interface >>>>>> *must* also have own interface and only invocations of methods of >>>>>> this >>>>>> interface will be taken into account for metrics collection). >>>>>> - user always can use own metrics framework instead of our (just do >>>>>> not implement this new special interface). >>>>>> >>>>>> About metrics enabling/disabling. At present IGNITE-11927 doesn't >>>>>> solve this problem. Just because there is no metrics implementation >>>>>> for services :) >>>>>> Anyway we should provide a way for configuring service metrics (in >>>>>> sense of enabled/disabled) during service deploy. It's easy for >>>>>> cases >>>>>> where deploy() methods have ServiceConfiguration as parameter. But >>>>>> there are "short cut" methods like deployXxxSingleton(). I have >>>>>> ideas >>>>>> how to solve this problem. For example we can introduce "short cut" >>>>>> factory methods like nodeSingletonConfiguration(String name, Service >>>>>> service) and clusterSingletonConfiguration(String name, Service >>>>>> service). This methods will return configuration which has >>>>>> parameters >>>>>> for given type of deployment and could be modified, e.g. metrics >>>>>> could >>>>>> be enabled. >>>>>> >>>>>> WDYT? >>>>>> >>>>>> On Wed, Mar 4, 2020 at 8:42 PM Vladimir Steshin <[hidden email]> >>>>>> wrote: >>>>>>> Vyacheslav, Denis, hi again. >>>>>>> >>>>>>> >>>>>>> >>>>>>>>>> I agree with the proposal to introduce a new method which >>>>>>>>>> returns >>>>>> proxy >>>>>>> include the case of locally deployed services. >>>>>>> >>>>>>> >>>>>>> >>>>>>> I see one is restricted to use an interface for service with >>>>>>> IgniteServiceProcessor#serviceProxy(…): >>>>>>> >>>>>>> >>>>>>> >>>>>>> A.ensure(svcItf.isInterface(), "Service class must be an >>>>>>> interface: " + >>>>>>> svcItf); >>>>>>> >>>>>>> >>>>>>> >>>>>>> What if we change IgniteService#serviceProxy(...) so that it >>>>>>> will return >>>>>>> proxy everytime? That looks safe for user code. Doing so we >>>>>>> might only >>>>>>> deprecate IgniteService#service(...). >>>>>>> >>>>>>> >>>>>>> >>>>>>> вт, 3 мар. 2020 г., 11:03 Vyacheslav Daradur <[hidden email]>: >>>>>>> >>>>>>>> Denis, finally I understood your arguments about interfaces check, >>>>>> thank >>>>>>>> you for the explanation. >>>>>>>> >>>>>>>> I agree with the proposal to introduce a new method which >>>>>>>> returns proxy >>>>>>>> include the case of locally deployed services. >>>>>>>> >>>>>>>> Also, such a method should be able to work in mode "local services >>>>>>>> preferred", perhaps with load-balancing (in case of multiple >>>>>>>> locally >>>>>>>> deployed instances). This allows our end-users to reach better >>>>>> performance. >>>>>>>> >>>>>>>> On Mon, Mar 2, 2020 at 7:51 PM Denis Mekhanikov >>>>>>>> <[hidden email] >>>>>>>> wrote: >>>>>>>> >>>>>>>>> Vyacheslav, >>>>>>>>> >>>>>>>>> You can't make service interfaces extend >>>>>>>>> *org.apache.ignite.services.Service*. Currently it works >>>>>>>>> perfectly if >>>>>>>>> *org.apache.ignite.services.Service* and a user-defined >>>>>>>>> interface are >>>>>>>>> independent. This is actually the case in our current examples: >>>>>>>>> >>>>>>>>> >>>>>> https://github.com/apache/ignite/blob/master/examples/src/main/java/org/apache/ignite/examples/servicegrid/SimpleMapService.java >>>>>> >>>>>>>>> I mentioned the *Serializable* interface just as an example of an >>>>>>>> interface >>>>>>>>> that can be present, but it's not the one that is going to be >>>>>>>>> called >>>>>> by a >>>>>>>>> user. >>>>>>>>> >>>>>>>>> What I'm trying to say is that there is no way to say whether the >>>>>> service >>>>>>>>> is going to be used through a proxy only, or usage of a local >>>>>> instance is >>>>>>>>> also possible. >>>>>>>>> >>>>>>>>> Vladimir, >>>>>>>>> >>>>>>>>> I don't like the idea, that enabling or disabling of metrics will >>>>>> change >>>>>>>>> the behaviour of the component you collect the metrics for. Such >>>>>>>> behaviour >>>>>>>>> is far from obvious. >>>>>>>>> >>>>>>>>> Nikolay, >>>>>>>>> >>>>>>>>> I agree, that such approach is valid and makes total sense. But >>>>>> making >>>>>>>> the >>>>>>>>> *IgniteServices#serviceProxy()* method always return a proxy >>>>>>>>> instead >>>>>> of a >>>>>>>>> local instance will change the public contract. The javadoc >>>>>>>>> currently >>>>>>>> says >>>>>>>>> the following: >>>>>>>>> >>>>>>>>>> If service is available locally, then local instance is >>>>>>>>>> returned, >>>>>>>>>> otherwise, a remote proxy is dynamically created and provided >>>>>>>>>> for >>>>>> the >>>>>>>>>> specified service. >>>>>>>>> I propose introducing a new method that will always return a >>>>>>>>> service >>>>>>>> proxy >>>>>>>>> regardless of local availability, and deprecating >>>>>>>>> *serviceProxy()* >>>>>> and >>>>>>>>> *service() >>>>>>>>> *methods. What do you think? >>>>>>>>> >>>>>>>>> Denis >>>>>>>>> >>>>>>>>> пн, 2 мар. 2020 г. в 16:08, Nikolay Izhikov >>>>>>>>> <[hidden email]>: >>>>>>>>> >>>>>>>>>> Hello, Vladimir. >>>>>>>>>> >>>>>>>>>>> What if we just provide an option to disable service metrics at >>>>>> all? >>>>>>>>>> I don't think we should create an explicit property for service >>>>>>>> metrics. >>>>>>>>>> We will implement the way to disable any metrics in the scope of >>>>>>>>>> IGNITE-11927 [1]. >>>>>>>>>> >>>>>>>>>>> Usage of a proxy instead of service instances can lead to >>>>>> performance >>>>>>>>>>> degradation for local instances, which is another argument >>>>>> against >>>>>>>> such >>>>>>>>>> change. >>>>>>>>>> >>>>>>>>>> As far as I know, many and many modern frameworks use a proxy >>>>>> approach. >>>>>>>>>> Just to name one - Spring framework works with the proxy. >>>>>>>>>> >>>>>>>>>> We should measure the impact on the performance that brings >>>>>>>> proxy+metric >>>>>>>>>> and after it make the decision on local service metrics >>>>>> implementation. >>>>>>>>>> Vladimir, can you, as a contributor of this task make this >>>>>> measurement? >>>>>>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-11927 >>>>>>>>>> >>>>>>>>>> пн, 2 мар. 2020 г. в 12:56, Vladimir Steshin >>>>>>>>>> <[hidden email]>: >>>>>>>>>> >>>>>>>>>>> Denis, Vyacheslav, hi. >>>>>>>>>>> >>>>>>>>>>> What if we just provide an option to disable service metrics at >>>>>> all? >>>>>>>> It >>>>>>>>>>> would keep direct references for local services. Also, we can >>>>>> make >>>>>>>>>> service >>>>>>>>>>> metrics disabled by default to keep current code working. A >>>>>> warning >>>>>>>> of >>>>>>>>>>> local service issues will be set with the option. >>>>>>>>>>> >>>>>>>>>>> пн, 2 мар. 2020 г. в 11:26, Vyacheslav Daradur < >>>>>> [hidden email] >>>>>>>>> : >>>>>>>>>>>>>> Moreover, I don't see a way of implementing such a check. >>>>>> Are >>>>>>>> you >>>>>>>>>>> going >>>>>>>>>>>> to look just for any interface? What about Serializable? Will >>>>>> it >>>>>>>> do? >>>>>>>>>>>> The check should look for the interface which implements >>>>>>>>>>>> "org.apache.ignite.services.Service", it covers the >>>>>> requirement to >>>>>>>> be >>>>>>>>>>>> Serializable. >>>>>>>>>>>> >>>>>>>>>>>>>> For now though the best thing we can do is to calculate >>>>>> remote >>>>>>>>>>>> invocations only, since all of them go through a proxy. >>>>>>>>>>>> >>>>>>>>>>>> Let's introduce a system property to manage local services >>>>>>>>> monitoring: >>>>>>>>>>>> - local services monitoring will be disabled by default - to >>>>>> avoid >>>>>>>>> any >>>>>>>>>>>> backward compatibility issues; >>>>>>>>>>>> - local services monitoring can be enabled runtime with a >>>>>>>>>>>> known >>>>>>>>>>> limitation >>>>>>>>>>>> for new services for example; >>>>>>>>>>>> Moreover, if we introduce such a feature flag to >>>>>>>>> ServiceConfiguration - >>>>>>>>>>>> the new feature can be enabled per service separately. >>>>>>>>>>>> >>>>>>>>>>>> What do you think? >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> On Mon, Mar 2, 2020 at 12:33 AM Denis Mekhanikov < >>>>>>>>>> [hidden email]> >>>>>>>>>>>> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> Vladimir, Slava, >>>>>>>>>>>>> >>>>>>>>>>>>> In general, I like the idea of abstracting the service >>>>>> deployment >>>>>>>>> from >>>>>>>>>>>>> its usage, but there are some backward-compatibility >>>>>>>> considerations >>>>>>>>>> that >>>>>>>>>>>>> won't let us do so. >>>>>>>>>>>>> >>>>>>>>>>>>> Or we can declare usage of services without interfaces >>>>>> incorrect >>>>>>>>>>>>> I don't think we can introduce a requirement for all services >>>>>> to >>>>>>>>> have >>>>>>>>>> an >>>>>>>>>>>>> interface, unfortunately. Such change can potentially break >>>>>>>> existing >>>>>>>>>>> code, >>>>>>>>>>>>> since such requirement doesn't exist currently. >>>>>>>>>>>>> Moreover, I don't see a way of implementing such a check. Are >>>>>> you >>>>>>>>>> going >>>>>>>>>>>>> to look just for any interface? What about Serializable? Will >>>>>> it >>>>>>>> do? >>>>>>>>>>>>> Usage of a proxy instead of service instances can lead to >>>>>>>>> performance >>>>>>>>>>>>> degradation for local instances, which is another argument >>>>>> against >>>>>>>>>> such >>>>>>>>>>>>> change. >>>>>>>>>>>>> >>>>>>>>>>>>> I think, it will make sense to make all service invocations >>>>>> work >>>>>>>>>> through >>>>>>>>>>>>> a proxy in Ignite 3. >>>>>>>>>>>>> For now though the best thing we can do is to calculate >>>>>>>>>>>>> remote >>>>>>>>>>>>> invocations only, since all of them go through a proxy. >>>>>>>>>>>>> Another option is to provide a simple way for a user to >>>>>> account >>>>>>>> the >>>>>>>>>>>>> service invocations themselves. >>>>>>>>>>>>> >>>>>>>>>>>>> What do you guys think? >>>>>>>>>>>>> >>>>>>>>>>>>> Denis >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> вт, 25 февр. 2020 г. в 16:50, Vyacheslav Daradur < >>>>>>>>> [hidden email] >>>>>>>>>>> : >>>>>>>>>>>>>> It is not a change of public API from my point of view. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Also, there is a check to allow getting proxy only for an >>>>>>>>> interface, >>>>>>>>>>> not >>>>>>>>>>>>>> implementation. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Denis, what do you think? >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> вт, 25 февр. 2020 г. в 16:28, Vladimir Steshin < >>>>>>>> [hidden email] >>>>>>>>>> : >>>>>>>>>>>>>>> Vyacheslav, this is exactly what I found. I'm doing [1] >>>>>> (metrics >>>>>>>>> for >>>>>>>>>>>>>>> services) and realized I have to wrap local calls by a >>>>>> proxy. Is >>>>>>>>> it >>>>>>>>>> a >>>>>>>>>>>>>>> change of public API and should come with major release >>>>>> only? Or >>>>>>>>> we >>>>>>>>>>> can >>>>>>>>>>>>>>> declare usage of services without interfaces incorrect? >>>>>>>>>>>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-12464 >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> вт, 25 февр. 2020 г. в 16:17, Vyacheslav Daradur < >>>>>>>>>> [hidden email] >>>>>>>>>>>> : >>>>>>>>>>>>>>>> {IgniteServices#service(String name)} returns direct >>>>>> reference >>>>>>>> in >>>>>>>>>> the >>>>>>>>>>>>>>>> current implementation. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> So, class casting should work for your example: >>>>>>>>>>>>>>>> >>>>>> ((MyServiceImpl)ignite.services().service(“myService”)).bar(); >>>>>>>>>>>>>>>> It is safer to use an interface instead of an >>>>>> implementation, >>>>>>>>> there >>>>>>>>>>> is >>>>>>>>>>>>>>>> no guarantee that in future releases direct link will be >>>>>>>>> returned, >>>>>>>>>> a >>>>>>>>>>>>>>>> service instance might be wrapped for monitoring for >>>>>> example. >>>>>>>>>>>>>>>> On Tue, Feb 25, 2020 at 4:09 PM Vladimir Steshin < >>>>>>>>>> [hidden email] >>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Vyacheslav, Hi. >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> I see. But can we consider 'locally deployed service' >>>>>>>>>>>>>>>>> is a >>>>>>>> proxy >>>>>>>>>>> too, >>>>>>>>>>>>>>>>> not direct reference? What if I need to wrap it? This >>>>>> would be >>>>>>>>>> local >>>>>>>>>>>>>>>>> service working via proxy or null. >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> вт, 25 февр. 2020 г. в 16:03, Vyacheslav Daradur < >>>>>>>>>>> [hidden email] >>>>>>>>>>>>>>>>>> : >>>>>>>>>>>>>>>>>> Hi, Vladimir >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> The answer is in API docs: "Gets *locally deployed >>>>>> service* >>>>>>>>> with >>>>>>>>>>>>>>>>>> specified name." [1] >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> That means {IgniteServices#service(String name)} returns >>>>>> only >>>>>>>>>>>>>>>>>> locally deployed instance or null. >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> {IgniteServices#serviceProxy(…)} returns proxy to call >>>>>>>>> instances >>>>>>>>>>>>>>>>>> across the cluster. Might be used for load-balancing. >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> [1] >>>>>>>>>>>>>>>>>> >>>>>> https://github.com/apache/ignite/blob/56975c266e7019f307bb9da42333a6db4e47365e/modules/core/src/main/java/org/apache/ignite/IgniteServices.java#L569 >>>>>> >>>>>>>>>>>>>>>>>> On Tue, Feb 25, 2020 at 3:51 PM Vladimir Steshin < >>>>>>>>>>> [hidden email]> >>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> Hello, Igniters. >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> Previous e-mail was with wrong topic ' >>>>>> [hidden email]' >>>>>>>> :) >>>>>>>>>>>>>>>>>>> I got a question what exactly >>>>>> IgniteServices#service(String >>>>>>>>>> name) >>>>>>>>>>>>>>>>>>> is supposed to return: reference to the object or a >>>>>> proxy >>>>>>>> for >>>>>>>>>>> some reason >>>>>>>>>>>>>>>>>>> like IgniteServices#serviceProxy(…)? Vyacheslav D., can >>>>>> you >>>>>>>>> tell >>>>>>>>>>> me your >>>>>>>>>>>>>>>>>>> opinion? >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> public interface MyService { >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> public void foo(); >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> } >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> public class MyServiceImpl implements Service, >>>>>> MyService { >>>>>>>>>>>>>>>>>>> @Override public void foo(){ … } >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> public void bar(){ … }; >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> } >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> // Is it required to support >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> MyServiceImpl srvc = >>>>>> ignite.services().service(“myService”); >>>>>>>>>>>>>>>>>>> srvc.foo(); >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> srvc.bar(); >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> // Or is the only correct way: >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> MyService srvc = >>>>>>>>>>>>>>>>>>> ignite.services().service(“myService”); >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> srvc.foo(); >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> -- >>>>>>>>>>>>>>>>>> Best Regards, Vyacheslav D. >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> -- >>>>>>>>>>>>>>>> Best Regards, Vyacheslav D. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> -- >>>>>>>>>>>>>> Best Regards, Vyacheslav D. >>>>>>>>>>>>>> >>>>>>>>>>>> -- >>>>>>>>>>>> Best Regards, Vyacheslav D. >>>>>>>>>>>> >>>>>>>> -- >>>>>>>> Best Regards, Vyacheslav D. >>>>>>>> |
Free forum by Nabble | Edit this page |