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 > >> >
