ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sergi Vladykin <sergi.vlady...@gmail.com>
Subject Re: IgniteException throws from asynchronous cache operation.
Date Fri, 20 Mar 2015 16:37:25 GMT
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.
> > > > > >
> > > > >
> > > >
> > >
> >
>

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