ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Павлухин Иван <vololo...@gmail.com>
Subject Re: IGNITE-7285 Add default query timeout
Date Thu, 15 Aug 2019 07:30:04 GMT
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, Павлухин Иван <vololo100@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.maitra@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 <vololo100@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.maitra@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 Павлухин Иван <vololo100@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, Павлухин Иван <vololo100@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.maitra@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 Павлухин Иван
<vololo100@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

Mime
View raw message