IGNITE-3055

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

IGNITE-3055

Vladislav Pyatkov
Igniters,

I have implemented timeout in DataStreamer by issue IGNITE-3055
<https://issues.apache.org/jira/browse/IGNITE-3055>(*IgniteDataStreamer
can't be timed out*).

That lead the change public API:

1) Added set and get methods (IgniteDataStreamer.timeout(long) and
IgniteDataStreamer.timeout()) for establish. Default value is -1, means
unlimited time.

2) Added new TimeoutException (inherited IgniteException). The exception
will thrown when timeout will be reached (may be take place in
IgniteDataStreamer.addData, IgniteDataStreamer.close and
IgniteDataStreamer.flash operations).

Dmitry, colleagues, please comment or approve.
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3055

dsetrakyan
Hi Vlad,

Can you please list the API changes in the ticket, so others can review
without digging in code?

Thanks,
D.

On Mon, Jul 11, 2016 at 1:14 PM, Vladislav Pyatkov <[hidden email]>
wrote:

> Igniters,
>
> I have implemented timeout in DataStreamer by issue IGNITE-3055
> <https://issues.apache.org/jira/browse/IGNITE-3055>(*IgniteDataStreamer
> can't be timed out*).
>
> That lead the change public API:
>
> 1) Added set and get methods (IgniteDataStreamer.timeout(long) and
> IgniteDataStreamer.timeout()) for establish. Default value is -1, means
> unlimited time.
>
> 2) Added new TimeoutException (inherited IgniteException). The exception
> will thrown when timeout will be reached (may be take place in
> IgniteDataStreamer.addData, IgniteDataStreamer.close and
> IgniteDataStreamer.flash operations).
>
> Dmitry, colleagues, please comment or approve.
>
>
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3055

Vladislav Pyatkov
Dmitriy,

I have added the description of changes in the JIRA ticket.

On Tue, Jul 12, 2016 at 1:28 PM, Dmitriy Setrakyan <[hidden email]>
wrote:

> Hi Vlad,
>
> Can you please list the API changes in the ticket, so others can review
> without digging in code?
>
> Thanks,
> D.
>
> On Mon, Jul 11, 2016 at 1:14 PM, Vladislav Pyatkov <[hidden email]>
> wrote:
>
> > Igniters,
> >
> > I have implemented timeout in DataStreamer by issue IGNITE-3055
> > <https://issues.apache.org/jira/browse/IGNITE-3055>(*IgniteDataStreamer
> > can't be timed out*).
> >
> > That lead the change public API:
> >
> > 1) Added set and get methods (IgniteDataStreamer.timeout(long) and
> > IgniteDataStreamer.timeout()) for establish. Default value is -1, means
> > unlimited time.
> >
> > 2) Added new TimeoutException (inherited IgniteException). The exception
> > will thrown when timeout will be reached (may be take place in
> > IgniteDataStreamer.addData, IgniteDataStreamer.close and
> > IgniteDataStreamer.flash operations).
> >
> > Dmitry, colleagues, please comment or approve.
> >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3055

dsetrakyan
Thanks Vlad.

At a high level, the changes look OK. However, I am not sure about
TimeoutException. Don’t we already have other timeout exceptions in Ignite?
Can we reuse them?

D.

On Tue, Jul 12, 2016 at 1:51 PM, Vladislav Pyatkov <[hidden email]>
wrote:

> Dmitriy,
>
> I have added the description of changes in the JIRA ticket.
>
> On Tue, Jul 12, 2016 at 1:28 PM, Dmitriy Setrakyan <[hidden email]>
> wrote:
>
> > Hi Vlad,
> >
> > Can you please list the API changes in the ticket, so others can review
> > without digging in code?
> >
> > Thanks,
> > D.
> >
> > On Mon, Jul 11, 2016 at 1:14 PM, Vladislav Pyatkov <
> [hidden email]>
> > wrote:
> >
> > > Igniters,
> > >
> > > I have implemented timeout in DataStreamer by issue IGNITE-3055
> > > <https://issues.apache.org/jira/browse/IGNITE-3055
> >(*IgniteDataStreamer
> > > can't be timed out*).
> > >
> > > That lead the change public API:
> > >
> > > 1) Added set and get methods (IgniteDataStreamer.timeout(long) and
> > > IgniteDataStreamer.timeout()) for establish. Default value is -1, means
> > > unlimited time.
> > >
> > > 2) Added new TimeoutException (inherited IgniteException). The
> exception
> > > will thrown when timeout will be reached (may be take place in
> > > IgniteDataStreamer.addData, IgniteDataStreamer.close and
> > > IgniteDataStreamer.flash operations).
> > >
> > > Dmitry, colleagues, please comment or approve.
> > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3055

