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 < [email protected]> 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 <[email protected]> > 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 <[email protected]>: > > > > > 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 < > > [email protected]> > > > 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 <[email protected] > >: > > > > > > > > > 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 < > > [email protected] > > > > > > > > > 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 < > > > > > [email protected]> > > > > > > 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 < > > > > > > [email protected] > > > > > > > > > > > > > > > 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 < > > > > > > > [email protected]> > > > > > > > > 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 < > > > > > > > > [email protected] > > > > > > > > > > > > > > > > > > > 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. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
