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 Павлухин Иван <vololo...@gmail.com> 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, Павлухин Иван <vololo...@gmail.com>: > > > > 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 <saikat.mai...@gmail.com>: > > > > > > 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 Павлухин Иван <vololo...@gmail.com> > wrote: > > > > > > > Hi Saikat, > > > > > > > > It a compatibility with previous versions the reason for an > indefinite > > > > timeout by default? > > > > > > > > сб, 27 апр. 2019 г. в 16:58, Saikat Maitra <saikat.mai...@gmail.com > >: > > > > > > > > > > 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 < > akuznet...@apache.org> > > > > > 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 Павлухин Иван < > vololo...@gmail.com> > > > > 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 < > saikat.mai...@gmail.com > > > > >: > > > > > > > > > > > > > > > > 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 Павлухин Иван < > vololo...@gmail.com > > > > > > > > > > > > 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 < > > > > saikat.mai...@gmail.com > > > > > > >: > > > > > > > > > > > > > > > > > > > > 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 >
