Hi, everyone.
I've done some benchmarks and wonder whether we need to disable service
metrics somehow. As Denis suggested, proxies might be involved for local
services. If I enable current proxy for local services, I see the flowing
cases:
1) Service proxy is too slow with trivial operations to care about metrics
performance.
2) Service metrics do not hit performance significantly or do not hit at
all on real operations.
I doubt other implementation of service proxy could bring serious changes
and would worth. Do we need to care about performance hit of the service
metrics and think how to disable?
Without metrics:
- Test [3] : proxy [2] is 6.5 times slower than local [1] (11kk ops/s VS
72kk ops/s)
- Test [4] : proxy is 19% slower than local (990k ops/s VS 1182k ops/s)
- Test [5] : proxy is not slower (same 250k ops/s)
With metrics:
- Test [3] : proxy [2] is 7.5 times slower than local [1] (9.6kk ops/s VS
73kk ops/s)
- Test [4] : proxy is 21% slower than local (995k ops/s VS 1200k ops/s)
- Test [5] : proxy is 10% slower than local (244k ops/s VS 264k ops/s)
private TestService localService [1];
private TestService serviceProxy [2];
private IgniteEx ignite;
private IgniteCache cache;
interface TestService {
Object handleVal(int value);
}
static class TestServiceImpl implements Service, TestService {
…
private IgniteCache cache;
@Override public Object handleVal(int value) {
[3] return randomInt(value);
[4] return cache.get(value);
[5] return cache.getAndPut(randomInt(value), value);
}
}
@Benchmark
public void localService(Blackhole blackhole) throws Exception {
blackhole.consume( localService.handleVal(1+randomInt(MAX_VALUE)) );
}
@Benchmark
public void proxiedService(Blackhole blackhole) throws Exception {
blackhole.consume( serviceProxy.handleVal(1+randomInt(MAX_VALUE)) );
}
@Setup
public void setup() throws Exception {
ignite = (IgniteEx)Ignition.start(configuration("grid0"));
cache = ignite.createCache("cache");
for(int i=0; i<MAX_VALUE; i++) cache.put(i, i); //1 000 000 values
ignite.services().deployNodeSingleton("srv", new TestServiceImpl());
[1] localService = ignite.services().service("srv");
//Current service proxy implementation. Might be used for local
services.
[2] serviceProxy = new GridServiceProxy<>(ignite.cluster(),"srv",
TestService.class, true, 0, ignite.context()).proxy();
}
чт, 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.
> > >
>
