ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dmitrii Ryabov <somefire...@gmail.com>
Subject Re: Proposal of new event QUERY_EXECUTION_EVENT
Date Wed, 02 Sep 2020 19:33:15 GMT
I agree with Max, we need to add a separate event for starting query
execution, and EVT_CACHE_QUERY_EXECUTED shouldn't be deprecated,
because it is another case - it is fired when cache query was
successfully finished.

> Would the event notification be synchronous? In which thread?
As Max said, synchronicity depends on implementation. As I see, we
don't use a separate thread for any record calls.

> What happens in case the event listener fails?
Exceptions are logged by `U.error(...)` call. Errors are thrown out.

> Should we discuss this within this topic?
I suggest to separate adding a new event and configuring existing events.

пн, 20 июл. 2020 г. в 14:37, Max Timonin <timonin.maxim@gmail.com>:
>
> Looks like EVT_CACHE_QUERY_EXECUTED just works for different use cases:
> 1. it relates to a specific cache (Event for SQL queries looks different as
> it could contain multiple caches or none of them);
> 2. Also the EVT_CACHE_QUERY_EXECUTED event fires multiple times for
> distributed queries, see GridMapQueryExecutor class (For SQL query it's
> required to fire once independently on how many nodes are affected).
>
> So there are different patterns for events. I think
> EVT_CACHE_QUERY_EXECUTED should not be deprecated or changed.
>
> > What happens in case the event listener fails?
> > Would the event notification be synchronous?
> It depends on how other events are implemented. As I see for the
> EVT_CACHE_QUERY_EXECUTED event - it's synchronous, and listener errors
> aren't handled.
>
> I think these questions are related to GridEventStorageManager as it just
> provides an API for recording events. Internal implementations (sync
> / async, error handling) is not related to an event, is it?
>
> > I have some doubts about provide text of a query even with
> hidden arguments, probably it should be configured due to it could lead
> to security leak
> Currently event EVT_CACHE_QUERY_EXECUTED provides a sql clause without
> limitations. If we're going to provide some restrictions it will require
> additional investigation. I see at least 2 configurations here:
> 1. Ignite can be configured to hide clase, params only or nothing for all
> listeners;
> 2. Only authorized listeners can subscribe to the event.
>
> Should we discuss this within this topic?
>
> On Mon, Jul 20, 2020 at 1:55 PM Юрий <jury.gerzhedowich@gmail.com> wrote:
>
> > In my opinion existing events EVT_CACHE_QUERY_EXECUTION_STARTED should be
> > deprecated and added two new for start and for end of queries which should
> > cover all SQL query types.
> > I have some doubts about provide text of a query even with hidden
> > arguments, probably it should be configured due to it could lead to
> > security leak
> >
> > пн, 20 июл. 2020 г. в 12:49, Stanislav Lukyanov <stanlukyanov@gmail.com>:
> >
> > > Maksim,
> > >
> > > Can we change the EVT_CACHE_QUERY_EXECUTED to fire earlier? Or should
> > > there be an EVT_CACHE_QUERY_EXECUTION_STARTED for the query start, while
> > > the old event would continue to work for query finish?
> > > I think the new event needs to either reuse the old one, or be its mirror
> > > for the query start. It also means that we probably should resolve the
> > > issues you've listed.
> > >
> > > Would the event notification be synchronous? In which thread?
> > Asynchronous
> > > is generally preferred - would it work?
> > >
> > > What happens in case the event listener fails?
> > >
> > > Thanks,
> > > Stan
> > >
> > > > On 16 Jul 2020, at 18:49, Denis Magda <dmagda@apache.org> wrote:
> > > >
> > > > Taras, Yury, Ivan,
> > > >
> > > > Could you please join this thread and share your thoughts? Do we
> > already
> > > > have any plans on tracking of the DDL and DML queries?
> > > >
> > > > -
> > > > Denis
> > > >
> > > >
> > > > On Wed, Jul 15, 2020 at 12:09 AM Max Timonin <timonin.maxim@gmail.com>
> > > > wrote:
> > > >
> > > >> Hi Denis, thanks for the answer!
> > > >>
> > > >> We already checked EVT_CACHE_QUERY_EXECUTED and found that it works
> > > only in
> > > >> cases:
> > > >> 1. Scan queries and Select queries (common pattern is access to cache
> > > >> data);
> > > >> 2. This event triggers only if query execution succeeds, in case of
> > > failure
> > > >> while execution this event won't fire.
> > > >>
> > > >> Our additional requirements are to protocol queries:
> > > >> 1. that aren't cache related (for example, alter user);
> > > >> 2. that relate to multiple caches (while EVT_CACHE_QUERY_EXECUTED
have
> > > >> field cacheName related to specific cache);
> > > >> 3. we need to protocol also DDL and DML queries.
> > > >>
> > > >> Regards,
> > > >> Maksim
> > > >>
> > > >> On Tue, Jul 14, 2020 at 10:20 PM Denis Magda <dmagda@apache.org>
> > wrote:
> > > >>
> > > >>> Hi Max,
> > > >>>
> > > >>> Could you check if the EVT_CACHE_QUERY_EXECUTED event is what
you're
> > > >>> looking for?
> > > >>>
> > > >>>
> > > >>
> > >
> > https://www.gridgain.com/docs/latest/developers-guide/events/events#cache-query-events
> > > >>>
> > > >>> -
> > > >>> Denis
> > > >>>
> > > >>>
> > > >>> On Fri, Jul 10, 2020 at 3:54 AM Max Timonin <timonin.maxim@gmail.com
> > >
> > > >>> wrote:
> > > >>>
> > > >>>> Hi Igniters!
> > > >>>>
> > > >>>> We're going to protocol all input SQL queries from our users.
> > > Currently
> > > >>>> there is no such mechanism in Ignite to use for it. So we're
> > proposing
> > > >> to
> > > >>>> add a new event: QUERY_EXECUITION_EVENT.
> > > >>>>
> > > >>>> Requirements for the event:
> > > >>>> 1. If this event fires it means that a query is correct and
will be
> > > >>>> executed (and failed only in exceptional cases);
> > > >>>>
> > > >>>> 2. Event fires for all query types;
> > > >>>>
> > > >>>> 3. Required fields are:
> > > >>>> - text of a query (with hidden arguments);
> > > >>>> - arguments of query;
> > > >>>> - query type;
> > > >>>> - node id.
> > > >>>>
> > > >>>> Looks that this event should go along with `runningQryMgr::register`
> > > in
> > > >>>> class `IgniteH2Indexing` as this method invoked for all input
> > queries
> > > >>> too.
> > > >>>>
> > > >>>> What do you think?
> > > >>>>
> > > >>>> Regards,
> > > >>>> Maksim
> > > >>>>
> > > >>>
> > > >>
> > >
> > >
> >
> > --
> > Живи с улыбкой! :D
> >

Mime
View raw message