I think Alexey is right: QueryMetrics and QueryDetailsMetrics are
substantially different and we will not be able to merge them to a single
class without breaking compatibility.

Probably it really makes sense to keep them separate for now and
deprecate older QueryMetrics. Though I don't really like the name
"QueryDetailsMetrics".
May be we can come up with something better?

Sergi

2016-10-26 4:31 GMT+03:00 Alexey Kuznetsov <[email protected]>:

> Dmitry,
>
> Current QueryMetrics collect metrics for all queries executed on cache
> without distinguish them,
>  i.e. if you execute SCAN query and SQL_FIELDS_QUERY you will get single
> metrics with 2 executions  (and other counters).
>
> I _added_ metrics that are grouped by query type and query text.
> So, in my new metrics the above example will result in two metrics:
> SCAN - 1 execution (and other counters).
> SQL_FIELDS_QUERY - 1 execution (and other counters).
>
> Actually I think it is a good idea to replace QueryMetrics API we already
> have with new functionality.
> From my point of view current QueryMetrics is not very useful because users
> could only get very general metrics from it.
>
> But how we could maintain backward compatibility if needed?
> Or we could implement QueryDetailsMetrics  for Ignite 1.8 and mark
> QueryMetrics  as deprecated and drop in Ignite 2.0?
>
> What do you think?
>
>
> On Wed, Oct 26, 2016 at 7:15 AM, Dmitriy Setrakyan <[email protected]>
> wrote:
>
> > Alexey,
> >
> > I am a bit confused. I just looked at QueryMetrics API and it looks like
> a
> > subset of the QueryDetailsMetrics you are suggesting. Why not just expand
> > on QueryMetrics API we already have?
> >
> > D.
> >
> > On Mon, Oct 24, 2016 at 9:41 PM, Alexey Kuznetsov <[email protected]
> >
> > wrote:
> >
> > > Dmitriy, thanks for your feedback.
> > >
> > > >> 1. Are these metrics going to be available as MBean?
> > > >> 4. Do we need an MBean for queryMetricsHIstor()?
> > > Yes. I will create sub-ticket for that or implement as part of
> > IGNITE-3443
> > > if it is easy to do.
> > >
> > > >> 2. QueryDetailsMetrics -> QueryMetrics
> > > This name is already busy. We already have interface with name
> > > QueryMetrics.
> > >
> > > >>3. getCompletions() -> getSuccesses() - to be consistent with
> > >  getFailures()
> > > Will rename.
> > >
> > >
> > >
> > > On Tue, Oct 25, 2016 at 12:00 AM, Dmitriy Setrakyan <
> > [email protected]
> > > >
> > > wrote:
> > >
> > > > Several comments:
> > > >
> > > >    1. Are these metrics going to be available as MBean?
> > > >    2. QueryDetailsMetrics -> QueryMetrics
> > > >    3. getCompletions() -> getSuccesses() - to be consistent with
> > > >    getFailures()
> > > >    4. Do we need an MBean for queryMetricsHIstor()?
> > > >
> > > > D.
> > > >
> > > > On Mon, Oct 24, 2016 at 3:58 AM, Alexey Kuznetsov <
> > [email protected]
> > > >
> > > > wrote:
> > > >
> > > > > Igniters,
> > > > >
> > > > > I'm working on IGNITE-3443: Implement collecting what SQL
> statements
> > > > > executed on cluster and their metrics [1]. (I also updated issue
> > > > > description with text below.)
> > > > >
> > > > > And I would like that my changes of public API looks good.
> > > > >
> > > > > Please, give your feedback.
> > > > >
> > > > > So, I introduced interface org.apache.ignite.cache.query.
> > > QueryDetailsMetrics
> > > > with
> > > > > metrics:
> > > > >
> > > > > /**
> > > > >  * Query metrics aggregated by query type and its textual
> > > representation.
> > > > >  */
> > > > > public interface QueryDetailsMetrics {
> > > > >     /**
> > > > >      * @return Query type.
> > > > >      */
> > > > >     public String getQueryType();
> > > > >
> > > > >     /**
> > > > >      * @return Textual representation of query.
> > > > >      */
> > > > >     public String getQuery();
> > > > >
> > > > >     /**
> > > > >      * @return Cache where query was executed.
> > > > >      */
> > > > >     public String getCache();
> > > > >
> > > > >     /**
> > > > >      * Gets total number execution of query.
> > > > >      *
> > > > >      * @return Number of executions.
> > > > >      */
> > > > >     public int getExecutions();
> > > > >
> > > > >     /**
> > > > >      * Gets number of completed execution of query.
> > > > >      *
> > > > >      * @return Number of completed executions.
> > > > >      */
> > > > >     public int getCompletions();
> > > > >
> > > > >     /**
> > > > >      * Gets number of times a query execution failed.
> > > > >      *
> > > > >      * @return Number of times a query execution failed.
> > > > >      */
> > > > >     public int getFailures();
> > > > >
> > > > >     /**
> > > > >      * Gets minimum execution time of query.
> > > > >      *
> > > > >      * @return Minimum execution time of query.
> > > > >      */
> > > > >     public long getMinimumTime();
> > > > >
> > > > >     /**
> > > > >      * Gets maximum execution time of query.
> > > > >      *
> > > > >      * @return Maximum execution time of query.
> > > > >      */
> > > > >     public long getMaximumTime();
> > > > >
> > > > >     /**
> > > > >      * Gets average execution time of query.
> > > > >      *
> > > > >      * @return Average execution time of query.
> > > > >      */
> > > > >     public double getAverageTime();
> > > > >
> > > > >     /**
> > > > >      * Gets total time of all query executions.
> > > > >      *
> > > > >      * @return Total time of all query executions.
> > > > >      */
> > > > >     public long getTotalTime();
> > > > >
> > > > >     /**
> > > > >      * Gets latest query start time.
> > > > >      *
> > > > >      * @return Latest time query was stared.
> > > > >      */
> > > > >     public long getLastStartTime();
> > > > > }
> > > > >
> > > > >
> > > > > And added method on org.apache.ignite.IgniteCache:
> > > > >
> > > > > /**
> > > > >  * Gets query metrics details.
> > > > >  *
> > > > >  * @return Metrics.
> > > > >  */
> > > > > public Collection<? extends QueryDetailsMetrics>
> > queryMetricsHistory();
> > > > >
> > > > >
> > > > > And also I added new property on org.apache.ignite.configuration.
> > > > CacheConfiguration:
> > > > >
> > > > > /**
> > > > >  * Gets size of queries metrics history that will be stored in
> memory
> > > > for monitoring purposes.
> > > > >  * If {@code 0} then history will not be collected.
> > > > >  * Note, Larger number may lead to higher memory consumption.
> > > > >  *
> > > > >  * @return Maximum number of query metrics that will be stored in
> > > memory.
> > > > >  */
> > > > > public int getQueryMetricsHistorySize() {
> > > > >     return qryMetricsHistSz;
> > > > > }
> > > > >
> > > > > /**
> > > > >  * Sets size of queries metrics history that will be stored in
> memory
> > > > for monitoring purposes.
> > > > >  *
> > > > >  * @param qryMetricsHistSz Maximum number of latest queries metrics
> > > that
> > > > will be stored in memory.
> > > > >  * @return {@code this} for chaining.
> > > > >  */
> > > > > public CacheConfiguration<K, V> setQueryMetricsHistorySize(int
> > > > qryMetricsHistSz) {
> > > > >     this.qryMetricsHistSz = qryMetricsHistSz;
> > > > >
> > > > >     return this;
> > > > > }
> > > > >
> > > > >
> > > > >
> > > > > [1] https://issues.apache.org/jira/browse/IGNITE-3443
> > > > >
> > > > > --
> > > > > Alexey Kuznetsov
> > > > >
> > > >
> > >
> > >
> > >
> > > --
> > > Alexey Kuznetsov
> > >
> >
>
>
>
> --
> Alexey Kuznetsov
> GridGain Systems
> www.gridgain.com
>

Reply via email to