You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Павлухин Иван <vo...@gmail.com> on 2019/05/01 06:00:40 UTC

Re: IGNITE-7285 Add default query timeout

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 <sa...@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 Павлухин Иван <vo...@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 <sa...@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 <ak...@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 Павлухин Иван <vo...@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.maitra@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 Павлухин Иван <vololo100@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.maitra@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

Re: IGNITE-7285 Add default query timeout

Posted by Павлухин Иван <vo...@gmail.com>.
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, Павлухин Иван <vo...@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 <sa...@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 <vo...@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 <sa...@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 Павлухин Иван <vo...@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, Павлухин Иван <vo...@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 <sa...@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 Павлухин Иван <vo...@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.maitra@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 <
> > > >> akuznetsov@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 Павлухин Иван <
> > > >> vololo100@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.maitra@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 Павлухин Иван <
> > > >> vololo100@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.maitra@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

Re: IGNITE-7285 Add default query timeout

Posted by Павлухин Иван <vo...@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 <sa...@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 <vo...@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 <sa...@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 Павлухин Иван <vo...@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, Павлухин Иван <vo...@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 <sa...@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 Павлухин Иван <vo...@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.maitra@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 <
> > >> akuznetsov@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 Павлухин Иван <
> > >> vololo100@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.maitra@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 Павлухин Иван <
> > >> vololo100@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.maitra@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

Re: IGNITE-7285 Add default query timeout

Posted by Saikat Maitra <sa...@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 <vo...@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 <sa...@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 Павлухин Иван <vo...@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, Павлухин Иван <vo...@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 <sa...@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 Павлухин Иван <vo...@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.maitra@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 <
> >> akuznetsov@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 Павлухин Иван <
> >> vololo100@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.maitra@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 Павлухин Иван <
> >> vololo100@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.maitra@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
> >>
>

Re: IGNITE-7285 Add default query timeout

Posted by Ivan Pavlukhina <vo...@gmail.com>.
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 <sa...@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 Павлухин Иван <vo...@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, Павлухин Иван <vo...@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 <sa...@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 Павлухин Иван <vo...@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.maitra@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 <
>> akuznetsov@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 Павлухин Иван <
>> vololo100@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.maitra@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 Павлухин Иван <
>> vololo100@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.maitra@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
>> 

Re: IGNITE-7285 Add default query timeout

Posted by Saikat Maitra <sa...@gmail.com>.
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 Павлухин Иван <vo...@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, Павлухин Иван <vo...@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 <sa...@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 Павлухин Иван <vo...@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.maitra@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 <
> akuznetsov@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 Павлухин Иван <
> vololo100@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.maitra@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 Павлухин Иван <
> vololo100@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.maitra@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
>

Re: IGNITE-7285 Add default query timeout

Posted by Павлухин Иван <vo...@gmail.com>.
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, Павлухин Иван <vo...@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 <sa...@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 Павлухин Иван <vo...@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 <sa...@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 <ak...@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 Павлухин Иван <vo...@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.maitra@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 Павлухин Иван <vololo100@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.maitra@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