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.
