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 Mon, 07 Sep 2020 20:53:04 GMT
Any objections to create a separate event, which will be fired before
executing a query?

ср, 2 сент. 2020 г. в 22:33, Dmitrii Ryabov <somefireone@gmail.com>:
>
> 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