Proposal of new event QUERY_EXECUTION_EVENT

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

Proposal of new event QUERY_EXECUTION_EVENT

Maksim Timonin
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
Reply | Threaded
Open this post in threaded view
|

Re: Proposal of new event QUERY_EXECUTION_EVENT

dmagda
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 <[hidden email]> 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
>
Reply | Threaded
Open this post in threaded view
|

Re: Proposal of new event QUERY_EXECUTION_EVENT

Maksim Timonin
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 <[hidden email]> 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 <[hidden email]>
> 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
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: Proposal of new event QUERY_EXECUTION_EVENT

dmagda
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 <[hidden email]>
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 <[hidden email]> 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 <[hidden email]>
> > 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
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: Proposal of new event QUERY_EXECUTION_EVENT

Stanislav Lukyanov
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 <[hidden email]> 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 <[hidden email]>
> 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 <[hidden email]> 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 <[hidden email]>
>>> 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
>>>>
>>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: Proposal of new event QUERY_EXECUTION_EVENT

Юрий
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 <[hidden email]>:

> 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 <[hidden email]> 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 <[hidden email]>
> > 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 <[hidden email]> 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 <[hidden email]>
> >>> 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
Reply | Threaded
Open this post in threaded view
|

Re: Proposal of new event QUERY_EXECUTION_EVENT

Maksim Timonin
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 Юрий <[hidden email]> 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 <[hidden email]>:
>
> > 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 <[hidden email]> 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 <[hidden email]>
> > > 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 <[hidden email]>
> 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 <[hidden email]
> >
> > >>> 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
>
Reply | Threaded
Open this post in threaded view
|

Re: Proposal of new event QUERY_EXECUTION_EVENT

Dmitrii Ryabov
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 <[hidden email]>:

>
> 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 Юрий <[hidden email]> 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 <[hidden email]>:
> >
> > > 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 <[hidden email]> 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 <[hidden email]>
> > > > 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 <[hidden email]>
> > 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 <[hidden email]
> > >
> > > >>> 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
> >
Reply | Threaded
Open this post in threaded view
|

Re: Proposal of new event QUERY_EXECUTION_EVENT

Dmitrii Ryabov
Any objections to create a separate event, which will be fired before
executing a query?

ср, 2 сент. 2020 г. в 22:33, Dmitrii Ryabov <[hidden email]>:

>
> 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 <[hidden email]>:
> >
> > 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 Юрий <[hidden email]> 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 <[hidden email]>:
> > >
> > > > 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 <[hidden email]> 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 <[hidden email]>
> > > > > 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 <[hidden email]>
> > > 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 <[hidden email]
> > > >
> > > > >>> 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
> > >
Reply | Threaded
Open this post in threaded view
|

Re: Proposal of new event QUERY_EXECUTION_EVENT

dmagda
Ignite SQL experts, could you please step in?

-
Denis


On Mon, Sep 7, 2020 at 1:53 PM Dmitrii Ryabov <[hidden email]> wrote:

> Any objections to create a separate event, which will be fired before
> executing a query?
>
> ср, 2 сент. 2020 г. в 22:33, Dmitrii Ryabov <[hidden email]>:
> >
> > 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 <[hidden email]>:
> > >
> > > 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 Юрий <[hidden email]>
> 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 <
> [hidden email]>:
> > > >
> > > > > 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 <[hidden email]> 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 <
> [hidden email]>
> > > > > > 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 <[hidden email]
> >
> > > > 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 <
> [hidden email]
> > > > >
> > > > > >>> 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
> > > >
>
Reply | Threaded
Open this post in threaded view
|

Re: Proposal of new event QUERY_EXECUTION_EVENT

Юрий
In reply to this post by Dmitrii Ryabov
Dmitrii, seems you are right and we can go with new separate event

пн, 7 сент. 2020 г. в 23:53, Dmitrii Ryabov <[hidden email]>:

> Any objections to create a separate event, which will be fired before
> executing a query?
>
> ср, 2 сент. 2020 г. в 22:33, Dmitrii Ryabov <[hidden email]>:
> >
> > 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 <[hidden email]>:
> > >
> > > 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 Юрий <[hidden email]>
> 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 <
> [hidden email]>:
> > > >
> > > > > 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 <[hidden email]> 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 <
> [hidden email]>
> > > > > > 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 <[hidden email]
> >
> > > > 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 <
> [hidden email]
> > > > >
> > > > > >>> 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
> > > >
>


--
Живи с улыбкой! :D
Reply | Threaded
Open this post in threaded view
|

Re: Proposal of new event QUERY_EXECUTION_EVENT

Dmitrii Ryabov
Ok, I created a ticket [1].

[1] https://issues.apache.org/jira/browse/IGNITE-13450

пн, 14 сент. 2020 г. в 14:59, Юрий <[hidden email]>:

>
> Dmitrii, seems you are right and we can go with new separate event
>
> пн, 7 сент. 2020 г. в 23:53, Dmitrii Ryabov <[hidden email]>:
>
> > Any objections to create a separate event, which will be fired before
> > executing a query?
> >
> > ср, 2 сент. 2020 г. в 22:33, Dmitrii Ryabov <[hidden email]>:
> > >
> > > 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 <[hidden email]>:
> > > >
> > > > 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 Юрий <[hidden email]>
> > 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 <
> > [hidden email]>:
> > > > >
> > > > > > 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 <[hidden email]> 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 <
> > [hidden email]>
> > > > > > > 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 <[hidden email]
> > >
> > > > > 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 <
> > [hidden email]
> > > > > >
> > > > > > >>> 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
> > > > >
> >
>
>
> --
> Живи с улыбкой! :D
Reply | Threaded
Open this post in threaded view
|

Re: Proposal of new event QUERY_EXECUTION_EVENT

Dmitrii Ryabov
Юрий, can you take a look?
JIRA - https://issues.apache.org/jira/browse/IGNITE-13450
PR - https://github.com/apache/ignite/pull/8252

вт, 15 сент. 2020 г. в 08:59, Dmitrii Ryabov <[hidden email]>:

>
> Ok, I created a ticket [1].
>
> [1] https://issues.apache.org/jira/browse/IGNITE-13450
>
> пн, 14 сент. 2020 г. в 14:59, Юрий <[hidden email]>:
> >
> > Dmitrii, seems you are right and we can go with new separate event
> >
> > пн, 7 сент. 2020 г. в 23:53, Dmitrii Ryabov <[hidden email]>:
> >
> > > Any objections to create a separate event, which will be fired before
> > > executing a query?
> > >
> > > ср, 2 сент. 2020 г. в 22:33, Dmitrii Ryabov <[hidden email]>:
> > > >
> > > > 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 <[hidden email]>:
> > > > >
> > > > > 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 Юрий <[hidden email]>
> > > 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 <
> > > [hidden email]>:
> > > > > >
> > > > > > > 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 <[hidden email]> 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 <
> > > [hidden email]>
> > > > > > > > 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 <[hidden email]
> > > >
> > > > > > 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 <
> > > [hidden email]
> > > > > > >
> > > > > > > >>> 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
> > > > > >
> > >
> >
> >
> > --
> > Живи с улыбкой! :D