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 |
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 > |
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 > > > |
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 > > > > > > |
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 >>>> >>> >> |
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 |
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 > |
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 > > |
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 > > > |
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 > > > > > |
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 |
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 |
Юрий, 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 |
Free forum by Nabble | Edit this page |