Hi,
I have raised a PR for the below issue. IGNITE-7285 Add default query timeout PR - https://github.com/apache/ignite/pull/6490 Please take a look and share feedback. Regards, Saikat |
Hi Saikat,
I see that a configuration property is added in PR but I do not see how the property is used. Was it done intentionally? Also, we need to decide whether such timeout should be configured per cache or for all caches in one place. For example, we have already TransactionConfiguration#setDefaultTxTimeout which is a global one. Share you thoughts. вс, 21 апр. 2019 г. в 00:38, Saikat Maitra <[hidden email]>: > > Hi, > > I have raised a PR for the below issue. > > IGNITE-7285 Add default query timeout > > PR - https://github.com/apache/ignite/pull/6490 > > Please take a look and share feedback. > > Regards, > Saikat -- Best regards, Ivan Pavlukhin |
Hi Ivan,
Thank you for your email. My understanding from the jira issue was it will be cache level configuration for query default timeout. I need more info on the usage for this config property and if it is shared in this jira issue I can make changes or if there is a separate jira issue I can assign myself. Regards, Saikat On Mon, Apr 22, 2019 at 5:31 AM Павлухин Иван <[hidden email]> wrote: > Hi Saikat, > > I see that a configuration property is added in PR but I do not see > how the property is used. Was it done intentionally? > > Also, we need to decide whether such timeout should be configured per > cache or for all caches in one place. For example, we have already > TransactionConfiguration#setDefaultTxTimeout which is a global one. > > Share you thoughts. > > вс, 21 апр. 2019 г. в 00:38, Saikat Maitra <[hidden email]>: > > > > Hi, > > > > I have raised a PR for the below issue. > > > > IGNITE-7285 Add default query timeout > > > > PR - https://github.com/apache/ignite/pull/6490 > > > > Please take a look and share feedback. > > > > Regards, > > Saikat > > > > -- > Best regards, > Ivan Pavlukhin > |
Hi Saikat,
I think that we should have a discussion and choose a place where a "default query timeout" property will be configured. Generally, I think that a real (user) problem is possibility for queries to execute indefinitely. And I have no doubts that we can improve there. There could be several implementation strategies. One is adding a property to CacheConfiguration. But it opens various questions. E.g. how should it work if we execute SQL JOIN spanning multiple tables (caches)? Also I am concerned about queries executed not via cache.query() method. We have multiple alternative options, e.g. thin clients (IgniteClient.query) or JDBC. I believe that introducing a default timeout for all them is not a bad idea. What do you think? вт, 23 апр. 2019 г. в 03:01, Saikat Maitra <[hidden email]>: > > Hi Ivan, > > Thank you for your email. My understanding from the jira issue was it will > be cache level configuration for query default timeout. > > I need more info on the usage for this config property and if it is shared > in this jira issue I can make changes or if there is a separate jira issue > I can assign myself. > > > Regards, > Saikat > > On Mon, Apr 22, 2019 at 5:31 AM Павлухин Иван <[hidden email]> wrote: > > > Hi Saikat, > > > > I see that a configuration property is added in PR but I do not see > > how the property is used. Was it done intentionally? > > > > Also, we need to decide whether such timeout should be configured per > > cache or for all caches in one place. For example, we have already > > TransactionConfiguration#setDefaultTxTimeout which is a global one. > > > > Share you thoughts. > > > > вс, 21 апр. 2019 г. в 00:38, Saikat Maitra <[hidden email]>: > > > > > > Hi, > > > > > > I have raised a PR for the below issue. > > > > > > IGNITE-7285 Add default query timeout > > > > > > PR - https://github.com/apache/ignite/pull/6490 > > > > > > Please take a look and share feedback. > > > > > > Regards, > > > Saikat > > > > > > > > -- > > Best regards, > > Ivan Pavlukhin > > -- Best regards, Ivan Pavlukhin |
Hi Saikat and Ivan,
I think that properties that related to SQL should not be configured on caches. We already put a lot of effort to decouple SQL from caches. I think we should develop some kind of "Queries" options on Ignite configuration. What do you think? On Wed, Apr 24, 2019 at 3:22 PM Павлухин Иван <[hidden email]> wrote: > Hi Saikat, > > I think that we should have a discussion and choose a place where a > "default query timeout" property will be configured. > > Generally, I think that a real (user) problem is possibility for > queries to execute indefinitely. And I have no doubts that we can > improve there. There could be several implementation strategies. One > is adding a property to CacheConfiguration. But it opens various > questions. E.g. how should it work if we execute SQL JOIN spanning > multiple tables (caches)? Also I am concerned about queries executed > not via cache.query() method. We have multiple alternative options, > e.g. thin clients (IgniteClient.query) or JDBC. I believe that > introducing a default timeout for all them is not a bad idea. > > What do you think? > > вт, 23 апр. 2019 г. в 03:01, Saikat Maitra <[hidden email]>: > > > > Hi Ivan, > > > > Thank you for your email. My understanding from the jira issue was it > will > > be cache level configuration for query default timeout. > > > > I need more info on the usage for this config property and if it is > shared > > in this jira issue I can make changes or if there is a separate jira > issue > > I can assign myself. > > > > > > Regards, > > Saikat > > > > On Mon, Apr 22, 2019 at 5:31 AM Павлухин Иван <[hidden email]> > wrote: > > > > > Hi Saikat, > > > > > > I see that a configuration property is added in PR but I do not see > > > how the property is used. Was it done intentionally? > > > > > > Also, we need to decide whether such timeout should be configured per > > > cache or for all caches in one place. For example, we have already > > > TransactionConfiguration#setDefaultTxTimeout which is a global one. > > > > > > Share you thoughts. > > > > > > вс, 21 апр. 2019 г. в 00:38, Saikat Maitra <[hidden email]>: > > > > > > > > Hi, > > > > > > > > I have raised a PR for the below issue. > > > > > > > > IGNITE-7285 Add default query timeout > > > > > > > > PR - https://github.com/apache/ignite/pull/6490 > > > > > > > > Please take a look and share feedback. > > > > > > > > Regards, > > > > Saikat > > > > > > > > > > > > -- > > > Best regards, > > > Ivan Pavlukhin > > > > > > > -- > Best regards, > Ivan Pavlukhin > -- Alexey Kuznetsov |
Hi Alexey, Ivan, Andrew
I think we can provide an option to configure defaultQueryOption at IgniteConfiguration and set the default value = 0 to imply if not set it will be execute indefinitely but then user can set this value based on the application preferences and use it in addition to SQL query timeout. I have updated the PR accordingly. Please review and share if any changes required. Regards, Saikat On Wed, Apr 24, 2019 at 4:33 AM Alexey Kuznetsov <[hidden email]> wrote: > Hi Saikat and Ivan, > > I think that properties that related to SQL should not be configured on > caches. > We already put a lot of effort to decouple SQL from caches. > > I think we should develop some kind of "Queries" options on Ignite > configuration. > > What do you think? > > > On Wed, Apr 24, 2019 at 3:22 PM Павлухин Иван <[hidden email]> wrote: > > > Hi Saikat, > > > > I think that we should have a discussion and choose a place where a > > "default query timeout" property will be configured. > > > > Generally, I think that a real (user) problem is possibility for > > queries to execute indefinitely. And I have no doubts that we can > > improve there. There could be several implementation strategies. One > > is adding a property to CacheConfiguration. But it opens various > > questions. E.g. how should it work if we execute SQL JOIN spanning > > multiple tables (caches)? Also I am concerned about queries executed > > not via cache.query() method. We have multiple alternative options, > > e.g. thin clients (IgniteClient.query) or JDBC. I believe that > > introducing a default timeout for all them is not a bad idea. > > > > What do you think? > > > > вт, 23 апр. 2019 г. в 03:01, Saikat Maitra <[hidden email]>: > > > > > > Hi Ivan, > > > > > > Thank you for your email. My understanding from the jira issue was it > > will > > > be cache level configuration for query default timeout. > > > > > > I need more info on the usage for this config property and if it is > > shared > > > in this jira issue I can make changes or if there is a separate jira > > issue > > > I can assign myself. > > > > > > > > > Regards, > > > Saikat > > > > > > On Mon, Apr 22, 2019 at 5:31 AM Павлухин Иван <[hidden email]> > > wrote: > > > > > > > Hi Saikat, > > > > > > > > I see that a configuration property is added in PR but I do not see > > > > how the property is used. Was it done intentionally? > > > > > > > > Also, we need to decide whether such timeout should be configured per > > > > cache or for all caches in one place. For example, we have already > > > > TransactionConfiguration#setDefaultTxTimeout which is a global one. > > > > > > > > Share you thoughts. > > > > > > > > вс, 21 апр. 2019 г. в 00:38, Saikat Maitra <[hidden email] > >: > > > > > > > > > > Hi, > > > > > > > > > > I have raised a PR for the below issue. > > > > > > > > > > IGNITE-7285 Add default query timeout > > > > > > > > > > PR - https://github.com/apache/ignite/pull/6490 > > > > > > > > > > Please take a look and share feedback. > > > > > > > > > > Regards, > > > > > Saikat > > > > > > > > > > > > > > > > -- > > > > Best regards, > > > > Ivan Pavlukhin > > > > > > > > > > > > -- > > Best regards, > > Ivan Pavlukhin > > > > > -- > Alexey Kuznetsov > |
Hi Saikat,
It a compatibility with previous versions the reason for an indefinite timeout by default? сб, 27 апр. 2019 г. в 16:58, Saikat Maitra <[hidden email]>: > > Hi Alexey, Ivan, Andrew > > I think we can provide an option to configure defaultQueryOption at > IgniteConfiguration and set the default value = 0 to imply if not set it > will be execute indefinitely but then user can set this value based on the > application preferences and use it in addition to SQL query timeout. > > I have updated the PR accordingly. > > Please review and share if any changes required. > > Regards, > Saikat > > On Wed, Apr 24, 2019 at 4:33 AM Alexey Kuznetsov <[hidden email]> > wrote: > > > Hi Saikat and Ivan, > > > > I think that properties that related to SQL should not be configured on > > caches. > > We already put a lot of effort to decouple SQL from caches. > > > > I think we should develop some kind of "Queries" options on Ignite > > configuration. > > > > What do you think? > > > > > > On Wed, Apr 24, 2019 at 3:22 PM Павлухин Иван <[hidden email]> wrote: > > > > > Hi Saikat, > > > > > > I think that we should have a discussion and choose a place where a > > > "default query timeout" property will be configured. > > > > > > Generally, I think that a real (user) problem is possibility for > > > queries to execute indefinitely. And I have no doubts that we can > > > improve there. There could be several implementation strategies. One > > > is adding a property to CacheConfiguration. But it opens various > > > questions. E.g. how should it work if we execute SQL JOIN spanning > > > multiple tables (caches)? Also I am concerned about queries executed > > > not via cache.query() method. We have multiple alternative options, > > > e.g. thin clients (IgniteClient.query) or JDBC. I believe that > > > introducing a default timeout for all them is not a bad idea. > > > > > > What do you think? > > > > > > вт, 23 апр. 2019 г. в 03:01, Saikat Maitra <[hidden email]>: > > > > > > > > Hi Ivan, > > > > > > > > Thank you for your email. My understanding from the jira issue was it > > > will > > > > be cache level configuration for query default timeout. > > > > > > > > I need more info on the usage for this config property and if it is > > > shared > > > > in this jira issue I can make changes or if there is a separate jira > > > issue > > > > I can assign myself. > > > > > > > > > > > > Regards, > > > > Saikat > > > > > > > > On Mon, Apr 22, 2019 at 5:31 AM Павлухин Иван <[hidden email]> > > > wrote: > > > > > > > > > Hi Saikat, > > > > > > > > > > I see that a configuration property is added in PR but I do not see > > > > > how the property is used. Was it done intentionally? > > > > > > > > > > Also, we need to decide whether such timeout should be configured per > > > > > cache or for all caches in one place. For example, we have already > > > > > TransactionConfiguration#setDefaultTxTimeout which is a global one. > > > > > > > > > > Share you thoughts. > > > > > > > > > > вс, 21 апр. 2019 г. в 00:38, Saikat Maitra <[hidden email] > > >: > > > > > > > > > > > > Hi, > > > > > > > > > > > > I have raised a PR for the below issue. > > > > > > > > > > > > IGNITE-7285 Add default query timeout > > > > > > > > > > > > PR - https://github.com/apache/ignite/pull/6490 > > > > > > > > > > > > Please take a look and share feedback. > > > > > > > > > > > > Regards, > > > > > > Saikat > > > > > > > > > > > > > > > > > > > > -- > > > > > Best regards, > > > > > Ivan Pavlukhin > > > > > > > > > > > > > > > > > -- > > > Best regards, > > > Ivan Pavlukhin > > > > > > > > > -- > > Alexey Kuznetsov > > -- Best regards, Ivan Pavlukhin |
Hi Ivan,
Yes, I checked this CacheQuery default value https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/query/CacheQuery.java#L200 Also, Andrew recommended the same in review feedback. https://github.com/apache/ignite/pull/6490#discussion_r277730394 Regards, Saikat On Mon, Apr 29, 2019 at 3:18 AM Павлухин Иван <[hidden email]> wrote: > Hi Saikat, > > It a compatibility with previous versions the reason for an indefinite > timeout by default? > > сб, 27 апр. 2019 г. в 16:58, Saikat Maitra <[hidden email]>: > > > > Hi Alexey, Ivan, Andrew > > > > I think we can provide an option to configure defaultQueryOption at > > IgniteConfiguration and set the default value = 0 to imply if not set it > > will be execute indefinitely but then user can set this value based on > the > > application preferences and use it in addition to SQL query timeout. > > > > I have updated the PR accordingly. > > > > Please review and share if any changes required. > > > > Regards, > > Saikat > > > > On Wed, Apr 24, 2019 at 4:33 AM Alexey Kuznetsov <[hidden email]> > > wrote: > > > > > Hi Saikat and Ivan, > > > > > > I think that properties that related to SQL should not be configured on > > > caches. > > > We already put a lot of effort to decouple SQL from caches. > > > > > > I think we should develop some kind of "Queries" options on Ignite > > > configuration. > > > > > > What do you think? > > > > > > > > > On Wed, Apr 24, 2019 at 3:22 PM Павлухин Иван <[hidden email]> > wrote: > > > > > > > Hi Saikat, > > > > > > > > I think that we should have a discussion and choose a place where a > > > > "default query timeout" property will be configured. > > > > > > > > Generally, I think that a real (user) problem is possibility for > > > > queries to execute indefinitely. And I have no doubts that we can > > > > improve there. There could be several implementation strategies. One > > > > is adding a property to CacheConfiguration. But it opens various > > > > questions. E.g. how should it work if we execute SQL JOIN spanning > > > > multiple tables (caches)? Also I am concerned about queries executed > > > > not via cache.query() method. We have multiple alternative options, > > > > e.g. thin clients (IgniteClient.query) or JDBC. I believe that > > > > introducing a default timeout for all them is not a bad idea. > > > > > > > > What do you think? > > > > > > > > вт, 23 апр. 2019 г. в 03:01, Saikat Maitra <[hidden email] > >: > > > > > > > > > > Hi Ivan, > > > > > > > > > > Thank you for your email. My understanding from the jira issue was > it > > > > will > > > > > be cache level configuration for query default timeout. > > > > > > > > > > I need more info on the usage for this config property and if it is > > > > shared > > > > > in this jira issue I can make changes or if there is a separate > jira > > > > issue > > > > > I can assign myself. > > > > > > > > > > > > > > > Regards, > > > > > Saikat > > > > > > > > > > On Mon, Apr 22, 2019 at 5:31 AM Павлухин Иван <[hidden email] > > > > > > wrote: > > > > > > > > > > > Hi Saikat, > > > > > > > > > > > > I see that a configuration property is added in PR but I do not > see > > > > > > how the property is used. Was it done intentionally? > > > > > > > > > > > > Also, we need to decide whether such timeout should be > configured per > > > > > > cache or for all caches in one place. For example, we have > already > > > > > > TransactionConfiguration#setDefaultTxTimeout which is a global > one. > > > > > > > > > > > > Share you thoughts. > > > > > > > > > > > > вс, 21 апр. 2019 г. в 00:38, Saikat Maitra < > [hidden email] > > > >: > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > I have raised a PR for the below issue. > > > > > > > > > > > > > > IGNITE-7285 Add default query timeout > > > > > > > > > > > > > > PR - https://github.com/apache/ignite/pull/6490 > > > > > > > > > > > > > > Please take a look and share feedback. > > > > > > > > > > > > > > Regards, > > > > > > > Saikat > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > Best regards, > > > > > > Ivan Pavlukhin > > > > > > > > > > > > > > > > > > > > > > -- > > > > Best regards, > > > > Ivan Pavlukhin > > > > > > > > > > > > > -- > > > Alexey Kuznetsov > > > > > > > -- > Best regards, > Ivan Pavlukhin > |
Hi Saikat,
I left a couple of comment on GitHub [1]. Perhaps I am missing it but what are the plans for making a default query timeout workable by using an introduced property in query execution flow? [1] https://github.com/apache/ignite/pull/6490 вт, 30 апр. 2019 г. в 02:50, Saikat Maitra <[hidden email]>: > > Hi Ivan, > > Yes, I checked this CacheQuery default value > https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/query/CacheQuery.java#L200 > > Also, Andrew recommended the same in review feedback. > > https://github.com/apache/ignite/pull/6490#discussion_r277730394 > > Regards, > Saikat > > > On Mon, Apr 29, 2019 at 3:18 AM Павлухин Иван <[hidden email]> wrote: > > > Hi Saikat, > > > > It a compatibility with previous versions the reason for an indefinite > > timeout by default? > > > > сб, 27 апр. 2019 г. в 16:58, Saikat Maitra <[hidden email]>: > > > > > > Hi Alexey, Ivan, Andrew > > > > > > I think we can provide an option to configure defaultQueryOption at > > > IgniteConfiguration and set the default value = 0 to imply if not set it > > > will be execute indefinitely but then user can set this value based on > > the > > > application preferences and use it in addition to SQL query timeout. > > > > > > I have updated the PR accordingly. > > > > > > Please review and share if any changes required. > > > > > > Regards, > > > Saikat > > > > > > On Wed, Apr 24, 2019 at 4:33 AM Alexey Kuznetsov <[hidden email]> > > > wrote: > > > > > > > Hi Saikat and Ivan, > > > > > > > > I think that properties that related to SQL should not be configured on > > > > caches. > > > > We already put a lot of effort to decouple SQL from caches. > > > > > > > > I think we should develop some kind of "Queries" options on Ignite > > > > configuration. > > > > > > > > What do you think? > > > > > > > > > > > > On Wed, Apr 24, 2019 at 3:22 PM Павлухин Иван <[hidden email]> > > wrote: > > > > > > > > > Hi Saikat, > > > > > > > > > > I think that we should have a discussion and choose a place where a > > > > > "default query timeout" property will be configured. > > > > > > > > > > Generally, I think that a real (user) problem is possibility for > > > > > queries to execute indefinitely. And I have no doubts that we can > > > > > improve there. There could be several implementation strategies. One > > > > > is adding a property to CacheConfiguration. But it opens various > > > > > questions. E.g. how should it work if we execute SQL JOIN spanning > > > > > multiple tables (caches)? Also I am concerned about queries executed > > > > > not via cache.query() method. We have multiple alternative options, > > > > > e.g. thin clients (IgniteClient.query) or JDBC. I believe that > > > > > introducing a default timeout for all them is not a bad idea. > > > > > > > > > > What do you think? > > > > > > > > > > вт, 23 апр. 2019 г. в 03:01, Saikat Maitra <[hidden email] > > >: > > > > > > > > > > > > Hi Ivan, > > > > > > > > > > > > Thank you for your email. My understanding from the jira issue was > > it > > > > > will > > > > > > be cache level configuration for query default timeout. > > > > > > > > > > > > I need more info on the usage for this config property and if it is > > > > > shared > > > > > > in this jira issue I can make changes or if there is a separate > > jira > > > > > issue > > > > > > I can assign myself. > > > > > > > > > > > > > > > > > > Regards, > > > > > > Saikat > > > > > > > > > > > > On Mon, Apr 22, 2019 at 5:31 AM Павлухин Иван <[hidden email] > > > > > > > > wrote: > > > > > > > > > > > > > Hi Saikat, > > > > > > > > > > > > > > I see that a configuration property is added in PR but I do not > > see > > > > > > > how the property is used. Was it done intentionally? > > > > > > > > > > > > > > Also, we need to decide whether such timeout should be > > configured per > > > > > > > cache or for all caches in one place. For example, we have > > already > > > > > > > TransactionConfiguration#setDefaultTxTimeout which is a global > > one. > > > > > > > > > > > > > > Share you thoughts. > > > > > > > > > > > > > > вс, 21 апр. 2019 г. в 00:38, Saikat Maitra < > > [hidden email] > > > > >: > > > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > I have raised a PR for the below issue. > > > > > > > > > > > > > > > > IGNITE-7285 Add default query timeout > > > > > > > > > > > > > > > > PR - https://github.com/apache/ignite/pull/6490 > > > > > > > > > > > > > > > > Please take a look and share feedback. > > > > > > > > > > > > > > > > Regards, > > > > > > > > Saikat > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > Best regards, > > > > > > > Ivan Pavlukhin > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > Best regards, > > > > > Ivan Pavlukhin > > > > > > > > > > > > > > > > > -- > > > > Alexey Kuznetsov > > > > > > > > > > > > -- > > Best regards, > > Ivan Pavlukhin > > -- Best regards, Ivan Pavlukhin |
Andrey K.,
> I think we should develop some kind of "Queries" options on Ignite > configuration. Quite a reasonable idea. We already have couple of query-related properties in IgniteConfiguration and we can move them (in a compatible way) to a query properties sub-aggregate. I think it is better to raise a separate topic for that. ср, 1 мая 2019 г. в 09:00, Павлухин Иван <[hidden email]>: > > Hi Saikat, > > I left a couple of comment on GitHub [1]. > > Perhaps I am missing it but what are the plans for making a default > query timeout workable by using an introduced property in query > execution flow? > > [1] https://github.com/apache/ignite/pull/6490 > > вт, 30 апр. 2019 г. в 02:50, Saikat Maitra <[hidden email]>: > > > > Hi Ivan, > > > > Yes, I checked this CacheQuery default value > > https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/query/CacheQuery.java#L200 > > > > Also, Andrew recommended the same in review feedback. > > > > https://github.com/apache/ignite/pull/6490#discussion_r277730394 > > > > Regards, > > Saikat > > > > > > On Mon, Apr 29, 2019 at 3:18 AM Павлухин Иван <[hidden email]> wrote: > > > > > Hi Saikat, > > > > > > It a compatibility with previous versions the reason for an indefinite > > > timeout by default? > > > > > > сб, 27 апр. 2019 г. в 16:58, Saikat Maitra <[hidden email]>: > > > > > > > > Hi Alexey, Ivan, Andrew > > > > > > > > I think we can provide an option to configure defaultQueryOption at > > > > IgniteConfiguration and set the default value = 0 to imply if not set it > > > > will be execute indefinitely but then user can set this value based on > > > the > > > > application preferences and use it in addition to SQL query timeout. > > > > > > > > I have updated the PR accordingly. > > > > > > > > Please review and share if any changes required. > > > > > > > > Regards, > > > > Saikat > > > > > > > > On Wed, Apr 24, 2019 at 4:33 AM Alexey Kuznetsov <[hidden email]> > > > > wrote: > > > > > > > > > Hi Saikat and Ivan, > > > > > > > > > > I think that properties that related to SQL should not be configured on > > > > > caches. > > > > > We already put a lot of effort to decouple SQL from caches. > > > > > > > > > > I think we should develop some kind of "Queries" options on Ignite > > > > > configuration. > > > > > > > > > > What do you think? > > > > > > > > > > > > > > > On Wed, Apr 24, 2019 at 3:22 PM Павлухин Иван <[hidden email]> > > > wrote: > > > > > > > > > > > Hi Saikat, > > > > > > > > > > > > I think that we should have a discussion and choose a place where a > > > > > > "default query timeout" property will be configured. > > > > > > > > > > > > Generally, I think that a real (user) problem is possibility for > > > > > > queries to execute indefinitely. And I have no doubts that we can > > > > > > improve there. There could be several implementation strategies. One > > > > > > is adding a property to CacheConfiguration. But it opens various > > > > > > questions. E.g. how should it work if we execute SQL JOIN spanning > > > > > > multiple tables (caches)? Also I am concerned about queries executed > > > > > > not via cache.query() method. We have multiple alternative options, > > > > > > e.g. thin clients (IgniteClient.query) or JDBC. I believe that > > > > > > introducing a default timeout for all them is not a bad idea. > > > > > > > > > > > > What do you think? > > > > > > > > > > > > вт, 23 апр. 2019 г. в 03:01, Saikat Maitra <[hidden email] > > > >: > > > > > > > > > > > > > > Hi Ivan, > > > > > > > > > > > > > > Thank you for your email. My understanding from the jira issue was > > > it > > > > > > will > > > > > > > be cache level configuration for query default timeout. > > > > > > > > > > > > > > I need more info on the usage for this config property and if it is > > > > > > shared > > > > > > > in this jira issue I can make changes or if there is a separate > > > jira > > > > > > issue > > > > > > > I can assign myself. > > > > > > > > > > > > > > > > > > > > > Regards, > > > > > > > Saikat > > > > > > > > > > > > > > On Mon, Apr 22, 2019 at 5:31 AM Павлухин Иван <[hidden email] > > > > > > > > > > wrote: > > > > > > > > > > > > > > > Hi Saikat, > > > > > > > > > > > > > > > > I see that a configuration property is added in PR but I do not > > > see > > > > > > > > how the property is used. Was it done intentionally? > > > > > > > > > > > > > > > > Also, we need to decide whether such timeout should be > > > configured per > > > > > > > > cache or for all caches in one place. For example, we have > > > already > > > > > > > > TransactionConfiguration#setDefaultTxTimeout which is a global > > > one. > > > > > > > > > > > > > > > > Share you thoughts. > > > > > > > > > > > > > > > > вс, 21 апр. 2019 г. в 00:38, Saikat Maitra < > > > [hidden email] > > > > > >: > > > > > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > I have raised a PR for the below issue. > > > > > > > > > > > > > > > > > > IGNITE-7285 Add default query timeout > > > > > > > > > > > > > > > > > > PR - https://github.com/apache/ignite/pull/6490 > > > > > > > > > > > > > > > > > > Please take a look and share feedback. > > > > > > > > > > > > > > > > > > Regards, > > > > > > > > > Saikat > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > Best regards, > > > > > > > > Ivan Pavlukhin > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > Best regards, > > > > > > Ivan Pavlukhin > > > > > > > > > > > > > > > > > > > > > -- > > > > > Alexey Kuznetsov > > > > > > > > > > > > > > > > > -- > > > Best regards, > > > Ivan Pavlukhin > > > > > > > -- > Best regards, > Ivan Pavlukhin -- Best regards, Ivan Pavlukhin |
Hi Ivan,
Thank you for reviewing the PR. I have updated the PR. Please review and share your feedback. I was thinking of making a separate PR for using the defaultQueryTimeout property in query execution flow. Regards, Saikat On Wed, May 1, 2019 at 1:04 AM Павлухин Иван <[hidden email]> wrote: > Andrey K., > > > I think we should develop some kind of "Queries" options on Ignite > > configuration. > > Quite a reasonable idea. We already have couple of query-related > properties in IgniteConfiguration and we can move them (in a > compatible way) to a query properties sub-aggregate. I think it is > better to raise a separate topic for that. > > ср, 1 мая 2019 г. в 09:00, Павлухин Иван <[hidden email]>: > > > > Hi Saikat, > > > > I left a couple of comment on GitHub [1]. > > > > Perhaps I am missing it but what are the plans for making a default > > query timeout workable by using an introduced property in query > > execution flow? > > > > [1] https://github.com/apache/ignite/pull/6490 > > > > вт, 30 апр. 2019 г. в 02:50, Saikat Maitra <[hidden email]>: > > > > > > Hi Ivan, > > > > > > Yes, I checked this CacheQuery default value > > > > https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/query/CacheQuery.java#L200 > > > > > > Also, Andrew recommended the same in review feedback. > > > > > > https://github.com/apache/ignite/pull/6490#discussion_r277730394 > > > > > > Regards, > > > Saikat > > > > > > > > > On Mon, Apr 29, 2019 at 3:18 AM Павлухин Иван <[hidden email]> > wrote: > > > > > > > Hi Saikat, > > > > > > > > It a compatibility with previous versions the reason for an > indefinite > > > > timeout by default? > > > > > > > > сб, 27 апр. 2019 г. в 16:58, Saikat Maitra <[hidden email] > >: > > > > > > > > > > Hi Alexey, Ivan, Andrew > > > > > > > > > > I think we can provide an option to configure defaultQueryOption at > > > > > IgniteConfiguration and set the default value = 0 to imply if not > set it > > > > > will be execute indefinitely but then user can set this value > based on > > > > the > > > > > application preferences and use it in addition to SQL query > timeout. > > > > > > > > > > I have updated the PR accordingly. > > > > > > > > > > Please review and share if any changes required. > > > > > > > > > > Regards, > > > > > Saikat > > > > > > > > > > On Wed, Apr 24, 2019 at 4:33 AM Alexey Kuznetsov < > [hidden email]> > > > > > wrote: > > > > > > > > > > > Hi Saikat and Ivan, > > > > > > > > > > > > I think that properties that related to SQL should not be > configured on > > > > > > caches. > > > > > > We already put a lot of effort to decouple SQL from caches. > > > > > > > > > > > > I think we should develop some kind of "Queries" options on > Ignite > > > > > > configuration. > > > > > > > > > > > > What do you think? > > > > > > > > > > > > > > > > > > On Wed, Apr 24, 2019 at 3:22 PM Павлухин Иван < > [hidden email]> > > > > wrote: > > > > > > > > > > > > > Hi Saikat, > > > > > > > > > > > > > > I think that we should have a discussion and choose a place > where a > > > > > > > "default query timeout" property will be configured. > > > > > > > > > > > > > > Generally, I think that a real (user) problem is possibility > for > > > > > > > queries to execute indefinitely. And I have no doubts that we > can > > > > > > > improve there. There could be several implementation > strategies. One > > > > > > > is adding a property to CacheConfiguration. But it opens > various > > > > > > > questions. E.g. how should it work if we execute SQL JOIN > spanning > > > > > > > multiple tables (caches)? Also I am concerned about queries > executed > > > > > > > not via cache.query() method. We have multiple alternative > options, > > > > > > > e.g. thin clients (IgniteClient.query) or JDBC. I believe that > > > > > > > introducing a default timeout for all them is not a bad idea. > > > > > > > > > > > > > > What do you think? > > > > > > > > > > > > > > вт, 23 апр. 2019 г. в 03:01, Saikat Maitra < > [hidden email] > > > > >: > > > > > > > > > > > > > > > > Hi Ivan, > > > > > > > > > > > > > > > > Thank you for your email. My understanding from the jira > issue was > > > > it > > > > > > > will > > > > > > > > be cache level configuration for query default timeout. > > > > > > > > > > > > > > > > I need more info on the usage for this config property and > if it is > > > > > > > shared > > > > > > > > in this jira issue I can make changes or if there is a > separate > > > > jira > > > > > > > issue > > > > > > > > I can assign myself. > > > > > > > > > > > > > > > > > > > > > > > > Regards, > > > > > > > > Saikat > > > > > > > > > > > > > > > > On Mon, Apr 22, 2019 at 5:31 AM Павлухин Иван < > [hidden email] > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > Hi Saikat, > > > > > > > > > > > > > > > > > > I see that a configuration property is added in PR but I > do not > > > > see > > > > > > > > > how the property is used. Was it done intentionally? > > > > > > > > > > > > > > > > > > Also, we need to decide whether such timeout should be > > > > configured per > > > > > > > > > cache or for all caches in one place. For example, we have > > > > already > > > > > > > > > TransactionConfiguration#setDefaultTxTimeout which is a > global > > > > one. > > > > > > > > > > > > > > > > > > Share you thoughts. > > > > > > > > > > > > > > > > > > вс, 21 апр. 2019 г. в 00:38, Saikat Maitra < > > > > [hidden email] > > > > > > >: > > > > > > > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > > > I have raised a PR for the below issue. > > > > > > > > > > > > > > > > > > > > IGNITE-7285 Add default query timeout > > > > > > > > > > > > > > > > > > > > PR - https://github.com/apache/ignite/pull/6490 > > > > > > > > > > > > > > > > > > > > Please take a look and share feedback. > > > > > > > > > > > > > > > > > > > > Regards, > > > > > > > > > > Saikat > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > Best regards, > > > > > > > > > Ivan Pavlukhin > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > Best regards, > > > > > > > Ivan Pavlukhin > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > Alexey Kuznetsov > > > > > > > > > > > > > > > > > > > > > > -- > > > > Best regards, > > > > Ivan Pavlukhin > > > > > > > > > > > > -- > > Best regards, > > Ivan Pavlukhin > > > > -- > Best regards, > Ivan Pavlukhin > |
Hi Saikat,
It is good that we agreed how property in question should be configured. But I worry about the following. If the PR merged it will not contain a user value yet because an introduced property is not used. Consequently we must start using that property before a next AI release. Just one thing to keep in mind. Sent from my iPhone > On 4 May 2019, at 05:56, Saikat Maitra <[hidden email]> wrote: > > Hi Ivan, > > Thank you for reviewing the PR. I have updated the PR. Please review and > share your feedback. > > I was thinking of making a separate PR for using the defaultQueryTimeout > property in query execution flow. > > Regards, > Saikat > >> On Wed, May 1, 2019 at 1:04 AM Павлухин Иван <[hidden email]> wrote: >> >> Andrey K., >> >>> I think we should develop some kind of "Queries" options on Ignite >>> configuration. >> >> Quite a reasonable idea. We already have couple of query-related >> properties in IgniteConfiguration and we can move them (in a >> compatible way) to a query properties sub-aggregate. I think it is >> better to raise a separate topic for that. >> >> ср, 1 мая 2019 г. в 09:00, Павлухин Иван <[hidden email]>: >>> >>> Hi Saikat, >>> >>> I left a couple of comment on GitHub [1]. >>> >>> Perhaps I am missing it but what are the plans for making a default >>> query timeout workable by using an introduced property in query >>> execution flow? >>> >>> [1] https://github.com/apache/ignite/pull/6490 >>> >>> вт, 30 апр. 2019 г. в 02:50, Saikat Maitra <[hidden email]>: >>>> >>>> Hi Ivan, >>>> >>>> Yes, I checked this CacheQuery default value >>>> >> https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/query/CacheQuery.java#L200 >>>> >>>> Also, Andrew recommended the same in review feedback. >>>> >>>> https://github.com/apache/ignite/pull/6490#discussion_r277730394 >>>> >>>> Regards, >>>> Saikat >>>> >>>> >>>> On Mon, Apr 29, 2019 at 3:18 AM Павлухин Иван <[hidden email]> >> wrote: >>>> >>>>> Hi Saikat, >>>>> >>>>> It a compatibility with previous versions the reason for an >> indefinite >>>>> timeout by default? >>>>> >>>>> сб, 27 апр. 2019 г. в 16:58, Saikat Maitra <[hidden email] >>> : >>>>>> >>>>>> Hi Alexey, Ivan, Andrew >>>>>> >>>>>> I think we can provide an option to configure defaultQueryOption at >>>>>> IgniteConfiguration and set the default value = 0 to imply if not >> set it >>>>>> will be execute indefinitely but then user can set this value >> based on >>>>> the >>>>>> application preferences and use it in addition to SQL query >> timeout. >>>>>> >>>>>> I have updated the PR accordingly. >>>>>> >>>>>> Please review and share if any changes required. >>>>>> >>>>>> Regards, >>>>>> Saikat >>>>>> >>>>>> On Wed, Apr 24, 2019 at 4:33 AM Alexey Kuznetsov < >> [hidden email]> >>>>>> wrote: >>>>>> >>>>>>> Hi Saikat and Ivan, >>>>>>> >>>>>>> I think that properties that related to SQL should not be >> configured on >>>>>>> caches. >>>>>>> We already put a lot of effort to decouple SQL from caches. >>>>>>> >>>>>>> I think we should develop some kind of "Queries" options on >> Ignite >>>>>>> configuration. >>>>>>> >>>>>>> What do you think? >>>>>>> >>>>>>> >>>>>>> On Wed, Apr 24, 2019 at 3:22 PM Павлухин Иван < >> [hidden email]> >>>>> wrote: >>>>>>> >>>>>>>> Hi Saikat, >>>>>>>> >>>>>>>> I think that we should have a discussion and choose a place >> where a >>>>>>>> "default query timeout" property will be configured. >>>>>>>> >>>>>>>> Generally, I think that a real (user) problem is possibility >> for >>>>>>>> queries to execute indefinitely. And I have no doubts that we >> can >>>>>>>> improve there. There could be several implementation >> strategies. One >>>>>>>> is adding a property to CacheConfiguration. But it opens >> various >>>>>>>> questions. E.g. how should it work if we execute SQL JOIN >> spanning >>>>>>>> multiple tables (caches)? Also I am concerned about queries >> executed >>>>>>>> not via cache.query() method. We have multiple alternative >> options, >>>>>>>> e.g. thin clients (IgniteClient.query) or JDBC. I believe that >>>>>>>> introducing a default timeout for all them is not a bad idea. >>>>>>>> >>>>>>>> What do you think? >>>>>>>> >>>>>>>> вт, 23 апр. 2019 г. в 03:01, Saikat Maitra < >> [hidden email] >>>>>> : >>>>>>>>> >>>>>>>>> Hi Ivan, >>>>>>>>> >>>>>>>>> Thank you for your email. My understanding from the jira >> issue was >>>>> it >>>>>>>> will >>>>>>>>> be cache level configuration for query default timeout. >>>>>>>>> >>>>>>>>> I need more info on the usage for this config property and >> if it is >>>>>>>> shared >>>>>>>>> in this jira issue I can make changes or if there is a >> separate >>>>> jira >>>>>>>> issue >>>>>>>>> I can assign myself. >>>>>>>>> >>>>>>>>> >>>>>>>>> Regards, >>>>>>>>> Saikat >>>>>>>>> >>>>>>>>> On Mon, Apr 22, 2019 at 5:31 AM Павлухин Иван < >> [hidden email] >>>>>> >>>>>>>> wrote: >>>>>>>>> >>>>>>>>>> Hi Saikat, >>>>>>>>>> >>>>>>>>>> I see that a configuration property is added in PR but I >> do not >>>>> see >>>>>>>>>> how the property is used. Was it done intentionally? >>>>>>>>>> >>>>>>>>>> Also, we need to decide whether such timeout should be >>>>> configured per >>>>>>>>>> cache or for all caches in one place. For example, we have >>>>> already >>>>>>>>>> TransactionConfiguration#setDefaultTxTimeout which is a >> global >>>>> one. >>>>>>>>>> >>>>>>>>>> Share you thoughts. >>>>>>>>>> >>>>>>>>>> вс, 21 апр. 2019 г. в 00:38, Saikat Maitra < >>>>> [hidden email] >>>>>>>> : >>>>>>>>>>> >>>>>>>>>>> Hi, >>>>>>>>>>> >>>>>>>>>>> I have raised a PR for the below issue. >>>>>>>>>>> >>>>>>>>>>> IGNITE-7285 Add default query timeout >>>>>>>>>>> >>>>>>>>>>> PR - https://github.com/apache/ignite/pull/6490 >>>>>>>>>>> >>>>>>>>>>> Please take a look and share feedback. >>>>>>>>>>> >>>>>>>>>>> Regards, >>>>>>>>>>> Saikat >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> Best regards, >>>>>>>>>> Ivan Pavlukhin >>>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> Best regards, >>>>>>>> Ivan Pavlukhin >>>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> Alexey Kuznetsov >>>>>>> >>>>> >>>>> >>>>> >>>>> -- >>>>> Best regards, >>>>> Ivan Pavlukhin >>>>> >>> >>> >>> >>> -- >>> Best regards, >>> Ivan Pavlukhin >> >> >> >> -- >> Best regards, >> Ivan Pavlukhin >> |
Hi Ivan,
Thank you for your email. I have updated the PR to use default query timeout. Please take a look. https://github.com/apache/ignite/pull/6490/files Regards Saikat On Tue, May 7, 2019 at 12:30 AM Ivan Pavlukhina <[hidden email]> wrote: > Hi Saikat, > > It is good that we agreed how property in question should be configured. > But I worry about the following. If the PR merged it will not contain a > user value yet because an introduced property is not used. Consequently we > must start using that property before a next AI release. Just one thing to > keep in mind. > > Sent from my iPhone > > > On 4 May 2019, at 05:56, Saikat Maitra <[hidden email]> wrote: > > > > Hi Ivan, > > > > Thank you for reviewing the PR. I have updated the PR. Please review and > > share your feedback. > > > > I was thinking of making a separate PR for using the defaultQueryTimeout > > property in query execution flow. > > > > Regards, > > Saikat > > > >> On Wed, May 1, 2019 at 1:04 AM Павлухин Иван <[hidden email]> > wrote: > >> > >> Andrey K., > >> > >>> I think we should develop some kind of "Queries" options on Ignite > >>> configuration. > >> > >> Quite a reasonable idea. We already have couple of query-related > >> properties in IgniteConfiguration and we can move them (in a > >> compatible way) to a query properties sub-aggregate. I think it is > >> better to raise a separate topic for that. > >> > >> ср, 1 мая 2019 г. в 09:00, Павлухин Иван <[hidden email]>: > >>> > >>> Hi Saikat, > >>> > >>> I left a couple of comment on GitHub [1]. > >>> > >>> Perhaps I am missing it but what are the plans for making a default > >>> query timeout workable by using an introduced property in query > >>> execution flow? > >>> > >>> [1] https://github.com/apache/ignite/pull/6490 > >>> > >>> вт, 30 апр. 2019 г. в 02:50, Saikat Maitra <[hidden email]>: > >>>> > >>>> Hi Ivan, > >>>> > >>>> Yes, I checked this CacheQuery default value > >>>> > >> > https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/query/CacheQuery.java#L200 > >>>> > >>>> Also, Andrew recommended the same in review feedback. > >>>> > >>>> https://github.com/apache/ignite/pull/6490#discussion_r277730394 > >>>> > >>>> Regards, > >>>> Saikat > >>>> > >>>> > >>>> On Mon, Apr 29, 2019 at 3:18 AM Павлухин Иван <[hidden email]> > >> wrote: > >>>> > >>>>> Hi Saikat, > >>>>> > >>>>> It a compatibility with previous versions the reason for an > >> indefinite > >>>>> timeout by default? > >>>>> > >>>>> сб, 27 апр. 2019 г. в 16:58, Saikat Maitra <[hidden email] > >>> : > >>>>>> > >>>>>> Hi Alexey, Ivan, Andrew > >>>>>> > >>>>>> I think we can provide an option to configure defaultQueryOption at > >>>>>> IgniteConfiguration and set the default value = 0 to imply if not > >> set it > >>>>>> will be execute indefinitely but then user can set this value > >> based on > >>>>> the > >>>>>> application preferences and use it in addition to SQL query > >> timeout. > >>>>>> > >>>>>> I have updated the PR accordingly. > >>>>>> > >>>>>> Please review and share if any changes required. > >>>>>> > >>>>>> Regards, > >>>>>> Saikat > >>>>>> > >>>>>> On Wed, Apr 24, 2019 at 4:33 AM Alexey Kuznetsov < > >> [hidden email]> > >>>>>> wrote: > >>>>>> > >>>>>>> Hi Saikat and Ivan, > >>>>>>> > >>>>>>> I think that properties that related to SQL should not be > >> configured on > >>>>>>> caches. > >>>>>>> We already put a lot of effort to decouple SQL from caches. > >>>>>>> > >>>>>>> I think we should develop some kind of "Queries" options on > >> Ignite > >>>>>>> configuration. > >>>>>>> > >>>>>>> What do you think? > >>>>>>> > >>>>>>> > >>>>>>> On Wed, Apr 24, 2019 at 3:22 PM Павлухин Иван < > >> [hidden email]> > >>>>> wrote: > >>>>>>> > >>>>>>>> Hi Saikat, > >>>>>>>> > >>>>>>>> I think that we should have a discussion and choose a place > >> where a > >>>>>>>> "default query timeout" property will be configured. > >>>>>>>> > >>>>>>>> Generally, I think that a real (user) problem is possibility > >> for > >>>>>>>> queries to execute indefinitely. And I have no doubts that we > >> can > >>>>>>>> improve there. There could be several implementation > >> strategies. One > >>>>>>>> is adding a property to CacheConfiguration. But it opens > >> various > >>>>>>>> questions. E.g. how should it work if we execute SQL JOIN > >> spanning > >>>>>>>> multiple tables (caches)? Also I am concerned about queries > >> executed > >>>>>>>> not via cache.query() method. We have multiple alternative > >> options, > >>>>>>>> e.g. thin clients (IgniteClient.query) or JDBC. I believe that > >>>>>>>> introducing a default timeout for all them is not a bad idea. > >>>>>>>> > >>>>>>>> What do you think? > >>>>>>>> > >>>>>>>> вт, 23 апр. 2019 г. в 03:01, Saikat Maitra < > >> [hidden email] > >>>>>> : > >>>>>>>>> > >>>>>>>>> Hi Ivan, > >>>>>>>>> > >>>>>>>>> Thank you for your email. My understanding from the jira > >> issue was > >>>>> it > >>>>>>>> will > >>>>>>>>> be cache level configuration for query default timeout. > >>>>>>>>> > >>>>>>>>> I need more info on the usage for this config property and > >> if it is > >>>>>>>> shared > >>>>>>>>> in this jira issue I can make changes or if there is a > >> separate > >>>>> jira > >>>>>>>> issue > >>>>>>>>> I can assign myself. > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> Regards, > >>>>>>>>> Saikat > >>>>>>>>> > >>>>>>>>> On Mon, Apr 22, 2019 at 5:31 AM Павлухин Иван < > >> [hidden email] > >>>>>> > >>>>>>>> wrote: > >>>>>>>>> > >>>>>>>>>> Hi Saikat, > >>>>>>>>>> > >>>>>>>>>> I see that a configuration property is added in PR but I > >> do not > >>>>> see > >>>>>>>>>> how the property is used. Was it done intentionally? > >>>>>>>>>> > >>>>>>>>>> Also, we need to decide whether such timeout should be > >>>>> configured per > >>>>>>>>>> cache or for all caches in one place. For example, we have > >>>>> already > >>>>>>>>>> TransactionConfiguration#setDefaultTxTimeout which is a > >> global > >>>>> one. > >>>>>>>>>> > >>>>>>>>>> Share you thoughts. > >>>>>>>>>> > >>>>>>>>>> вс, 21 апр. 2019 г. в 00:38, Saikat Maitra < > >>>>> [hidden email] > >>>>>>>> : > >>>>>>>>>>> > >>>>>>>>>>> Hi, > >>>>>>>>>>> > >>>>>>>>>>> I have raised a PR for the below issue. > >>>>>>>>>>> > >>>>>>>>>>> IGNITE-7285 Add default query timeout > >>>>>>>>>>> > >>>>>>>>>>> PR - https://github.com/apache/ignite/pull/6490 > >>>>>>>>>>> > >>>>>>>>>>> Please take a look and share feedback. > >>>>>>>>>>> > >>>>>>>>>>> Regards, > >>>>>>>>>>> Saikat > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> -- > >>>>>>>>>> Best regards, > >>>>>>>>>> Ivan Pavlukhin > >>>>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> -- > >>>>>>>> Best regards, > >>>>>>>> Ivan Pavlukhin > >>>>>>>> > >>>>>>> > >>>>>>> > >>>>>>> -- > >>>>>>> Alexey Kuznetsov > >>>>>>> > >>>>> > >>>>> > >>>>> > >>>>> -- > >>>>> Best regards, > >>>>> Ivan Pavlukhin > >>>>> > >>> > >>> > >>> > >>> -- > >>> Best regards, > >>> Ivan Pavlukhin > >> > >> > >> > >> -- > >> Best regards, > >> Ivan Pavlukhin > >> > |
Hi Saikat,
Thank you for driving it. I left my comments [1]. [1] https://issues.apache.org/jira/browse/IGNITE-7285 сб, 15 июн. 2019 г. в 20:48, Saikat Maitra <[hidden email]>: > > Hi Ivan, > > Thank you for your email. I have updated the PR to use default query > timeout. > > Please take a look. > > https://github.com/apache/ignite/pull/6490/files > > Regards > Saikat > > On Tue, May 7, 2019 at 12:30 AM Ivan Pavlukhina <[hidden email]> wrote: > > > Hi Saikat, > > > > It is good that we agreed how property in question should be configured. > > But I worry about the following. If the PR merged it will not contain a > > user value yet because an introduced property is not used. Consequently we > > must start using that property before a next AI release. Just one thing to > > keep in mind. > > > > Sent from my iPhone > > > > > On 4 May 2019, at 05:56, Saikat Maitra <[hidden email]> wrote: > > > > > > Hi Ivan, > > > > > > Thank you for reviewing the PR. I have updated the PR. Please review and > > > share your feedback. > > > > > > I was thinking of making a separate PR for using the defaultQueryTimeout > > > property in query execution flow. > > > > > > Regards, > > > Saikat > > > > > >> On Wed, May 1, 2019 at 1:04 AM Павлухин Иван <[hidden email]> > > wrote: > > >> > > >> Andrey K., > > >> > > >>> I think we should develop some kind of "Queries" options on Ignite > > >>> configuration. > > >> > > >> Quite a reasonable idea. We already have couple of query-related > > >> properties in IgniteConfiguration and we can move them (in a > > >> compatible way) to a query properties sub-aggregate. I think it is > > >> better to raise a separate topic for that. > > >> > > >> ср, 1 мая 2019 г. в 09:00, Павлухин Иван <[hidden email]>: > > >>> > > >>> Hi Saikat, > > >>> > > >>> I left a couple of comment on GitHub [1]. > > >>> > > >>> Perhaps I am missing it but what are the plans for making a default > > >>> query timeout workable by using an introduced property in query > > >>> execution flow? > > >>> > > >>> [1] https://github.com/apache/ignite/pull/6490 > > >>> > > >>> вт, 30 апр. 2019 г. в 02:50, Saikat Maitra <[hidden email]>: > > >>>> > > >>>> Hi Ivan, > > >>>> > > >>>> Yes, I checked this CacheQuery default value > > >>>> > > >> > > https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/query/CacheQuery.java#L200 > > >>>> > > >>>> Also, Andrew recommended the same in review feedback. > > >>>> > > >>>> https://github.com/apache/ignite/pull/6490#discussion_r277730394 > > >>>> > > >>>> Regards, > > >>>> Saikat > > >>>> > > >>>> > > >>>> On Mon, Apr 29, 2019 at 3:18 AM Павлухин Иван <[hidden email]> > > >> wrote: > > >>>> > > >>>>> Hi Saikat, > > >>>>> > > >>>>> It a compatibility with previous versions the reason for an > > >> indefinite > > >>>>> timeout by default? > > >>>>> > > >>>>> сб, 27 апр. 2019 г. в 16:58, Saikat Maitra <[hidden email] > > >>> : > > >>>>>> > > >>>>>> Hi Alexey, Ivan, Andrew > > >>>>>> > > >>>>>> I think we can provide an option to configure defaultQueryOption at > > >>>>>> IgniteConfiguration and set the default value = 0 to imply if not > > >> set it > > >>>>>> will be execute indefinitely but then user can set this value > > >> based on > > >>>>> the > > >>>>>> application preferences and use it in addition to SQL query > > >> timeout. > > >>>>>> > > >>>>>> I have updated the PR accordingly. > > >>>>>> > > >>>>>> Please review and share if any changes required. > > >>>>>> > > >>>>>> Regards, > > >>>>>> Saikat > > >>>>>> > > >>>>>> On Wed, Apr 24, 2019 at 4:33 AM Alexey Kuznetsov < > > >> [hidden email]> > > >>>>>> wrote: > > >>>>>> > > >>>>>>> Hi Saikat and Ivan, > > >>>>>>> > > >>>>>>> I think that properties that related to SQL should not be > > >> configured on > > >>>>>>> caches. > > >>>>>>> We already put a lot of effort to decouple SQL from caches. > > >>>>>>> > > >>>>>>> I think we should develop some kind of "Queries" options on > > >> Ignite > > >>>>>>> configuration. > > >>>>>>> > > >>>>>>> What do you think? > > >>>>>>> > > >>>>>>> > > >>>>>>> On Wed, Apr 24, 2019 at 3:22 PM Павлухин Иван < > > >> [hidden email]> > > >>>>> wrote: > > >>>>>>> > > >>>>>>>> Hi Saikat, > > >>>>>>>> > > >>>>>>>> I think that we should have a discussion and choose a place > > >> where a > > >>>>>>>> "default query timeout" property will be configured. > > >>>>>>>> > > >>>>>>>> Generally, I think that a real (user) problem is possibility > > >> for > > >>>>>>>> queries to execute indefinitely. And I have no doubts that we > > >> can > > >>>>>>>> improve there. There could be several implementation > > >> strategies. One > > >>>>>>>> is adding a property to CacheConfiguration. But it opens > > >> various > > >>>>>>>> questions. E.g. how should it work if we execute SQL JOIN > > >> spanning > > >>>>>>>> multiple tables (caches)? Also I am concerned about queries > > >> executed > > >>>>>>>> not via cache.query() method. We have multiple alternative > > >> options, > > >>>>>>>> e.g. thin clients (IgniteClient.query) or JDBC. I believe that > > >>>>>>>> introducing a default timeout for all them is not a bad idea. > > >>>>>>>> > > >>>>>>>> What do you think? > > >>>>>>>> > > >>>>>>>> вт, 23 апр. 2019 г. в 03:01, Saikat Maitra < > > >> [hidden email] > > >>>>>> : > > >>>>>>>>> > > >>>>>>>>> Hi Ivan, > > >>>>>>>>> > > >>>>>>>>> Thank you for your email. My understanding from the jira > > >> issue was > > >>>>> it > > >>>>>>>> will > > >>>>>>>>> be cache level configuration for query default timeout. > > >>>>>>>>> > > >>>>>>>>> I need more info on the usage for this config property and > > >> if it is > > >>>>>>>> shared > > >>>>>>>>> in this jira issue I can make changes or if there is a > > >> separate > > >>>>> jira > > >>>>>>>> issue > > >>>>>>>>> I can assign myself. > > >>>>>>>>> > > >>>>>>>>> > > >>>>>>>>> Regards, > > >>>>>>>>> Saikat > > >>>>>>>>> > > >>>>>>>>> On Mon, Apr 22, 2019 at 5:31 AM Павлухин Иван < > > >> [hidden email] > > >>>>>> > > >>>>>>>> wrote: > > >>>>>>>>> > > >>>>>>>>>> Hi Saikat, > > >>>>>>>>>> > > >>>>>>>>>> I see that a configuration property is added in PR but I > > >> do not > > >>>>> see > > >>>>>>>>>> how the property is used. Was it done intentionally? > > >>>>>>>>>> > > >>>>>>>>>> Also, we need to decide whether such timeout should be > > >>>>> configured per > > >>>>>>>>>> cache or for all caches in one place. For example, we have > > >>>>> already > > >>>>>>>>>> TransactionConfiguration#setDefaultTxTimeout which is a > > >> global > > >>>>> one. > > >>>>>>>>>> > > >>>>>>>>>> Share you thoughts. > > >>>>>>>>>> > > >>>>>>>>>> вс, 21 апр. 2019 г. в 00:38, Saikat Maitra < > > >>>>> [hidden email] > > >>>>>>>> : > > >>>>>>>>>>> > > >>>>>>>>>>> Hi, > > >>>>>>>>>>> > > >>>>>>>>>>> I have raised a PR for the below issue. > > >>>>>>>>>>> > > >>>>>>>>>>> IGNITE-7285 Add default query timeout > > >>>>>>>>>>> > > >>>>>>>>>>> PR - https://github.com/apache/ignite/pull/6490 > > >>>>>>>>>>> > > >>>>>>>>>>> Please take a look and share feedback. > > >>>>>>>>>>> > > >>>>>>>>>>> Regards, > > >>>>>>>>>>> Saikat > > >>>>>>>>>> > > >>>>>>>>>> > > >>>>>>>>>> > > >>>>>>>>>> -- > > >>>>>>>>>> Best regards, > > >>>>>>>>>> Ivan Pavlukhin > > >>>>>>>>>> > > >>>>>>>> > > >>>>>>>> > > >>>>>>>> > > >>>>>>>> -- > > >>>>>>>> Best regards, > > >>>>>>>> Ivan Pavlukhin > > >>>>>>>> > > >>>>>>> > > >>>>>>> > > >>>>>>> -- > > >>>>>>> Alexey Kuznetsov > > >>>>>>> > > >>>>> > > >>>>> > > >>>>> > > >>>>> -- > > >>>>> Best regards, > > >>>>> Ivan Pavlukhin > > >>>>> > > >>> > > >>> > > >>> > > >>> -- > > >>> Best regards, > > >>> Ivan Pavlukhin > > >> > > >> > > >> > > >> -- > > >> Best regards, > > >> Ivan Pavlukhin > > >> > > -- Best regards, Ivan Pavlukhin |
Just to keep history connected. The discussion continued in
http://apache-ignite-developers.2346864.n4.nabble.com/SQL-query-timeout-in-progress-or-abandoned-td42964.html вт, 18 июн. 2019 г. в 12:22, Павлухин Иван <[hidden email]>: > > Hi Saikat, > > Thank you for driving it. I left my comments [1]. > > [1] https://issues.apache.org/jira/browse/IGNITE-7285 > > сб, 15 июн. 2019 г. в 20:48, Saikat Maitra <[hidden email]>: > > > > Hi Ivan, > > > > Thank you for your email. I have updated the PR to use default query > > timeout. > > > > Please take a look. > > > > https://github.com/apache/ignite/pull/6490/files > > > > Regards > > Saikat > > > > On Tue, May 7, 2019 at 12:30 AM Ivan Pavlukhina <[hidden email]> wrote: > > > > > Hi Saikat, > > > > > > It is good that we agreed how property in question should be configured. > > > But I worry about the following. If the PR merged it will not contain a > > > user value yet because an introduced property is not used. Consequently we > > > must start using that property before a next AI release. Just one thing to > > > keep in mind. > > > > > > Sent from my iPhone > > > > > > > On 4 May 2019, at 05:56, Saikat Maitra <[hidden email]> wrote: > > > > > > > > Hi Ivan, > > > > > > > > Thank you for reviewing the PR. I have updated the PR. Please review and > > > > share your feedback. > > > > > > > > I was thinking of making a separate PR for using the defaultQueryTimeout > > > > property in query execution flow. > > > > > > > > Regards, > > > > Saikat > > > > > > > >> On Wed, May 1, 2019 at 1:04 AM Павлухин Иван <[hidden email]> > > > wrote: > > > >> > > > >> Andrey K., > > > >> > > > >>> I think we should develop some kind of "Queries" options on Ignite > > > >>> configuration. > > > >> > > > >> Quite a reasonable idea. We already have couple of query-related > > > >> properties in IgniteConfiguration and we can move them (in a > > > >> compatible way) to a query properties sub-aggregate. I think it is > > > >> better to raise a separate topic for that. > > > >> > > > >> ср, 1 мая 2019 г. в 09:00, Павлухин Иван <[hidden email]>: > > > >>> > > > >>> Hi Saikat, > > > >>> > > > >>> I left a couple of comment on GitHub [1]. > > > >>> > > > >>> Perhaps I am missing it but what are the plans for making a default > > > >>> query timeout workable by using an introduced property in query > > > >>> execution flow? > > > >>> > > > >>> [1] https://github.com/apache/ignite/pull/6490 > > > >>> > > > >>> вт, 30 апр. 2019 г. в 02:50, Saikat Maitra <[hidden email]>: > > > >>>> > > > >>>> Hi Ivan, > > > >>>> > > > >>>> Yes, I checked this CacheQuery default value > > > >>>> > > > >> > > > https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/query/CacheQuery.java#L200 > > > >>>> > > > >>>> Also, Andrew recommended the same in review feedback. > > > >>>> > > > >>>> https://github.com/apache/ignite/pull/6490#discussion_r277730394 > > > >>>> > > > >>>> Regards, > > > >>>> Saikat > > > >>>> > > > >>>> > > > >>>> On Mon, Apr 29, 2019 at 3:18 AM Павлухин Иван <[hidden email]> > > > >> wrote: > > > >>>> > > > >>>>> Hi Saikat, > > > >>>>> > > > >>>>> It a compatibility with previous versions the reason for an > > > >> indefinite > > > >>>>> timeout by default? > > > >>>>> > > > >>>>> сб, 27 апр. 2019 г. в 16:58, Saikat Maitra <[hidden email] > > > >>> : > > > >>>>>> > > > >>>>>> Hi Alexey, Ivan, Andrew > > > >>>>>> > > > >>>>>> I think we can provide an option to configure defaultQueryOption at > > > >>>>>> IgniteConfiguration and set the default value = 0 to imply if not > > > >> set it > > > >>>>>> will be execute indefinitely but then user can set this value > > > >> based on > > > >>>>> the > > > >>>>>> application preferences and use it in addition to SQL query > > > >> timeout. > > > >>>>>> > > > >>>>>> I have updated the PR accordingly. > > > >>>>>> > > > >>>>>> Please review and share if any changes required. > > > >>>>>> > > > >>>>>> Regards, > > > >>>>>> Saikat > > > >>>>>> > > > >>>>>> On Wed, Apr 24, 2019 at 4:33 AM Alexey Kuznetsov < > > > >> [hidden email]> > > > >>>>>> wrote: > > > >>>>>> > > > >>>>>>> Hi Saikat and Ivan, > > > >>>>>>> > > > >>>>>>> I think that properties that related to SQL should not be > > > >> configured on > > > >>>>>>> caches. > > > >>>>>>> We already put a lot of effort to decouple SQL from caches. > > > >>>>>>> > > > >>>>>>> I think we should develop some kind of "Queries" options on > > > >> Ignite > > > >>>>>>> configuration. > > > >>>>>>> > > > >>>>>>> What do you think? > > > >>>>>>> > > > >>>>>>> > > > >>>>>>> On Wed, Apr 24, 2019 at 3:22 PM Павлухин Иван < > > > >> [hidden email]> > > > >>>>> wrote: > > > >>>>>>> > > > >>>>>>>> Hi Saikat, > > > >>>>>>>> > > > >>>>>>>> I think that we should have a discussion and choose a place > > > >> where a > > > >>>>>>>> "default query timeout" property will be configured. > > > >>>>>>>> > > > >>>>>>>> Generally, I think that a real (user) problem is possibility > > > >> for > > > >>>>>>>> queries to execute indefinitely. And I have no doubts that we > > > >> can > > > >>>>>>>> improve there. There could be several implementation > > > >> strategies. One > > > >>>>>>>> is adding a property to CacheConfiguration. But it opens > > > >> various > > > >>>>>>>> questions. E.g. how should it work if we execute SQL JOIN > > > >> spanning > > > >>>>>>>> multiple tables (caches)? Also I am concerned about queries > > > >> executed > > > >>>>>>>> not via cache.query() method. We have multiple alternative > > > >> options, > > > >>>>>>>> e.g. thin clients (IgniteClient.query) or JDBC. I believe that > > > >>>>>>>> introducing a default timeout for all them is not a bad idea. > > > >>>>>>>> > > > >>>>>>>> What do you think? > > > >>>>>>>> > > > >>>>>>>> вт, 23 апр. 2019 г. в 03:01, Saikat Maitra < > > > >> [hidden email] > > > >>>>>> : > > > >>>>>>>>> > > > >>>>>>>>> Hi Ivan, > > > >>>>>>>>> > > > >>>>>>>>> Thank you for your email. My understanding from the jira > > > >> issue was > > > >>>>> it > > > >>>>>>>> will > > > >>>>>>>>> be cache level configuration for query default timeout. > > > >>>>>>>>> > > > >>>>>>>>> I need more info on the usage for this config property and > > > >> if it is > > > >>>>>>>> shared > > > >>>>>>>>> in this jira issue I can make changes or if there is a > > > >> separate > > > >>>>> jira > > > >>>>>>>> issue > > > >>>>>>>>> I can assign myself. > > > >>>>>>>>> > > > >>>>>>>>> > > > >>>>>>>>> Regards, > > > >>>>>>>>> Saikat > > > >>>>>>>>> > > > >>>>>>>>> On Mon, Apr 22, 2019 at 5:31 AM Павлухин Иван < > > > >> [hidden email] > > > >>>>>> > > > >>>>>>>> wrote: > > > >>>>>>>>> > > > >>>>>>>>>> Hi Saikat, > > > >>>>>>>>>> > > > >>>>>>>>>> I see that a configuration property is added in PR but I > > > >> do not > > > >>>>> see > > > >>>>>>>>>> how the property is used. Was it done intentionally? > > > >>>>>>>>>> > > > >>>>>>>>>> Also, we need to decide whether such timeout should be > > > >>>>> configured per > > > >>>>>>>>>> cache or for all caches in one place. For example, we have > > > >>>>> already > > > >>>>>>>>>> TransactionConfiguration#setDefaultTxTimeout which is a > > > >> global > > > >>>>> one. > > > >>>>>>>>>> > > > >>>>>>>>>> Share you thoughts. > > > >>>>>>>>>> > > > >>>>>>>>>> вс, 21 апр. 2019 г. в 00:38, Saikat Maitra < > > > >>>>> [hidden email] > > > >>>>>>>> : > > > >>>>>>>>>>> > > > >>>>>>>>>>> Hi, > > > >>>>>>>>>>> > > > >>>>>>>>>>> I have raised a PR for the below issue. > > > >>>>>>>>>>> > > > >>>>>>>>>>> IGNITE-7285 Add default query timeout > > > >>>>>>>>>>> > > > >>>>>>>>>>> PR - https://github.com/apache/ignite/pull/6490 > > > >>>>>>>>>>> > > > >>>>>>>>>>> Please take a look and share feedback. > > > >>>>>>>>>>> > > > >>>>>>>>>>> Regards, > > > >>>>>>>>>>> Saikat > > > >>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>>>>>> -- > > > >>>>>>>>>> Best regards, > > > >>>>>>>>>> Ivan Pavlukhin > > > >>>>>>>>>> > > > >>>>>>>> > > > >>>>>>>> > > > >>>>>>>> > > > >>>>>>>> -- > > > >>>>>>>> Best regards, > > > >>>>>>>> Ivan Pavlukhin > > > >>>>>>>> > > > >>>>>>> > > > >>>>>>> > > > >>>>>>> -- > > > >>>>>>> Alexey Kuznetsov > > > >>>>>>> > > > >>>>> > > > >>>>> > > > >>>>> > > > >>>>> -- > > > >>>>> Best regards, > > > >>>>> Ivan Pavlukhin > > > >>>>> > > > >>> > > > >>> > > > >>> > > > >>> -- > > > >>> Best regards, > > > >>> Ivan Pavlukhin > > > >> > > > >> > > > >> > > > >> -- > > > >> Best regards, > > > >> Ivan Pavlukhin > > > >> > > > > > > > -- > Best regards, > Ivan Pavlukhin -- Best regards, Ivan Pavlukhin |
Free forum by Nabble | Edit this page |