ignite-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Vladimir Ozerov (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (IGNITE-10754) Query history statistics API
Date Wed, 09 Jan 2019 11:09:00 GMT

    [ https://issues.apache.org/jira/browse/IGNITE-10754?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16738110#comment-16738110

Vladimir Ozerov commented on IGNITE-10754:

[~jooger], my comments:
# We should not deprecate existing functional, as there is nothing wrong with - it just works
on a different level. Moreover, it is concerned with non-SQL queries (e.g. Scan), while this
ticket is only about SQL.
# No need for interfaces and adapters for {{QueryHistoryMetrics}}. Single concrete class would
e enough
# {{QueryHistoryMetrics}} should not be located in public package, as it will only be accessible
from SQL and web console.
# {{QueryHistoryManager}} - it is overkill to have separate manager to track history. Let's
merge it with running queries manager.
# No need to expose {{QueryHistoryManager}} from {{GridQueryProcessor}}
# {{QueryHistoryManager}} - {{clear}} operation cannot be implemented this way, as it is not
thread-safe and may lead to inconsistent state. Instead, both data structures should be encapsulated
in some volatile composite object. And {{clear}} should simply create new such object. This
way we will avoid races when data structure will contain queries created before {{clear}}
# {{QueryHistoryManager.collectMetrics}} - {{metricsFull}} should be called only when it is
need - immediately before shrink
# I am not quite understand the line {{if (qryMetrics.get(entry.key()) != entry)}} - what
is the purpose? From what I see, {{shrink}} method first removes an entry from queue, and
then from the map. So this check looks redundant to me. Otherwise, if we really need it, why
this check is not performed in {{else if (removeLink(node))}} branch?
# {{org.apache.ignite.internal.processors.query.QueryHistoryManager#explain}} - what is wrong
with EXPLAIN queries that we decided to exclude them?
# {{QueryHistoryManager.queryHistoryMetrics}} - implementation looks inefficient: why do we
create intermediate array only to covert it to the list later on? Moreover, why do we covert
queue to list before that (which creates another intermediate ArrayList internally)? Last,
queue -> list coversion is not safe in terms of duplicates - you may get the same entry
twice in case of concurrent query execution. Why can't we just iterate over the map without
any intermediate collections?
# {{RunningQueryManager.unregister}} - we should log only SQL queries, but this doesn't seem
to be the case
# {{RunningQueryManager.unregister}} - no need to add queries to history in case of stop

> Query history statistics API
> ----------------------------
>                 Key: IGNITE-10754
>                 URL: https://issues.apache.org/jira/browse/IGNITE-10754
>             Project: Ignite
>          Issue Type: Task
>          Components: sql
>            Reporter: Yury Gerzhedovich
>            Assignee: Yury Gerzhedovich
>            Priority: Major
>              Labels: iep-29, monitoring
> As of now we have query statistics (*_org.apache.ignite.IgniteCache#queryMetrics_*) ,
but have few issues.
> 1) Duration execution it just time between start execution and return cursor to client
and doesn't include all life time of query.
> 2) It doesn't know about multistatement queries. Such queries participate in statistics
as single query without splitting.
> 3) API to access the statistics expose as depend on cache, however query don't have such
> Need to create parallel similar realization as we already have.
> Use new infrastructure of tracking running queries developed under IGNITE-10621 and
update statistics on unregister phase.
> Expose API on upper level then it placed now. Right place will be written later.
> Old API will be marked as deprecated.

This message was sent by Atlassian JIRA

View raw message