You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Sergey Evdokimov <se...@gridgain.com> on 2015/03/20 13:32:30 UTC
IgniteException throws from asynchronous cache operation.
Hello,
Failed cache operations throw CacheException, but failed asynchronous
operations throw IgniteException. I think it's wrong. Same synchronous and
asynchronous operation must throw same exception.
BTW. According to contract of java.util.concurrent.Future#get() if result
of operation is an exception Future#get() should throw ExecutionException
that wrap result exception. We break this contract and throw result
exception directly from Future#get(), this may be cause of problems, for
example it's impossible to make out exceptions that threw during
computation and other runtime exceptions.
I propose to keep contract of Future#get() as described in JDK javadocs and
add our method "take" that throw exception directly as implemented at
Ignite currently.
Re: IgniteException throws from asynchronous cache operation.
Posted by Dmitriy Setrakyan <ds...@apache.org>.
Valentin, since you voted for removing the inheritance with Java Future,
can you go ahead and remove it today, before the release? :)
D.
On Fri, Mar 20, 2015 at 11:33 AM, Valentin Kulichenko <
valentin.kulichenko@gmail.com> wrote:
> +1 to removing 'extends Future'. We should either follow Java future's
> contract or introduce our own w/o extending it.
>
> --
> Val
>
> On Fri, Mar 20, 2015 at 9:46 AM, Sergi Vladykin <se...@gmail.com>
> wrote:
>
> > Another option is to have additional method getx() which will not throw
> any
> > checked exceptions, but Future API must be implemented correctly to work
> > with any utilities written for Future handling. Or yes, to be on a safe
> > side we must remove Future extending.
> >
> > Sergi
> >
> > 2015-03-20 19:41 GMT+03:00 Dmitriy Setrakyan <ds...@apache.org>:
> >
> > > Ok, then remove extending java.util.concurrent.Future then if it
> bothers
> > > everyone. There is no practical reason for extending it anyway. Let's
> not
> > > introduce checked exceptions to Ignite, given that the rest of the
> > project
> > > is using unchecked exceptions.
> > >
> > > D.
> > >
> > > On Fri, Mar 20, 2015 at 9:37 AM, Sergi Vladykin <
> > sergi.vladykin@gmail.com>
> > > wrote:
> > >
> > > > I agree with Vladimir. Extending Future and breaking its contract is
> > the
> > > > same thing as if we were supporting JCache but in incompatible way.
> Why
> > > we
> > > > run TCK then?
> > > > I'm not sure about collections of different futures but I remember
> > myself
> > > > writing utility methods like
> > > >
> > > > getFutureResult(Future fut) {
> > > > try {
> > > > return fut.get();
> > > > }
> > > > catch (ExecutionException e) {
> > > > ... do something
> > > > }
> > > > catch (InterruptedException e) {
> > > >
> > > > }
> > > > }
> > > >
> > > > It just will not work. And I don't see any profits from being
> > > incompatible
> > > > here.
> > > >
> > > > Sergi
> > > >
> > > >
> > > > 2015-03-20 19:17 GMT+03:00 Dmitriy Setrakyan <dsetrakyan@apache.org
> >:
> > > >
> > > > > I actually think we are creating a use case that does not exist.
> > Nobody
> > > > > will start casting IgniteFuture to Future. I simply do not see a
> > reason
> > > > for
> > > > > it. Has any of you ever had to create a collection of different
> types
> > > of
> > > > > futures from different products? It just won't happen.
> > > > >
> > > > > The reason we extended java.util.concurrent.Future is to make
> Ignite
> > > > users
> > > > > work with standard Java APIs. However exception handling in Java
> > Future
> > > > is
> > > > > very painful to work with, and that is exactly the reason why we
> > > removed
> > > > > all these checked exceptions from it.
> > > > >
> > > > > D.
> > > > >
> > > > > On Fri, Mar 20, 2015 at 8:36 AM, Vladimir Ozerov <
> > vozerov@gridgain.com
> > > >
> > > > > wrote:
> > > > >
> > > > > > + 1 for following future contract.
> > > > > >
> > > > > > If we have own contract, then why do we extends Future? This only
> > > > > confiuses
> > > > > > users. If he cast our future to Future, then he will expect
> checked
> > > > > > exceptions and will use try-catch blocks, which will never fire
> > > because
> > > > > we
> > > > > > breakes Future semantics.
> > > > > >
> > > > > > Furthermore, even new development effrots in Java 8 respects this
> > > > > contract.
> > > > > > E.g.:
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> http://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html
> > > > > > They explicitly mention the following: "To simplify usage in most
> > > > > contexts,
> > > > > > this class also defines methods join()
> > > > > > <
> > > > > >
> > > > >
> > > >
> > >
> >
> http://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html#join--
> > > > > > >
> > > > > > and getNow(T)
> > > > > > <
> > > > > >
> > > > >
> > > >
> > >
> >
> http://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html#getNow-T-
> > > > > > >
> > > > > > that
> > > > > > instead throw the CompletionException directly in these cases."
> > I.e.
> > > > > > initial contract is preserved still, but another shortcut methods
> > are
> > > > > added
> > > > > > to provide alternate semantics.
> > > > > >
> > > > > > So we either should remove "implements Future", or follow it's
> > > > contract.
> > > > > >
> > > > > > On Fri, Mar 20, 2015 at 6:25 PM, Dmitriy Setrakyan <
> > > > > dsetrakyan@apache.org>
> > > > > > wrote:
> > > > > >
> > > > > > > Hazelcast has a future now too? I wonder where they got that
> idea
> > > :)
> > > > > > >
> > > > > > > Our future overrides the Future.get() method specifically to
> > remove
> > > > all
> > > > > > > checked exceptions from it. This way we make it pretty clear
> that
> > > it
> > > > > will
> > > > > > > never throw ExecutionException. I actually like our design.
> > > > > > >
> > > > > > > D.
> > > > > > >
> > > > > > > On Fri, Mar 20, 2015 at 8:12 AM, Sergey Evdokimov <
> > > > > > sevdokimov@gridgain.com
> > > > > > > >
> > > > > > > wrote:
> > > > > > >
> > > > > > > > I've created a ticket:
> > > > > > https://issues.apache.org/jira/browse/IGNITE-527
> > > > > > > > (Asynchronous cache operations must throw CacheException
> > instead
> > > of
> > > > > > > > IgniteException)
> > > > > > > >
> > > > > > > > The reason of ExecutionException is separation exception that
> > is
> > > > > > > > computation result and other exceptions
> > > > > > > > like CancellationException, InterruptedException. I don't say
> > > that
> > > > > our
> > > > > > > > approach is bad (throwing exception directly without wrapping
> > > into
> > > > > > > > ExecutionException), but we must honor contract described in
> > > > > > > > java.util.concurrent.Future
> > > > > > > > because IgniteFuture extends java.util.concurrent.Future.
> > > > > > Theoretically,
> > > > > > > > user can pass our IgniteFuture to third party code as a
> simple
> > > > > > > > java.util.concurrent.Future, third party code will expect
> > > > > > > > ExecutionException
> > > > > > > > and java.util.concurrent.TimeoutException when call
> "get(...)".
> > > > > > > Hazelcast's
> > > > > > > > future has method 'getSafely()' that throws RuntimeException
> > > > > directly,
> > > > > > > but
> > > > > > > > "get()" works according to java.util.concurrent.Future
> > contract.
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > On Fri, Mar 20, 2015 at 5:13 PM, Dmitriy Setrakyan <
> > > > > > > dsetrakyan@apache.org>
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Agree that sync and async counterparts for the same
> operation
> > > > > should
> > > > > > > > > through the same exception. Is it really not the case now?
> If
> > > > not,
> > > > > we
> > > > > > > > > should fix it.
> > > > > > > > >
> > > > > > > > > Disagree about ExecutionException, as the only reason it
> was
> > > done
> > > > > is
> > > > > > to
> > > > > > > > > support checked exceptions. We have runtime exception, so
> we
> > > can
> > > > > > throw
> > > > > > > > the
> > > > > > > > > correct exception at all times.
> > > > > > > > >
> > > > > > > > > D.
> > > > > > > > >
> > > > > > > > > On Fri, Mar 20, 2015 at 5:32 AM, Sergey Evdokimov <
> > > > > > > > sevdokimov@gridgain.com
> > > > > > > > > >
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Hello,
> > > > > > > > > >
> > > > > > > > > > Failed cache operations throw CacheException, but failed
> > > > > > asynchronous
> > > > > > > > > > operations throw IgniteException. I think it's wrong.
> Same
> > > > > > > synchronous
> > > > > > > > > and
> > > > > > > > > > asynchronous operation must throw same exception.
> > > > > > > > > >
> > > > > > > > > > BTW. According to contract of
> > > java.util.concurrent.Future#get()
> > > > > if
> > > > > > > > result
> > > > > > > > > > of operation is an exception Future#get() should throw
> > > > > > > > ExecutionException
> > > > > > > > > > that wrap result exception. We break this contract and
> > throw
> > > > > result
> > > > > > > > > > exception directly from Future#get(), this may be cause
> of
> > > > > > problems,
> > > > > > > > for
> > > > > > > > > > example it's impossible to make out exceptions that threw
> > > > during
> > > > > > > > > > computation and other runtime exceptions.
> > > > > > > > > > I propose to keep contract of Future#get() as described
> in
> > > JDK
> > > > > > > javadocs
> > > > > > > > > and
> > > > > > > > > > add our method "take" that throw exception directly as
> > > > > implemented
> > > > > > at
> > > > > > > > > > Ignite currently.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: IgniteException throws from asynchronous cache operation.
Posted by Valentin Kulichenko <va...@gmail.com>.
+1 to removing 'extends Future'. We should either follow Java future's
contract or introduce our own w/o extending it.
--
Val
On Fri, Mar 20, 2015 at 9:46 AM, Sergi Vladykin <se...@gmail.com>
wrote:
> Another option is to have additional method getx() which will not throw any
> checked exceptions, but Future API must be implemented correctly to work
> with any utilities written for Future handling. Or yes, to be on a safe
> side we must remove Future extending.
>
> Sergi
>
> 2015-03-20 19:41 GMT+03:00 Dmitriy Setrakyan <ds...@apache.org>:
>
> > Ok, then remove extending java.util.concurrent.Future then if it bothers
> > everyone. There is no practical reason for extending it anyway. Let's not
> > introduce checked exceptions to Ignite, given that the rest of the
> project
> > is using unchecked exceptions.
> >
> > D.
> >
> > On Fri, Mar 20, 2015 at 9:37 AM, Sergi Vladykin <
> sergi.vladykin@gmail.com>
> > wrote:
> >
> > > I agree with Vladimir. Extending Future and breaking its contract is
> the
> > > same thing as if we were supporting JCache but in incompatible way. Why
> > we
> > > run TCK then?
> > > I'm not sure about collections of different futures but I remember
> myself
> > > writing utility methods like
> > >
> > > getFutureResult(Future fut) {
> > > try {
> > > return fut.get();
> > > }
> > > catch (ExecutionException e) {
> > > ... do something
> > > }
> > > catch (InterruptedException e) {
> > >
> > > }
> > > }
> > >
> > > It just will not work. And I don't see any profits from being
> > incompatible
> > > here.
> > >
> > > Sergi
> > >
> > >
> > > 2015-03-20 19:17 GMT+03:00 Dmitriy Setrakyan <ds...@apache.org>:
> > >
> > > > I actually think we are creating a use case that does not exist.
> Nobody
> > > > will start casting IgniteFuture to Future. I simply do not see a
> reason
> > > for
> > > > it. Has any of you ever had to create a collection of different types
> > of
> > > > futures from different products? It just won't happen.
> > > >
> > > > The reason we extended java.util.concurrent.Future is to make Ignite
> > > users
> > > > work with standard Java APIs. However exception handling in Java
> Future
> > > is
> > > > very painful to work with, and that is exactly the reason why we
> > removed
> > > > all these checked exceptions from it.
> > > >
> > > > D.
> > > >
> > > > On Fri, Mar 20, 2015 at 8:36 AM, Vladimir Ozerov <
> vozerov@gridgain.com
> > >
> > > > wrote:
> > > >
> > > > > + 1 for following future contract.
> > > > >
> > > > > If we have own contract, then why do we extends Future? This only
> > > > confiuses
> > > > > users. If he cast our future to Future, then he will expect checked
> > > > > exceptions and will use try-catch blocks, which will never fire
> > because
> > > > we
> > > > > breakes Future semantics.
> > > > >
> > > > > Furthermore, even new development effrots in Java 8 respects this
> > > > contract.
> > > > > E.g.:
> > > > >
> > > > >
> > > >
> > >
> >
> http://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html
> > > > > They explicitly mention the following: "To simplify usage in most
> > > > contexts,
> > > > > this class also defines methods join()
> > > > > <
> > > > >
> > > >
> > >
> >
> http://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html#join--
> > > > > >
> > > > > and getNow(T)
> > > > > <
> > > > >
> > > >
> > >
> >
> http://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html#getNow-T-
> > > > > >
> > > > > that
> > > > > instead throw the CompletionException directly in these cases."
> I.e.
> > > > > initial contract is preserved still, but another shortcut methods
> are
> > > > added
> > > > > to provide alternate semantics.
> > > > >
> > > > > So we either should remove "implements Future", or follow it's
> > > contract.
> > > > >
> > > > > On Fri, Mar 20, 2015 at 6:25 PM, Dmitriy Setrakyan <
> > > > dsetrakyan@apache.org>
> > > > > wrote:
> > > > >
> > > > > > Hazelcast has a future now too? I wonder where they got that idea
> > :)
> > > > > >
> > > > > > Our future overrides the Future.get() method specifically to
> remove
> > > all
> > > > > > checked exceptions from it. This way we make it pretty clear that
> > it
> > > > will
> > > > > > never throw ExecutionException. I actually like our design.
> > > > > >
> > > > > > D.
> > > > > >
> > > > > > On Fri, Mar 20, 2015 at 8:12 AM, Sergey Evdokimov <
> > > > > sevdokimov@gridgain.com
> > > > > > >
> > > > > > wrote:
> > > > > >
> > > > > > > I've created a ticket:
> > > > > https://issues.apache.org/jira/browse/IGNITE-527
> > > > > > > (Asynchronous cache operations must throw CacheException
> instead
> > of
> > > > > > > IgniteException)
> > > > > > >
> > > > > > > The reason of ExecutionException is separation exception that
> is
> > > > > > > computation result and other exceptions
> > > > > > > like CancellationException, InterruptedException. I don't say
> > that
> > > > our
> > > > > > > approach is bad (throwing exception directly without wrapping
> > into
> > > > > > > ExecutionException), but we must honor contract described in
> > > > > > > java.util.concurrent.Future
> > > > > > > because IgniteFuture extends java.util.concurrent.Future.
> > > > > Theoretically,
> > > > > > > user can pass our IgniteFuture to third party code as a simple
> > > > > > > java.util.concurrent.Future, third party code will expect
> > > > > > > ExecutionException
> > > > > > > and java.util.concurrent.TimeoutException when call "get(...)".
> > > > > > Hazelcast's
> > > > > > > future has method 'getSafely()' that throws RuntimeException
> > > > directly,
> > > > > > but
> > > > > > > "get()" works according to java.util.concurrent.Future
> contract.
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > On Fri, Mar 20, 2015 at 5:13 PM, Dmitriy Setrakyan <
> > > > > > dsetrakyan@apache.org>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Agree that sync and async counterparts for the same operation
> > > > should
> > > > > > > > through the same exception. Is it really not the case now? If
> > > not,
> > > > we
> > > > > > > > should fix it.
> > > > > > > >
> > > > > > > > Disagree about ExecutionException, as the only reason it was
> > done
> > > > is
> > > > > to
> > > > > > > > support checked exceptions. We have runtime exception, so we
> > can
> > > > > throw
> > > > > > > the
> > > > > > > > correct exception at all times.
> > > > > > > >
> > > > > > > > D.
> > > > > > > >
> > > > > > > > On Fri, Mar 20, 2015 at 5:32 AM, Sergey Evdokimov <
> > > > > > > sevdokimov@gridgain.com
> > > > > > > > >
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Hello,
> > > > > > > > >
> > > > > > > > > Failed cache operations throw CacheException, but failed
> > > > > asynchronous
> > > > > > > > > operations throw IgniteException. I think it's wrong. Same
> > > > > > synchronous
> > > > > > > > and
> > > > > > > > > asynchronous operation must throw same exception.
> > > > > > > > >
> > > > > > > > > BTW. According to contract of
> > java.util.concurrent.Future#get()
> > > > if
> > > > > > > result
> > > > > > > > > of operation is an exception Future#get() should throw
> > > > > > > ExecutionException
> > > > > > > > > that wrap result exception. We break this contract and
> throw
> > > > result
> > > > > > > > > exception directly from Future#get(), this may be cause of
> > > > > problems,
> > > > > > > for
> > > > > > > > > example it's impossible to make out exceptions that threw
> > > during
> > > > > > > > > computation and other runtime exceptions.
> > > > > > > > > I propose to keep contract of Future#get() as described in
> > JDK
> > > > > > javadocs
> > > > > > > > and
> > > > > > > > > add our method "take" that throw exception directly as
> > > > implemented
> > > > > at
> > > > > > > > > Ignite currently.
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: IgniteException throws from asynchronous cache operation.
Posted by Sergi Vladykin <se...@gmail.com>.
Another option is to have additional method getx() which will not throw any
checked exceptions, but Future API must be implemented correctly to work
with any utilities written for Future handling. Or yes, to be on a safe
side we must remove Future extending.
Sergi
2015-03-20 19:41 GMT+03:00 Dmitriy Setrakyan <ds...@apache.org>:
> Ok, then remove extending java.util.concurrent.Future then if it bothers
> everyone. There is no practical reason for extending it anyway. Let's not
> introduce checked exceptions to Ignite, given that the rest of the project
> is using unchecked exceptions.
>
> D.
>
> On Fri, Mar 20, 2015 at 9:37 AM, Sergi Vladykin <se...@gmail.com>
> wrote:
>
> > I agree with Vladimir. Extending Future and breaking its contract is the
> > same thing as if we were supporting JCache but in incompatible way. Why
> we
> > run TCK then?
> > I'm not sure about collections of different futures but I remember myself
> > writing utility methods like
> >
> > getFutureResult(Future fut) {
> > try {
> > return fut.get();
> > }
> > catch (ExecutionException e) {
> > ... do something
> > }
> > catch (InterruptedException e) {
> >
> > }
> > }
> >
> > It just will not work. And I don't see any profits from being
> incompatible
> > here.
> >
> > Sergi
> >
> >
> > 2015-03-20 19:17 GMT+03:00 Dmitriy Setrakyan <ds...@apache.org>:
> >
> > > I actually think we are creating a use case that does not exist. Nobody
> > > will start casting IgniteFuture to Future. I simply do not see a reason
> > for
> > > it. Has any of you ever had to create a collection of different types
> of
> > > futures from different products? It just won't happen.
> > >
> > > The reason we extended java.util.concurrent.Future is to make Ignite
> > users
> > > work with standard Java APIs. However exception handling in Java Future
> > is
> > > very painful to work with, and that is exactly the reason why we
> removed
> > > all these checked exceptions from it.
> > >
> > > D.
> > >
> > > On Fri, Mar 20, 2015 at 8:36 AM, Vladimir Ozerov <vozerov@gridgain.com
> >
> > > wrote:
> > >
> > > > + 1 for following future contract.
> > > >
> > > > If we have own contract, then why do we extends Future? This only
> > > confiuses
> > > > users. If he cast our future to Future, then he will expect checked
> > > > exceptions and will use try-catch blocks, which will never fire
> because
> > > we
> > > > breakes Future semantics.
> > > >
> > > > Furthermore, even new development effrots in Java 8 respects this
> > > contract.
> > > > E.g.:
> > > >
> > > >
> > >
> >
> http://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html
> > > > They explicitly mention the following: "To simplify usage in most
> > > contexts,
> > > > this class also defines methods join()
> > > > <
> > > >
> > >
> >
> http://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html#join--
> > > > >
> > > > and getNow(T)
> > > > <
> > > >
> > >
> >
> http://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html#getNow-T-
> > > > >
> > > > that
> > > > instead throw the CompletionException directly in these cases." I.e.
> > > > initial contract is preserved still, but another shortcut methods are
> > > added
> > > > to provide alternate semantics.
> > > >
> > > > So we either should remove "implements Future", or follow it's
> > contract.
> > > >
> > > > On Fri, Mar 20, 2015 at 6:25 PM, Dmitriy Setrakyan <
> > > dsetrakyan@apache.org>
> > > > wrote:
> > > >
> > > > > Hazelcast has a future now too? I wonder where they got that idea
> :)
> > > > >
> > > > > Our future overrides the Future.get() method specifically to remove
> > all
> > > > > checked exceptions from it. This way we make it pretty clear that
> it
> > > will
> > > > > never throw ExecutionException. I actually like our design.
> > > > >
> > > > > D.
> > > > >
> > > > > On Fri, Mar 20, 2015 at 8:12 AM, Sergey Evdokimov <
> > > > sevdokimov@gridgain.com
> > > > > >
> > > > > wrote:
> > > > >
> > > > > > I've created a ticket:
> > > > https://issues.apache.org/jira/browse/IGNITE-527
> > > > > > (Asynchronous cache operations must throw CacheException instead
> of
> > > > > > IgniteException)
> > > > > >
> > > > > > The reason of ExecutionException is separation exception that is
> > > > > > computation result and other exceptions
> > > > > > like CancellationException, InterruptedException. I don't say
> that
> > > our
> > > > > > approach is bad (throwing exception directly without wrapping
> into
> > > > > > ExecutionException), but we must honor contract described in
> > > > > > java.util.concurrent.Future
> > > > > > because IgniteFuture extends java.util.concurrent.Future.
> > > > Theoretically,
> > > > > > user can pass our IgniteFuture to third party code as a simple
> > > > > > java.util.concurrent.Future, third party code will expect
> > > > > > ExecutionException
> > > > > > and java.util.concurrent.TimeoutException when call "get(...)".
> > > > > Hazelcast's
> > > > > > future has method 'getSafely()' that throws RuntimeException
> > > directly,
> > > > > but
> > > > > > "get()" works according to java.util.concurrent.Future contract.
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Fri, Mar 20, 2015 at 5:13 PM, Dmitriy Setrakyan <
> > > > > dsetrakyan@apache.org>
> > > > > > wrote:
> > > > > >
> > > > > > > Agree that sync and async counterparts for the same operation
> > > should
> > > > > > > through the same exception. Is it really not the case now? If
> > not,
> > > we
> > > > > > > should fix it.
> > > > > > >
> > > > > > > Disagree about ExecutionException, as the only reason it was
> done
> > > is
> > > > to
> > > > > > > support checked exceptions. We have runtime exception, so we
> can
> > > > throw
> > > > > > the
> > > > > > > correct exception at all times.
> > > > > > >
> > > > > > > D.
> > > > > > >
> > > > > > > On Fri, Mar 20, 2015 at 5:32 AM, Sergey Evdokimov <
> > > > > > sevdokimov@gridgain.com
> > > > > > > >
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Hello,
> > > > > > > >
> > > > > > > > Failed cache operations throw CacheException, but failed
> > > > asynchronous
> > > > > > > > operations throw IgniteException. I think it's wrong. Same
> > > > > synchronous
> > > > > > > and
> > > > > > > > asynchronous operation must throw same exception.
> > > > > > > >
> > > > > > > > BTW. According to contract of
> java.util.concurrent.Future#get()
> > > if
> > > > > > result
> > > > > > > > of operation is an exception Future#get() should throw
> > > > > > ExecutionException
> > > > > > > > that wrap result exception. We break this contract and throw
> > > result
> > > > > > > > exception directly from Future#get(), this may be cause of
> > > > problems,
> > > > > > for
> > > > > > > > example it's impossible to make out exceptions that threw
> > during
> > > > > > > > computation and other runtime exceptions.
> > > > > > > > I propose to keep contract of Future#get() as described in
> JDK
> > > > > javadocs
> > > > > > > and
> > > > > > > > add our method "take" that throw exception directly as
> > > implemented
> > > > at
> > > > > > > > Ignite currently.
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: IgniteException throws from asynchronous cache operation.
Posted by Dmitriy Setrakyan <ds...@apache.org>.
Ok, then remove extending java.util.concurrent.Future then if it bothers
everyone. There is no practical reason for extending it anyway. Let's not
introduce checked exceptions to Ignite, given that the rest of the project
is using unchecked exceptions.
D.
On Fri, Mar 20, 2015 at 9:37 AM, Sergi Vladykin <se...@gmail.com>
wrote:
> I agree with Vladimir. Extending Future and breaking its contract is the
> same thing as if we were supporting JCache but in incompatible way. Why we
> run TCK then?
> I'm not sure about collections of different futures but I remember myself
> writing utility methods like
>
> getFutureResult(Future fut) {
> try {
> return fut.get();
> }
> catch (ExecutionException e) {
> ... do something
> }
> catch (InterruptedException e) {
>
> }
> }
>
> It just will not work. And I don't see any profits from being incompatible
> here.
>
> Sergi
>
>
> 2015-03-20 19:17 GMT+03:00 Dmitriy Setrakyan <ds...@apache.org>:
>
> > I actually think we are creating a use case that does not exist. Nobody
> > will start casting IgniteFuture to Future. I simply do not see a reason
> for
> > it. Has any of you ever had to create a collection of different types of
> > futures from different products? It just won't happen.
> >
> > The reason we extended java.util.concurrent.Future is to make Ignite
> users
> > work with standard Java APIs. However exception handling in Java Future
> is
> > very painful to work with, and that is exactly the reason why we removed
> > all these checked exceptions from it.
> >
> > D.
> >
> > On Fri, Mar 20, 2015 at 8:36 AM, Vladimir Ozerov <vo...@gridgain.com>
> > wrote:
> >
> > > + 1 for following future contract.
> > >
> > > If we have own contract, then why do we extends Future? This only
> > confiuses
> > > users. If he cast our future to Future, then he will expect checked
> > > exceptions and will use try-catch blocks, which will never fire because
> > we
> > > breakes Future semantics.
> > >
> > > Furthermore, even new development effrots in Java 8 respects this
> > contract.
> > > E.g.:
> > >
> > >
> >
> http://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html
> > > They explicitly mention the following: "To simplify usage in most
> > contexts,
> > > this class also defines methods join()
> > > <
> > >
> >
> http://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html#join--
> > > >
> > > and getNow(T)
> > > <
> > >
> >
> http://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html#getNow-T-
> > > >
> > > that
> > > instead throw the CompletionException directly in these cases." I.e.
> > > initial contract is preserved still, but another shortcut methods are
> > added
> > > to provide alternate semantics.
> > >
> > > So we either should remove "implements Future", or follow it's
> contract.
> > >
> > > On Fri, Mar 20, 2015 at 6:25 PM, Dmitriy Setrakyan <
> > dsetrakyan@apache.org>
> > > wrote:
> > >
> > > > Hazelcast has a future now too? I wonder where they got that idea :)
> > > >
> > > > Our future overrides the Future.get() method specifically to remove
> all
> > > > checked exceptions from it. This way we make it pretty clear that it
> > will
> > > > never throw ExecutionException. I actually like our design.
> > > >
> > > > D.
> > > >
> > > > On Fri, Mar 20, 2015 at 8:12 AM, Sergey Evdokimov <
> > > sevdokimov@gridgain.com
> > > > >
> > > > wrote:
> > > >
> > > > > I've created a ticket:
> > > https://issues.apache.org/jira/browse/IGNITE-527
> > > > > (Asynchronous cache operations must throw CacheException instead of
> > > > > IgniteException)
> > > > >
> > > > > The reason of ExecutionException is separation exception that is
> > > > > computation result and other exceptions
> > > > > like CancellationException, InterruptedException. I don't say that
> > our
> > > > > approach is bad (throwing exception directly without wrapping into
> > > > > ExecutionException), but we must honor contract described in
> > > > > java.util.concurrent.Future
> > > > > because IgniteFuture extends java.util.concurrent.Future.
> > > Theoretically,
> > > > > user can pass our IgniteFuture to third party code as a simple
> > > > > java.util.concurrent.Future, third party code will expect
> > > > > ExecutionException
> > > > > and java.util.concurrent.TimeoutException when call "get(...)".
> > > > Hazelcast's
> > > > > future has method 'getSafely()' that throws RuntimeException
> > directly,
> > > > but
> > > > > "get()" works according to java.util.concurrent.Future contract.
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > On Fri, Mar 20, 2015 at 5:13 PM, Dmitriy Setrakyan <
> > > > dsetrakyan@apache.org>
> > > > > wrote:
> > > > >
> > > > > > Agree that sync and async counterparts for the same operation
> > should
> > > > > > through the same exception. Is it really not the case now? If
> not,
> > we
> > > > > > should fix it.
> > > > > >
> > > > > > Disagree about ExecutionException, as the only reason it was done
> > is
> > > to
> > > > > > support checked exceptions. We have runtime exception, so we can
> > > throw
> > > > > the
> > > > > > correct exception at all times.
> > > > > >
> > > > > > D.
> > > > > >
> > > > > > On Fri, Mar 20, 2015 at 5:32 AM, Sergey Evdokimov <
> > > > > sevdokimov@gridgain.com
> > > > > > >
> > > > > > wrote:
> > > > > >
> > > > > > > Hello,
> > > > > > >
> > > > > > > Failed cache operations throw CacheException, but failed
> > > asynchronous
> > > > > > > operations throw IgniteException. I think it's wrong. Same
> > > > synchronous
> > > > > > and
> > > > > > > asynchronous operation must throw same exception.
> > > > > > >
> > > > > > > BTW. According to contract of java.util.concurrent.Future#get()
> > if
> > > > > result
> > > > > > > of operation is an exception Future#get() should throw
> > > > > ExecutionException
> > > > > > > that wrap result exception. We break this contract and throw
> > result
> > > > > > > exception directly from Future#get(), this may be cause of
> > > problems,
> > > > > for
> > > > > > > example it's impossible to make out exceptions that threw
> during
> > > > > > > computation and other runtime exceptions.
> > > > > > > I propose to keep contract of Future#get() as described in JDK
> > > > javadocs
> > > > > > and
> > > > > > > add our method "take" that throw exception directly as
> > implemented
> > > at
> > > > > > > Ignite currently.
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: IgniteException throws from asynchronous cache operation.
Posted by Sergi Vladykin <se...@gmail.com>.
I agree with Vladimir. Extending Future and breaking its contract is the
same thing as if we were supporting JCache but in incompatible way. Why we
run TCK then?
I'm not sure about collections of different futures but I remember myself
writing utility methods like
getFutureResult(Future fut) {
try {
return fut.get();
}
catch (ExecutionException e) {
... do something
}
catch (InterruptedException e) {
}
}
It just will not work. And I don't see any profits from being incompatible
here.
Sergi
2015-03-20 19:17 GMT+03:00 Dmitriy Setrakyan <ds...@apache.org>:
> I actually think we are creating a use case that does not exist. Nobody
> will start casting IgniteFuture to Future. I simply do not see a reason for
> it. Has any of you ever had to create a collection of different types of
> futures from different products? It just won't happen.
>
> The reason we extended java.util.concurrent.Future is to make Ignite users
> work with standard Java APIs. However exception handling in Java Future is
> very painful to work with, and that is exactly the reason why we removed
> all these checked exceptions from it.
>
> D.
>
> On Fri, Mar 20, 2015 at 8:36 AM, Vladimir Ozerov <vo...@gridgain.com>
> wrote:
>
> > + 1 for following future contract.
> >
> > If we have own contract, then why do we extends Future? This only
> confiuses
> > users. If he cast our future to Future, then he will expect checked
> > exceptions and will use try-catch blocks, which will never fire because
> we
> > breakes Future semantics.
> >
> > Furthermore, even new development effrots in Java 8 respects this
> contract.
> > E.g.:
> >
> >
> http://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html
> > They explicitly mention the following: "To simplify usage in most
> contexts,
> > this class also defines methods join()
> > <
> >
> http://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html#join--
> > >
> > and getNow(T)
> > <
> >
> http://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html#getNow-T-
> > >
> > that
> > instead throw the CompletionException directly in these cases." I.e.
> > initial contract is preserved still, but another shortcut methods are
> added
> > to provide alternate semantics.
> >
> > So we either should remove "implements Future", or follow it's contract.
> >
> > On Fri, Mar 20, 2015 at 6:25 PM, Dmitriy Setrakyan <
> dsetrakyan@apache.org>
> > wrote:
> >
> > > Hazelcast has a future now too? I wonder where they got that idea :)
> > >
> > > Our future overrides the Future.get() method specifically to remove all
> > > checked exceptions from it. This way we make it pretty clear that it
> will
> > > never throw ExecutionException. I actually like our design.
> > >
> > > D.
> > >
> > > On Fri, Mar 20, 2015 at 8:12 AM, Sergey Evdokimov <
> > sevdokimov@gridgain.com
> > > >
> > > wrote:
> > >
> > > > I've created a ticket:
> > https://issues.apache.org/jira/browse/IGNITE-527
> > > > (Asynchronous cache operations must throw CacheException instead of
> > > > IgniteException)
> > > >
> > > > The reason of ExecutionException is separation exception that is
> > > > computation result and other exceptions
> > > > like CancellationException, InterruptedException. I don't say that
> our
> > > > approach is bad (throwing exception directly without wrapping into
> > > > ExecutionException), but we must honor contract described in
> > > > java.util.concurrent.Future
> > > > because IgniteFuture extends java.util.concurrent.Future.
> > Theoretically,
> > > > user can pass our IgniteFuture to third party code as a simple
> > > > java.util.concurrent.Future, third party code will expect
> > > > ExecutionException
> > > > and java.util.concurrent.TimeoutException when call "get(...)".
> > > Hazelcast's
> > > > future has method 'getSafely()' that throws RuntimeException
> directly,
> > > but
> > > > "get()" works according to java.util.concurrent.Future contract.
> > > >
> > > >
> > > >
> > > >
> > > > On Fri, Mar 20, 2015 at 5:13 PM, Dmitriy Setrakyan <
> > > dsetrakyan@apache.org>
> > > > wrote:
> > > >
> > > > > Agree that sync and async counterparts for the same operation
> should
> > > > > through the same exception. Is it really not the case now? If not,
> we
> > > > > should fix it.
> > > > >
> > > > > Disagree about ExecutionException, as the only reason it was done
> is
> > to
> > > > > support checked exceptions. We have runtime exception, so we can
> > throw
> > > > the
> > > > > correct exception at all times.
> > > > >
> > > > > D.
> > > > >
> > > > > On Fri, Mar 20, 2015 at 5:32 AM, Sergey Evdokimov <
> > > > sevdokimov@gridgain.com
> > > > > >
> > > > > wrote:
> > > > >
> > > > > > Hello,
> > > > > >
> > > > > > Failed cache operations throw CacheException, but failed
> > asynchronous
> > > > > > operations throw IgniteException. I think it's wrong. Same
> > > synchronous
> > > > > and
> > > > > > asynchronous operation must throw same exception.
> > > > > >
> > > > > > BTW. According to contract of java.util.concurrent.Future#get()
> if
> > > > result
> > > > > > of operation is an exception Future#get() should throw
> > > > ExecutionException
> > > > > > that wrap result exception. We break this contract and throw
> result
> > > > > > exception directly from Future#get(), this may be cause of
> > problems,
> > > > for
> > > > > > example it's impossible to make out exceptions that threw during
> > > > > > computation and other runtime exceptions.
> > > > > > I propose to keep contract of Future#get() as described in JDK
> > > javadocs
> > > > > and
> > > > > > add our method "take" that throw exception directly as
> implemented
> > at
> > > > > > Ignite currently.
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: IgniteException throws from asynchronous cache operation.
Posted by Dmitriy Setrakyan <ds...@apache.org>.
I actually think we are creating a use case that does not exist. Nobody
will start casting IgniteFuture to Future. I simply do not see a reason for
it. Has any of you ever had to create a collection of different types of
futures from different products? It just won't happen.
The reason we extended java.util.concurrent.Future is to make Ignite users
work with standard Java APIs. However exception handling in Java Future is
very painful to work with, and that is exactly the reason why we removed
all these checked exceptions from it.
D.
On Fri, Mar 20, 2015 at 8:36 AM, Vladimir Ozerov <vo...@gridgain.com>
wrote:
> + 1 for following future contract.
>
> If we have own contract, then why do we extends Future? This only confiuses
> users. If he cast our future to Future, then he will expect checked
> exceptions and will use try-catch blocks, which will never fire because we
> breakes Future semantics.
>
> Furthermore, even new development effrots in Java 8 respects this contract.
> E.g.:
>
> http://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html
> They explicitly mention the following: "To simplify usage in most contexts,
> this class also defines methods join()
> <
> http://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html#join--
> >
> and getNow(T)
> <
> http://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html#getNow-T-
> >
> that
> instead throw the CompletionException directly in these cases." I.e.
> initial contract is preserved still, but another shortcut methods are added
> to provide alternate semantics.
>
> So we either should remove "implements Future", or follow it's contract.
>
> On Fri, Mar 20, 2015 at 6:25 PM, Dmitriy Setrakyan <ds...@apache.org>
> wrote:
>
> > Hazelcast has a future now too? I wonder where they got that idea :)
> >
> > Our future overrides the Future.get() method specifically to remove all
> > checked exceptions from it. This way we make it pretty clear that it will
> > never throw ExecutionException. I actually like our design.
> >
> > D.
> >
> > On Fri, Mar 20, 2015 at 8:12 AM, Sergey Evdokimov <
> sevdokimov@gridgain.com
> > >
> > wrote:
> >
> > > I've created a ticket:
> https://issues.apache.org/jira/browse/IGNITE-527
> > > (Asynchronous cache operations must throw CacheException instead of
> > > IgniteException)
> > >
> > > The reason of ExecutionException is separation exception that is
> > > computation result and other exceptions
> > > like CancellationException, InterruptedException. I don't say that our
> > > approach is bad (throwing exception directly without wrapping into
> > > ExecutionException), but we must honor contract described in
> > > java.util.concurrent.Future
> > > because IgniteFuture extends java.util.concurrent.Future.
> Theoretically,
> > > user can pass our IgniteFuture to third party code as a simple
> > > java.util.concurrent.Future, third party code will expect
> > > ExecutionException
> > > and java.util.concurrent.TimeoutException when call "get(...)".
> > Hazelcast's
> > > future has method 'getSafely()' that throws RuntimeException directly,
> > but
> > > "get()" works according to java.util.concurrent.Future contract.
> > >
> > >
> > >
> > >
> > > On Fri, Mar 20, 2015 at 5:13 PM, Dmitriy Setrakyan <
> > dsetrakyan@apache.org>
> > > wrote:
> > >
> > > > Agree that sync and async counterparts for the same operation should
> > > > through the same exception. Is it really not the case now? If not, we
> > > > should fix it.
> > > >
> > > > Disagree about ExecutionException, as the only reason it was done is
> to
> > > > support checked exceptions. We have runtime exception, so we can
> throw
> > > the
> > > > correct exception at all times.
> > > >
> > > > D.
> > > >
> > > > On Fri, Mar 20, 2015 at 5:32 AM, Sergey Evdokimov <
> > > sevdokimov@gridgain.com
> > > > >
> > > > wrote:
> > > >
> > > > > Hello,
> > > > >
> > > > > Failed cache operations throw CacheException, but failed
> asynchronous
> > > > > operations throw IgniteException. I think it's wrong. Same
> > synchronous
> > > > and
> > > > > asynchronous operation must throw same exception.
> > > > >
> > > > > BTW. According to contract of java.util.concurrent.Future#get() if
> > > result
> > > > > of operation is an exception Future#get() should throw
> > > ExecutionException
> > > > > that wrap result exception. We break this contract and throw result
> > > > > exception directly from Future#get(), this may be cause of
> problems,
> > > for
> > > > > example it's impossible to make out exceptions that threw during
> > > > > computation and other runtime exceptions.
> > > > > I propose to keep contract of Future#get() as described in JDK
> > javadocs
> > > > and
> > > > > add our method "take" that throw exception directly as implemented
> at
> > > > > Ignite currently.
> > > > >
> > > >
> > >
> >
>
Re: IgniteException throws from asynchronous cache operation.
Posted by Vladimir Ozerov <vo...@gridgain.com>.
+ 1 for following future contract.
If we have own contract, then why do we extends Future? This only confiuses
users. If he cast our future to Future, then he will expect checked
exceptions and will use try-catch blocks, which will never fire because we
breakes Future semantics.
Furthermore, even new development effrots in Java 8 respects this contract.
E.g.:
http://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html
They explicitly mention the following: "To simplify usage in most contexts,
this class also defines methods join()
<http://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html#join-->
and getNow(T)
<http://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html#getNow-T->
that
instead throw the CompletionException directly in these cases." I.e.
initial contract is preserved still, but another shortcut methods are added
to provide alternate semantics.
So we either should remove "implements Future", or follow it's contract.
On Fri, Mar 20, 2015 at 6:25 PM, Dmitriy Setrakyan <ds...@apache.org>
wrote:
> Hazelcast has a future now too? I wonder where they got that idea :)
>
> Our future overrides the Future.get() method specifically to remove all
> checked exceptions from it. This way we make it pretty clear that it will
> never throw ExecutionException. I actually like our design.
>
> D.
>
> On Fri, Mar 20, 2015 at 8:12 AM, Sergey Evdokimov <sevdokimov@gridgain.com
> >
> wrote:
>
> > I've created a ticket: https://issues.apache.org/jira/browse/IGNITE-527
> > (Asynchronous cache operations must throw CacheException instead of
> > IgniteException)
> >
> > The reason of ExecutionException is separation exception that is
> > computation result and other exceptions
> > like CancellationException, InterruptedException. I don't say that our
> > approach is bad (throwing exception directly without wrapping into
> > ExecutionException), but we must honor contract described in
> > java.util.concurrent.Future
> > because IgniteFuture extends java.util.concurrent.Future. Theoretically,
> > user can pass our IgniteFuture to third party code as a simple
> > java.util.concurrent.Future, third party code will expect
> > ExecutionException
> > and java.util.concurrent.TimeoutException when call "get(...)".
> Hazelcast's
> > future has method 'getSafely()' that throws RuntimeException directly,
> but
> > "get()" works according to java.util.concurrent.Future contract.
> >
> >
> >
> >
> > On Fri, Mar 20, 2015 at 5:13 PM, Dmitriy Setrakyan <
> dsetrakyan@apache.org>
> > wrote:
> >
> > > Agree that sync and async counterparts for the same operation should
> > > through the same exception. Is it really not the case now? If not, we
> > > should fix it.
> > >
> > > Disagree about ExecutionException, as the only reason it was done is to
> > > support checked exceptions. We have runtime exception, so we can throw
> > the
> > > correct exception at all times.
> > >
> > > D.
> > >
> > > On Fri, Mar 20, 2015 at 5:32 AM, Sergey Evdokimov <
> > sevdokimov@gridgain.com
> > > >
> > > wrote:
> > >
> > > > Hello,
> > > >
> > > > Failed cache operations throw CacheException, but failed asynchronous
> > > > operations throw IgniteException. I think it's wrong. Same
> synchronous
> > > and
> > > > asynchronous operation must throw same exception.
> > > >
> > > > BTW. According to contract of java.util.concurrent.Future#get() if
> > result
> > > > of operation is an exception Future#get() should throw
> > ExecutionException
> > > > that wrap result exception. We break this contract and throw result
> > > > exception directly from Future#get(), this may be cause of problems,
> > for
> > > > example it's impossible to make out exceptions that threw during
> > > > computation and other runtime exceptions.
> > > > I propose to keep contract of Future#get() as described in JDK
> javadocs
> > > and
> > > > add our method "take" that throw exception directly as implemented at
> > > > Ignite currently.
> > > >
> > >
> >
>
Re: IgniteException throws from asynchronous cache operation.
Posted by Dmitriy Setrakyan <ds...@apache.org>.
Hazelcast has a future now too? I wonder where they got that idea :)
Our future overrides the Future.get() method specifically to remove all
checked exceptions from it. This way we make it pretty clear that it will
never throw ExecutionException. I actually like our design.
D.
On Fri, Mar 20, 2015 at 8:12 AM, Sergey Evdokimov <se...@gridgain.com>
wrote:
> I've created a ticket: https://issues.apache.org/jira/browse/IGNITE-527
> (Asynchronous cache operations must throw CacheException instead of
> IgniteException)
>
> The reason of ExecutionException is separation exception that is
> computation result and other exceptions
> like CancellationException, InterruptedException. I don't say that our
> approach is bad (throwing exception directly without wrapping into
> ExecutionException), but we must honor contract described in
> java.util.concurrent.Future
> because IgniteFuture extends java.util.concurrent.Future. Theoretically,
> user can pass our IgniteFuture to third party code as a simple
> java.util.concurrent.Future, third party code will expect
> ExecutionException
> and java.util.concurrent.TimeoutException when call "get(...)". Hazelcast's
> future has method 'getSafely()' that throws RuntimeException directly, but
> "get()" works according to java.util.concurrent.Future contract.
>
>
>
>
> On Fri, Mar 20, 2015 at 5:13 PM, Dmitriy Setrakyan <ds...@apache.org>
> wrote:
>
> > Agree that sync and async counterparts for the same operation should
> > through the same exception. Is it really not the case now? If not, we
> > should fix it.
> >
> > Disagree about ExecutionException, as the only reason it was done is to
> > support checked exceptions. We have runtime exception, so we can throw
> the
> > correct exception at all times.
> >
> > D.
> >
> > On Fri, Mar 20, 2015 at 5:32 AM, Sergey Evdokimov <
> sevdokimov@gridgain.com
> > >
> > wrote:
> >
> > > Hello,
> > >
> > > Failed cache operations throw CacheException, but failed asynchronous
> > > operations throw IgniteException. I think it's wrong. Same synchronous
> > and
> > > asynchronous operation must throw same exception.
> > >
> > > BTW. According to contract of java.util.concurrent.Future#get() if
> result
> > > of operation is an exception Future#get() should throw
> ExecutionException
> > > that wrap result exception. We break this contract and throw result
> > > exception directly from Future#get(), this may be cause of problems,
> for
> > > example it's impossible to make out exceptions that threw during
> > > computation and other runtime exceptions.
> > > I propose to keep contract of Future#get() as described in JDK javadocs
> > and
> > > add our method "take" that throw exception directly as implemented at
> > > Ignite currently.
> > >
> >
>
Re: IgniteException throws from asynchronous cache operation.
Posted by Sergey Evdokimov <se...@gridgain.com>.
I've created a ticket: https://issues.apache.org/jira/browse/IGNITE-527
(Asynchronous cache operations must throw CacheException instead of
IgniteException)
The reason of ExecutionException is separation exception that is
computation result and other exceptions
like CancellationException, InterruptedException. I don't say that our
approach is bad (throwing exception directly without wrapping into
ExecutionException), but we must honor contract described in
java.util.concurrent.Future
because IgniteFuture extends java.util.concurrent.Future. Theoretically,
user can pass our IgniteFuture to third party code as a simple
java.util.concurrent.Future, third party code will expect ExecutionException
and java.util.concurrent.TimeoutException when call "get(...)". Hazelcast's
future has method 'getSafely()' that throws RuntimeException directly, but
"get()" works according to java.util.concurrent.Future contract.
On Fri, Mar 20, 2015 at 5:13 PM, Dmitriy Setrakyan <ds...@apache.org>
wrote:
> Agree that sync and async counterparts for the same operation should
> through the same exception. Is it really not the case now? If not, we
> should fix it.
>
> Disagree about ExecutionException, as the only reason it was done is to
> support checked exceptions. We have runtime exception, so we can throw the
> correct exception at all times.
>
> D.
>
> On Fri, Mar 20, 2015 at 5:32 AM, Sergey Evdokimov <sevdokimov@gridgain.com
> >
> wrote:
>
> > Hello,
> >
> > Failed cache operations throw CacheException, but failed asynchronous
> > operations throw IgniteException. I think it's wrong. Same synchronous
> and
> > asynchronous operation must throw same exception.
> >
> > BTW. According to contract of java.util.concurrent.Future#get() if result
> > of operation is an exception Future#get() should throw ExecutionException
> > that wrap result exception. We break this contract and throw result
> > exception directly from Future#get(), this may be cause of problems, for
> > example it's impossible to make out exceptions that threw during
> > computation and other runtime exceptions.
> > I propose to keep contract of Future#get() as described in JDK javadocs
> and
> > add our method "take" that throw exception directly as implemented at
> > Ignite currently.
> >
>
Re: IgniteException throws from asynchronous cache operation.
Posted by Dmitriy Setrakyan <ds...@apache.org>.
Agree that sync and async counterparts for the same operation should
through the same exception. Is it really not the case now? If not, we
should fix it.
Disagree about ExecutionException, as the only reason it was done is to
support checked exceptions. We have runtime exception, so we can throw the
correct exception at all times.
D.
On Fri, Mar 20, 2015 at 5:32 AM, Sergey Evdokimov <se...@gridgain.com>
wrote:
> Hello,
>
> Failed cache operations throw CacheException, but failed asynchronous
> operations throw IgniteException. I think it's wrong. Same synchronous and
> asynchronous operation must throw same exception.
>
> BTW. According to contract of java.util.concurrent.Future#get() if result
> of operation is an exception Future#get() should throw ExecutionException
> that wrap result exception. We break this contract and throw result
> exception directly from Future#get(), this may be cause of problems, for
> example it's impossible to make out exceptions that threw during
> computation and other runtime exceptions.
> I propose to keep contract of Future#get() as described in JDK javadocs and
> add our method "take" that throw exception directly as implemented at
> Ignite currently.
>