Vladimir,

Then it looks like we send wrong EVT_CACHE_QUERY_EXECUTED events too, since
we send it for the same cache, as in your example.

Evgenii



пн, 27 авг. 2018 г. в 13:20, Vladimir Ozerov <voze...@gridgain.com>:

> Folks,
>
> Can we leave query details metrics API unchanged, and simply fix how we
> record them? Historically it worked as follows:
> 1) IgniteCache.query() is invoked
> 2) Query is executed, possibly on other caches!
> 3) Metric is incremented for cache from p.1
>
> I.e. our query detail metrics were broken since their first days. I propose
> to fix them as follows:
> 1) Any SQL query() method is invoked (see GridQueryProcessor)
> 2) We collect all cache IDs during parsing (this already happens)
> 3) Record event for all cache IDs through
> GridCacheQueryManager#collectMetrics
>
> This appears to be as the simplest and consistent solution to the problem.
>
> On Tue, Aug 21, 2018 at 1:09 PM Evgenii Zhuravlev <
> e.zhuravlev...@gmail.com>
> wrote:
>
> > Hi Alex,
> >
> > I agree that we can't move all metrics to ignite.metrics() and SPI
> metrics
> > is a good example here. I propose to move at least dataRegionMetrics,
> > dataStorageMetrics to it, since the main API class is not a good place
> for
> > such things. If we will decide to choose option 1 from my first message,
> > then, I will also add QueryDetailMetrics for cacheless queries to this
> new
> > class.
> >
> > пн, 20 авг. 2018 г. в 23:28, Alex Plehanov <plehanov.a...@gmail.com>:
> >
> > > Hi Evgeny,
> > >
> > > Do you propose to move into IgniteMetrics absolutely all Ignite metrics
> > or
> > > just dataRegionMetrics, dataStorageMetrics and queryDetailMetrics? I
> > think
> > > you can't move all metrics into one place. Pluggable components and
> > > different SPI implementations may have their own metric sets, and
> perhaps
> > > it's not such a good idea to try to fit them in one common fixed
> > interface.
> > >
> > > 2018-08-20 18:14 GMT+03:00 Evgenii Zhuravlev <e.zhuravlev...@gmail.com
> >:
> > >
> > > > As for now, metrics are accumulated for cache only when the query is
> > run
> > > > directly over this cache, for example, using ignite.cache("Some
> > > > cache").sqlFieldsQuery("select ... from .."). When a query is started
> > > using
> > > > other APIs, it doesn't detect cache, to which this table belongs and
> > > > doesn't save any metrics.
> > > >
> > > >
> > > >
> > > > 2018-08-17 16:44 GMT+03:00 Vladimir Ozerov <voze...@gridgain.com>:
> > > >
> > > > > Query is not executed on specific cache. It is executed on many
> > caches.
> > > > >
> > > > > On Fri, Aug 17, 2018 at 6:10 AM Dmitriy Setrakyan <
> > > dsetrak...@apache.org
> > > > >
> > > > > wrote:
> > > > >
> > > > > > But internally the SQL query still runs on some cache, no? What
> > > happens
> > > > > to
> > > > > > the metrics accumulated on that cache?
> > > > > >
> > > > > > D.
> > > > > >
> > > > > > On Thu, Aug 16, 2018, 18:51 Alexey Kuznetsov <
> > akuznet...@apache.org>
> > > > > > wrote:
> > > > > >
> > > > > > > Dima,
> > > > > > >
> > > > > > > "cache-less" means that SQL executed directly on SQL engine.
> > > > > > >
> > > > > > > In previous version of Ignite we execute queries via cache:
> > > > > > >
> > > > > > > ignite.cache("Some cache").sqlFieldsQuery("select ... from ..")
> > > > > > >
> > > > > > > In current Ignite we can execute query directly without using
> > cache
> > > > as
> > > > > > > "gateway".
> > > > > > >
> > > > > > > And if we execute query directly, metrics not update.
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > On Fri, Aug 17, 2018 at 4:21 AM Dmitriy Setrakyan <
> > > > > dsetrak...@apache.org
> > > > > > >
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Evgeny, what is a "cache-less" SQL query?
> > > > > > > >
> > > > > > > > D.
> > > > > > > >
> > > > > > > > On Thu, Aug 16, 2018 at 6:36 AM, Evgenii Zhuravlev <
> > > > > > > > e.zhuravlev...@gmail.com
> > > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Hi Igniters,
> > > > > > > > >
> > > > > > > > > I've started to work on adding QueryDetailMetrics for
> > > cache-less
> > > > > SQL
> > > > > > > > > queries(issue
> > > https://issues.apache.org/jira/browse/IGNITE-6677)
> > > > > and
> > > > > > > > found
> > > > > > > > > that it's required to change API. I don't think that adding
> > > > methods
> > > > > > > like
> > > > > > > > > queryDetailMetrics, resetQueryDetailMetrics, as in
> > IgniteCache
> > > to
> > > > > > > Ignite
> > > > > > > > > class is a good idea. So, I see 2 possible solutions here:
> > > > > > > > >
> > > > > > > > > 1. Create IgniteMetrics(ignite.metrics()) and move metrics
> > from
> > > > > > > > > Ignite(like dataRegionMetrics and dataStorageMetrics) and
> > add a
> > > > new
> > > > > > > > > metric "queryDetailMetrics" to it. Of course, old methods
> > will
> > > be
> > > > > > > > > deprecated.
> > > > > > > > >
> > > > > > > > > 2. Finally create Ignite.sql() API, which was already
> > discussed
> > > > > here:
> > > > > > > > > http://apache-ignite-developers.2346864.n4.nabble.
> > > > > > > > >
> com/Rethink-native-SQL-API-in-Apache-Ignite-2-0-td14335.html
> > > > > > > > > and place "queryDetailMetrics" metric there. Here is the
> > ticket
> > > > for
> > > > > > > this
> > > > > > > > > change: https://issues.apache.org/jira/browse/IGNITE-4701
> > > > > > > > >
> > > > > > > > > Personally, I think that the second solution looks better
> in
> > > this
> > > > > > case,
> > > > > > > > > however, moving dataRegionMetrics and dataStorageMetrics to
> > > > > > > > > ignite.matrics() is still a good idea - IMO, Ignite class
> is
> > > not
> > > > > the
> > > > > > > > right
> > > > > > > > > place for them - we shouldn't change our main API class so
> > > often.
> > > > > > > > >
> > > > > > > > > What do you think?
> > > > > > > > >
> > > > > > > > > Thank you,
> > > > > > > > > Evgenii
> > > > > > > > >
> > > > > > > >
> > > > > > > > --
> > > > > > > > Alexey Kuznetsov
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Reply via email to