Vladislav Pyatkov
Dmitriy,

I discussed that with Denis and we have decided create new Exception type
by some reasons:
1) The exception need to be unchecked (the behavior means serious problem
in grid) hence, TimeoutException will implement IgniteException (as
RintimeException).

2) Other timeout exception, which follow the logic, are specific (for
example TransactionTimeoutException).

So TimeoutException will be RuntimeException, will not be specific for
DataStreamer and can be reuse.

On Tue, Jul 12, 2016 at 1:54 PM, Dmitriy Setrakyan <[hidden email]>
wrote:

> Thanks Vlad.
>
> At a high level, the changes look OK. However, I am not sure about
> TimeoutException. Don’t we already have other timeout exceptions in Ignite?
> Can we reuse them?
>
> D.
>
> On Tue, Jul 12, 2016 at 1:51 PM, Vladislav Pyatkov <[hidden email]>
> wrote:
>
> > Dmitriy,
> >
> > I have added the description of changes in the JIRA ticket.
> >
> > On Tue, Jul 12, 2016 at 1:28 PM, Dmitriy Setrakyan <
> [hidden email]>
> > wrote:
> >
> > > Hi Vlad,
> > >
> > > Can you please list the API changes in the ticket, so others can review
> > > without digging in code?
> > >
> > > Thanks,
> > > D.
> > >
> > > On Mon, Jul 11, 2016 at 1:14 PM, Vladislav Pyatkov <
> > [hidden email]>
> > > wrote:
> > >
> > > > Igniters,
> > > >
> > > > I have implemented timeout in DataStreamer by issue IGNITE-3055
> > > > <https://issues.apache.org/jira/browse/IGNITE-3055
> > >(*IgniteDataStreamer
> > > > can't be timed out*).
> > > >
> > > > That lead the change public API:
> > > >
> > > > 1) Added set and get methods (IgniteDataStreamer.timeout(long) and
> > > > IgniteDataStreamer.timeout()) for establish. Default value is -1,
> means
> > > > unlimited time.
> > > >
> > > > 2) Added new TimeoutException (inherited IgniteException). The
> > exception
> > > > will thrown when timeout will be reached (may be take place in
> > > > IgniteDataStreamer.addData, IgniteDataStreamer.close and
> > > > IgniteDataStreamer.flash operations).
> > > >
> > > > Dmitry, colleagues, please comment or approve.
> > > >
> > > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3055

dsetrakyan
If other Timeout Exceptions are specific, shouldn’t this one be specific to
data streamer as well, for consistency reasons, e.g.
IgniteDataStreamerTimeoutExcepiton?

On Tue, Jul 12, 2016 at 2:20 PM, Vladislav Pyatkov <[hidden email]>
wrote:

> Dmitriy,
>
> I discussed that with Denis and we have decided create new Exception type
> by some reasons:
> 1) The exception need to be unchecked (the behavior means serious problem
> in grid) hence, TimeoutException will implement IgniteException (as
> RintimeException).
>
> 2) Other timeout exception, which follow the logic, are specific (for
> example TransactionTimeoutException).
>
> So TimeoutException will be RuntimeException, will not be specific for
> DataStreamer and can be reuse.
>
> On Tue, Jul 12, 2016 at 1:54 PM, Dmitriy Setrakyan <[hidden email]>
> wrote:
>
> > Thanks Vlad.
> >
> > At a high level, the changes look OK. However, I am not sure about
> > TimeoutException. Don’t we already have other timeout exceptions in
> Ignite?
> > Can we reuse them?
> >
> > D.
> >
> > On Tue, Jul 12, 2016 at 1:51 PM, Vladislav Pyatkov <
> [hidden email]>
> > wrote:
> >
> > > Dmitriy,
> > >
> > > I have added the description of changes in the JIRA ticket.
> > >
> > > On Tue, Jul 12, 2016 at 1:28 PM, Dmitriy Setrakyan <
> > [hidden email]>
> > > wrote:
> > >
> > > > Hi Vlad,
> > > >
> > > > Can you please list the API changes in the ticket, so others can
> review
> > > > without digging in code?
> > > >
> > > > Thanks,
> > > > D.
> > > >
> > > > On Mon, Jul 11, 2016 at 1:14 PM, Vladislav Pyatkov <
> > > [hidden email]>
> > > > wrote:
> > > >
> > > > > Igniters,
> > > > >
> > > > > I have implemented timeout in DataStreamer by issue IGNITE-3055
> > > > > <https://issues.apache.org/jira/browse/IGNITE-3055
> > > >(*IgniteDataStreamer
> > > > > can't be timed out*).
> > > > >
> > > > > That lead the change public API:
> > > > >
> > > > > 1) Added set and get methods (IgniteDataStreamer.timeout(long) and
> > > > > IgniteDataStreamer.timeout()) for establish. Default value is -1,
> > means
> > > > > unlimited time.
> > > > >
> > > > > 2) Added new TimeoutException (inherited IgniteException). The
> > > exception
> > > > > will thrown when timeout will be reached (may be take place in
> > > > > IgniteDataStreamer.addData, IgniteDataStreamer.close and
> > > > > IgniteDataStreamer.flash operations).
> > > > >
> > > > > Dmitry, colleagues, please comment or approve.
> > > > >
> > > > >
> > > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3055

