ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Saikat Maitra <saikat.mai...@gmail.com>
Subject Re: IGNITE-7285 Add default query timeout
Date Sat, 15 Jun 2019 17:47:47 GMT
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
> >>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message