Similar question about .NET. I'm doing the sub-ticket with metrics
of the platform services. Should we return proxy every tome by
/IServices.GerServiceProxy()/ and probably deprecate
/IServices.GetService()/. Same problems with variate the behavior of
/GerServiceProxy()/ and statistics corruption (if enabled) with
/GetService()/.
24.01.2022 22:27, Valentin Kulichenko пишет:
Vladimir,
Agreed. My point is that the actual issue we want to fix is broken metrics.
Thus, we should have a single ticket/patch *about the broken metrics* that
incorporates all required changes, as well as relevant documentation
updates.
I don't think we need a vote. I will also change mine to +1 in the existing
thread.
-Val
On Mon, Jan 24, 2022 at 11:23 AM Vladimir Steshin<vlads...@gmail.com>
wrote:
Sure. This is a simple change. These 2 fixes could go together. And
the doc already has notes of service statistics traits. Ok, might be
even better.
I'm not sure whether we need extra voting for deprecating
'service()' and, as earlier and making 'serviceProxy()' return proxy
every time. Several active contributors have said their 'yes' for both
in the thread.
Several active contributors have already said their 'yes'.
23.01.2022 03:50, Valentin Kulichenko пишет:
If we already agreed to deprecate the service() method, then I'm ok with
the change. But I think we should do both fixes together and also clearly
document that using service() can lead to incorrect metrics.
-Val
On Fri, Jan 21, 2022 at 1:13 AM Vladimir Steshin<vlads...@gmail.com>
wrote:
Valentin, there are 2 notable issues:
1) Variate behavior of `/serviceProxy()/` depending on user setting. It
can return both proxy and reference.
2) Service metrics are corrupted by invocations through `/service()/`.
How we can fix:
1) Just return proxy every time.
2) Deprecate `/service()/`. We've already discussed that here: [1].
Please see my previous message in the thread.
[1]https://www.mail-archive.com/dev@ignite.apache.org/msg44062.html
On 20.01.2022 22:48, Valentin Kulichenko wrote:
So the proposed change will not actually fix the issue with metrics,
because it's still possible to get a local instance via the service()
method. At the same time, the change removes an existing performance
optimization.
Let's figure out how to fix the actual problem. If the *only* way to
have
metrics is to have a proxy, then this should be the only way to
interact
with a service. In that case, we need to do something with the
service()
method (deprecate it?). Or probably there are other ways to fix
metrics?
-Val
On Thu, Jan 20, 2022 at 3:32 AM Vladimir Steshin<vlads...@gmail.com>
wrote:
Yes. Invocations from direct reference are not measured. This
method
in the javadoc:
/* <b>NOTE:</b> Statistics are collected only with service proxies
obtaining by methods like/
/* {@link IgniteServices#serviceProxy(String, Class, boolean)} and
won't
work for direct reference of local/
/* services which you can get by, for example, {@link
IgniteServices#service(String)}./
On 20.01.2022 00:20, Valentin Kulichenko wrote:
BTW, there is also the service() method that can only return an
instance
and never returns a proxy. Does it corrupt the metrics as well?
-Val
On Wed, Jan 19, 2022 at 1:09 PM Valentin Kulichenko <
valentin.kuliche...@gmail.com> wrote:
Maxim,
The reason I'm asking is that I don't really understand how client
side
mechanics affect server side metrics (number of executions and their
durations). I feel that we might be fixing a wrong problem.
Could you elaborate on why we count metrics incorrectly when
the serviceProxy() returns an instance of a local service instead of
an
actual proxy?
-Val
On Tue, Jan 18, 2022 at 11:32 PM Maksim Timonin<
timoninma...@apache.org
wrote:
Hi, guys!
this is not a good idea to change the behavior of serviceProxy()
depending on statistics
I think that the patch doesn't change the behavior of
`serviceProxy()`.
This method promises a proxy and it actually returns it. The fact
that
`serviceProxy()` can return non-proxy objects is an internal Ignite
optimization, and users should not rely on this, there is a
separate
method
`service()` for that.
What are the metrics that are being affected by this?
Only service metrics, that calculates duration of service
execution.
Check
this ticket [1]
[1]https://issues.apache.org/jira/browse/IGNITE-12464
On Wed, Jan 19, 2022 at 1:22 AM Valentin Kulichenko <
valentin.kuliche...@gmail.com> wrote:
What are the metrics that are being affected by this?
-Val
On Tue, Jan 18, 2022 at 3:31 AM Вячеслав Коптилин <
slava.kopti...@gmail.com>
wrote:
Hello Igniters,
IMHO, this is not a good idea to change the behavior of
serviceProxy()
depending on statistics (enabled/disabled). It seems
counterintuitive
to
me.
Perhaps, we need to introduce a new method that should always
return
a
proxy to the user service.
Thanks,
Slava.
вт, 28 дек. 2021 г. в 13:57, Pavel Pereslegin<xxt...@gmail.com>:
Hi!
Agree with Maxim.
It seems to me quite normal to return a proxy for a local
instance
in
the case when the user has explicitly enabled statistics
collection
in
the service settings. Those. by default, we do not change the
behavior
and if the collection of metrics is not needed, a local instance
will
be returned. And I also think the javadoc should be changed to
reflect
the new behavior.
So, I'm for 1 + 3.
вт, 28 дек. 2021 г. в 10:51, Maksim Timonin <
timoninma...@apache.org>:
Hi!
I agree that users shouldn't expect a non-proxy when invoking
the
`IgniteServices#serviceProxy()` method. I think it's up to
Ignite
to
return
a non-proxy instance here as possible optimisation. But users
have to
use
interfaces in any case. There is the `IgniteServices#service()`
method
for
explicit return of local instances.
With enabling of metrics we can break users that explicitly
use `#serviceProxy` (proxy!), and then explicitly cast it to an
implementation class. In this case such users will get a
runtime
exception.
I think we can write a good javadoc for
`ServiceConfiguration#setEnableMetrics()`, it should mention
that
it
works
only with proxy, and it doesn't collect metrics with non-proxy
usages
with
`IgniteService#service()`.
So, I propose to proceed with two solutions - 1 and 3: fix docs
for
`#serviceProxy()` and provide detailed javadocs
for `ServiceConfiguration#setEnableMetrics()`.
If some users will enable metrics (even with such docs!) and
will
be
using
casting proxy(!) to an implementation, then they will get a
runtime
exception. But I believe that it is an obvious failure, and it
should
be
fixed on the user side.
On Mon, Dec 27, 2021 at 10:26 PM Vladimir Steshin <
vlads...@gmail.com>
wrote:
Hi, Igniters.
I'd like to suggest modifying
`/IgniteServices.serviceProxy(String
name,
Class<? super T> svcItf, boolean sticky)/` so that it may
return
proxy
even for local service. Motivation: service metrics [1]. To
measure
method call we need to wrap service somehow. Also, the method
name
says
`proxy`. For local one we return direct instance. Misleading.
Solutions:
1) Let’s return a proxy (`/GridServiceProxy/`) once we need.
Let’s
change the javadoc to say `@return Proxy over service’. Simply
works.
Pros: invocation like `/MyServiceImpl svc =
serviceProxy(«myservice»,
MyService.class)`/ would fail. Wierd usage to me. But
possible.
2) Introduce a new method with forced-proxy flag
like`IgniteSerives.serviceProxy(…, boolead focedProxy)`. And
add a
warning to other service-obtain methods like: «`/You enabled
service
metrics but it doesn’t work for local service instanse. Use
forced-proxy
serviceProxy(…, boolean forcedProxy)/` Pros: we’ve already
have
about
5-6 methods to get services in `/IgniteServices/`. I doubt we
need
one
more.
3) Fix the documentation so that it tells the service metrics
would
work
only with proxy. Pros: service metrics just won’t work for
local
services. Suddenly.
My vote is for #1: let's use a proxy. WDYT?
[1]https://issues.apache.org/jira/browse/IGNITE-12464