Denis Magda
Personally, I don’t get why we need to have specific timeout related exceptions. My preferences is to stop this practice starting from IgniteDataStreamer and create generic TimeoutException that can be used by the new features later.


Denis

> On Jul 12, 2016, at 2:35 PM, Dmitriy Setrakyan <[hidden email]> wrote:
>
> If other Timeout Exceptions are specific, shouldn’t this one be specific to
> data streamer as well, for consistency reasons, e.g.
> IgniteDataStreamerTimeoutExcepiton?
>
> On Tue, Jul 12, 2016 at 2:20 PM, Vladislav Pyatkov <[hidden email]>
> wrote:
>
>> Dmitriy,
>>
>> I discussed that with Denis and we have decided create new Exception type
>> by some reasons:
>> 1) The exception need to be unchecked (the behavior means serious problem
>> in grid) hence, TimeoutException will implement IgniteException (as
>> RintimeException).
>>
>> 2) Other timeout exception, which follow the logic, are specific (for
>> example TransactionTimeoutException).
>>
>> So TimeoutException will be RuntimeException, will not be specific for
>> DataStreamer and can be reuse.
>>
>> On Tue, Jul 12, 2016 at 1:54 PM, Dmitriy Setrakyan <[hidden email]>
>> wrote:
>>
>>> Thanks Vlad.
>>>
>>> At a high level, the changes look OK. However, I am not sure about
>>> TimeoutException. Don’t we already have other timeout exceptions in
>> Ignite?
>>> Can we reuse them?
>>>
>>> D.
>>>
>>> On Tue, Jul 12, 2016 at 1:51 PM, Vladislav Pyatkov <
>> [hidden email]>
>>> wrote:
>>>
>>>> Dmitriy,
>>>>
>>>> I have added the description of changes in the JIRA ticket.
>>>>
>>>> On Tue, Jul 12, 2016 at 1:28 PM, Dmitriy Setrakyan <
>>> [hidden email]>
>>>> wrote:
>>>>
>>>>> Hi Vlad,
>>>>>
>>>>> Can you please list the API changes in the ticket, so others can
>> review
>>>>> without digging in code?
>>>>>
>>>>> Thanks,
>>>>> D.
>>>>>
>>>>> On Mon, Jul 11, 2016 at 1:14 PM, Vladislav Pyatkov <
>>>> [hidden email]>
>>>>> wrote:
>>>>>
>>>>>> Igniters,
>>>>>>
>>>>>> I have implemented timeout in DataStreamer by issue IGNITE-3055
>>>>>> <https://issues.apache.org/jira/browse/IGNITE-3055
>>>>> (*IgniteDataStreamer
>>>>>> can't be timed out*).
>>>>>>
>>>>>> That lead the change public API:
>>>>>>
>>>>>> 1) Added set and get methods (IgniteDataStreamer.timeout(long) and
>>>>>> IgniteDataStreamer.timeout()) for establish. Default value is -1,
>>> means
>>>>>> unlimited time.
>>>>>>
>>>>>> 2) Added new TimeoutException (inherited IgniteException). The
>>>> exception
>>>>>> will thrown when timeout will be reached (may be take place in
>>>>>> IgniteDataStreamer.addData, IgniteDataStreamer.close and
>>>>>> IgniteDataStreamer.flash operations).
>>>>>>
>>>>>> Dmitry, colleagues, please comment or approve.
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3055

