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 <vlads...@gmail.com> 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 <vlads...@gmail.com> 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 <ag...@apache.org>: > >> > >>> 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 <vlads...@gmail.com> > >>> 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 <daradu...@gmail.com>: > >>>> > >>>>> 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 <dmekhani...@gmail.com > >>>>> 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 <nizhi...@apache.org>: > >>>>>> > >>>>>>> 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 <vlads...@gmail.com>: > >>>>>>> > >>>>>>>> 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 < > >>> daradu...@gmail.com > >>>>>> : > >>>>>>>>>>> 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 < > >>>>>>> dmekhani...@gmail.com> > >>>>>>>>> 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 < > >>>>>> daradu...@gmail.com > >>>>>>>> : > >>>>>>>>>>> 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 < > >>>>> vlads...@gmail.com > >>>>>>> : > >>>>>>>>>>>> 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 < > >>>>>>> daradu...@gmail.com > >>>>>>>>> : > >>>>>>>>>>>>> {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 < > >>>>>>> vlads...@gmail.com > >>>>>>>>>>>>> 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 < > >>>>>>>> daradu...@gmail.com > >>>>>>>>>>>>>>> : > >>>>>>>>>>>>>>> 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 < > >>>>>>>> vlads...@gmail.com> > >>>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Hello, Igniters. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Previous e-mail was with wrong topic ' > >>> daradu...@gmail.com' > >>>>> :) > >>>>>>>>>>>>>>>> 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. > >>>>>
