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, Павлухин Иван <vololo...@gmail.com>: > > 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 <saikat.mai...@gmail.com>: > > > > 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 <vololo...@gmail.com> 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 <saikat.mai...@gmail.com> 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 Павлухин Иван <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 > > > >> > > > > > > > -- > Best regards, > Ivan Pavlukhin -- Best regards, Ivan Pavlukhin