dsetrakyan
On Tue, Jul 12, 2016 at 2:40 PM, Denis Magda <[hidden email]> wrote:

> Personally, I don’t get why we need to have specific timeout related
> exceptions. My preferences is to stop this practice starting from
> IgniteDataStreamer and create generic TimeoutException that can be used by
> the new features later.
>

Disagree. We should either fix it everywhere, or remain consistent
everywhere. From the API standpoint, it will be very confusing if we don’t.

One way to fix it everywhere, and to preserve backward compatibility, we
could have all specific TimeoutExceptions inherit from the generic
TimeoutException and then deprecate them.


>
> —
> Denis
>
> > On Jul 12, 2016, at 2:35 PM, Dmitriy Setrakyan <[hidden email]>
> wrote:
> >
> > If other Timeout Exceptions are specific, shouldn’t this one be specific
> to
> > data streamer as well, for consistency reasons, e.g.
> > IgniteDataStreamerTimeoutExcepiton?
> >
> > On Tue, Jul 12, 2016 at 2:20 PM, Vladislav Pyatkov <
> [hidden email]>
> > wrote:
> >
> >> Dmitriy,
> >>
> >> I discussed that with Denis and we have decided create new Exception
> type
> >> by some reasons:
> >> 1) The exception need to be unchecked (the behavior means serious
> problem
> >> in grid) hence, TimeoutException will implement IgniteException (as
> >> RintimeException).
> >>
> >> 2) Other timeout exception, which follow the logic, are specific (for
> >> example TransactionTimeoutException).
> >>
> >> So TimeoutException will be RuntimeException, will not be specific for
> >> DataStreamer and can be reuse.
> >>
> >> On Tue, Jul 12, 2016 at 1:54 PM, Dmitriy Setrakyan <
> [hidden email]>
> >> wrote:
> >>
> >>> Thanks Vlad.
> >>>
> >>> At a high level, the changes look OK. However, I am not sure about
> >>> TimeoutException. Don’t we already have other timeout exceptions in
> >> Ignite?
> >>> Can we reuse them?
> >>>
> >>> D.
> >>>
> >>> On Tue, Jul 12, 2016 at 1:51 PM, Vladislav Pyatkov <
> >> [hidden email]>
> >>> wrote:
> >>>
> >>>> Dmitriy,
> >>>>
> >>>> I have added the description of changes in the JIRA ticket.
> >>>>
> >>>> On Tue, Jul 12, 2016 at 1:28 PM, Dmitriy Setrakyan <
> >>> [hidden email]>
> >>>> wrote:
> >>>>
> >>>>> Hi Vlad,
> >>>>>
> >>>>> Can you please list the API changes in the ticket, so others can
> >> review
> >>>>> without digging in code?
> >>>>>
> >>>>> Thanks,
> >>>>> D.
> >>>>>
> >>>>> On Mon, Jul 11, 2016 at 1:14 PM, Vladislav Pyatkov <
> >>>> [hidden email]>
> >>>>> wrote:
> >>>>>
> >>>>>> Igniters,
> >>>>>>
> >>>>>> I have implemented timeout in DataStreamer by issue IGNITE-3055
> >>>>>> <https://issues.apache.org/jira/browse/IGNITE-3055
> >>>>> (*IgniteDataStreamer
> >>>>>> can't be timed out*).
> >>>>>>
> >>>>>> That lead the change public API:
> >>>>>>
> >>>>>> 1) Added set and get methods (IgniteDataStreamer.timeout(long) and
> >>>>>> IgniteDataStreamer.timeout()) for establish. Default value is -1,
> >>> means
> >>>>>> unlimited time.
> >>>>>>
> >>>>>> 2) Added new TimeoutException (inherited IgniteException). The
> >>>> exception
> >>>>>> will thrown when timeout will be reached (may be take place in
> >>>>>> IgniteDataStreamer.addData, IgniteDataStreamer.close and
> >>>>>> IgniteDataStreamer.flash operations).
> >>>>>>
> >>>>>> Dmitry, colleagues, please comment or approve.
> >>>>>>
> >>>>>>
> >>>>>
> >>>>
> >>>
> >>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3055

