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.
