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