Denis Magda
Ok, then I would go for IgniteDataStreamerTimeoutException.


Denis

> On Jul 13, 2016, at 6:08 AM, Dmitriy Setrakyan <[hidden email]> wrote:
>
> On Tue, Jul 12, 2016 at 2:40 PM, Denis Magda <[hidden email]> wrote:
>
>> Personally, I don’t get why we need to have specific timeout related
>> exceptions. My preferences is to stop this practice starting from
>> IgniteDataStreamer and create generic TimeoutException that can be used by
>> the new features later.
>>
>
> Disagree. We should either fix it everywhere, or remain consistent
> everywhere. From the API standpoint, it will be very confusing if we don’t.
>
> One way to fix it everywhere, and to preserve backward compatibility, we
> could have all specific TimeoutExceptions inherit from the generic
> TimeoutException and then deprecate them.
>
>
>>
>> —
>> Denis
>>
>>> On Jul 12, 2016, at 2:35 PM, Dmitriy Setrakyan <[hidden email]>
>> wrote:
>>>
>>> If other Timeout Exceptions are specific, shouldn’t this one be specific
>> to
>>> data streamer as well, for consistency reasons, e.g.
>>> IgniteDataStreamerTimeoutExcepiton?
>>>
>>> On Tue, Jul 12, 2016 at 2:20 PM, Vladislav Pyatkov <
>> [hidden email]>
>>> wrote:
>>>
>>>> Dmitriy,
>>>>
>>>> I discussed that with Denis and we have decided create new Exception
>> type
>>>> by some reasons:
>>>> 1) The exception need to be unchecked (the behavior means serious
>> problem
>>>> in grid) hence, TimeoutException will implement IgniteException (as
>>>> RintimeException).
>>>>
>>>> 2) Other timeout exception, which follow the logic, are specific (for
>>>> example TransactionTimeoutException).
>>>>
>>>> So TimeoutException will be RuntimeException, will not be specific for
>>>> DataStreamer and can be reuse.
>>>>
>>>> On Tue, Jul 12, 2016 at 1:54 PM, Dmitriy Setrakyan <
>> [hidden email]>
>>>> wrote:
>>>>
>>>>> Thanks Vlad.
>>>>>
>>>>> At a high level, the changes look OK. However, I am not sure about
>>>>> TimeoutException. Don’t we already have other timeout exceptions in
>>>> Ignite?
>>>>> Can we reuse them?
>>>>>
>>>>> D.
>>>>>
>>>>> On Tue, Jul 12, 2016 at 1:51 PM, Vladislav Pyatkov <
>>>> [hidden email]>
>>>>> wrote:
>>>>>
>>>>>> Dmitriy,
>>>>>>
>>>>>> I have added the description of changes in the JIRA ticket.
>>>>>>
>>>>>> On Tue, Jul 12, 2016 at 1:28 PM, Dmitriy Setrakyan <
>>>>> [hidden email]>
>>>>>> wrote:
>>>>>>
>>>>>>> Hi Vlad,
>>>>>>>
>>>>>>> Can you please list the API changes in the ticket, so others can
>>>> review
>>>>>>> without digging in code?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> D.
>>>>>>>
>>>>>>> On Mon, Jul 11, 2016 at 1:14 PM, Vladislav Pyatkov <
>>>>>> [hidden email]>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Igniters,
>>>>>>>>
>>>>>>>> I have implemented timeout in DataStreamer by issue IGNITE-3055
>>>>>>>> <https://issues.apache.org/jira/browse/IGNITE-3055
>>>>>>> (*IgniteDataStreamer
>>>>>>>> can't be timed out*).
>>>>>>>>
>>>>>>>> That lead the change public API:
>>>>>>>>
>>>>>>>> 1) Added set and get methods (IgniteDataStreamer.timeout(long) and
>>>>>>>> IgniteDataStreamer.timeout()) for establish. Default value is -1,
>>>>> means
>>>>>>>> unlimited time.
>>>>>>>>
>>>>>>>> 2) Added new TimeoutException (inherited IgniteException). The
>>>>>> exception
>>>>>>>> will thrown when timeout will be reached (may be take place in
>>>>>>>> IgniteDataStreamer.addData, IgniteDataStreamer.close and
>>>>>>>> IgniteDataStreamer.flash operations).
>>>>>>>>
>>>>>>>> Dmitry, colleagues, please comment or approve.
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>
>>