Serializable futures?

[Valentin Kulichenko <va...@gmail.com>]

Igniters,

Anyone knows why GridFutureAdapter implements Externalizable? This is
useless from my point of view and error-prone, because it forces him to
have default constructor. It's invalid to use it directly in code, but for
some reason there are 84 (!!!) usages and I just caught an assertion
because of one of them.

I think we should remove Externalizable from futures, remove default
constructor and revisit all wrong usages.

If there are no objections, I will create a ticket.

Thanks!
--
Valentin

Re: Serializable futures?

[Dmitriy Setrakyan <ds...@apache.org>]

On Thu, Feb 19, 2015 at 5:46 AM, Yakov Zhdanov <yz...@apache.org> wrote:

> Guys,
>
> I looked a bit closer to java8 future. It has pretty cool features, but I
> suggest not to backport them. For now we have listenAsync() and chain()
> methods and if anyone needs such rich functionality he can implement that.
> We will definitely implement CompletableFuture as a part of java8
> migration, but a bit later.
>
> So, if there are no objections let's proceed with the issues (ticket
> updated):
>
> 1. Rename listenAsync(lsnr) -> listen(lsnr)
>

Agree.


> 1.1 Async listening may be implemented inside listener itself - handlong
> logic will be submitted to some pool. Convenience adapters may be added.
>

I am not sure what is the difference between listenAsync() and listen(), I
think this is just a renaming. However, you may be talking about
syncNotify() and concurrentNotify() flags. I don't think we should make an
effort of trying to change them as it is simply not worth it.


> 1.2 Async chaining may be implemented in the same way.
>

Again, not sure why we need this. We are not providing generic language
futures, like JDK8. We only should care about Ignite use cases here.


> 1.3 Java8 returns futures from "listening" methods that get done when
> listener completes. I don's think we should implement it now. Let's
> postpone it.
>

Agree.


> 2. we can remove context - it will not be used in future any more
>

The reason for the context was to do notifications in a different thread. I
bet this will be very hard to remove. I don't think it is worthwhile
tackling it right now.


> 3. remove boolean fields that manage async settings
>

Again, not sure why.


> 4. remove validity check + externalizable
>

Agree.


> 5. remove listeners collection, but have 1 listener and chain them if
> needed. I suspect in 99% of cases we have only 1 listener.
>

On the list of priorities, this would be very low for me. Should we even
spend time?



>
> --Yakov
>
> 2015-02-10 13:57 GMT+03:00 Dmitriy Setrakyan <ds...@apache.org>:
>
> > My only comment is that it seems to be a big rework and should not be
> > addressed in this sprint. I also suggest that we look at
> CompletableFuture
> > from JDK8:
> >
> >
> https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html
> >
> > D.
> >
> > On Tue, Feb 10, 2015 at 1:54 AM, Yakov Zhdanov <yz...@gridgain.com>
> > wrote:
> >
> > > Guys, I think that we should rework our future to make it more clean
> and
> > > lightweight.
> > >
> > > 1. remove async notification - everyone can create listener that
> executes
> > > in async manner
> > > 2. we can remove context - it will not be used in future any more
> > > 3. remove boolean fields that manage async settings
> > > 4. remove validity check + externalizable
> > > 5. remove listeners collection, but have 1 listener and chain them if
> > > needed. I suspect in 99% of cases we have only 1 listener.
> > >
> > > Feel free to comment -
> https://issues.apache.org/jira/browse/IGNITE-216
> > >
> > > --
> > > Yakov Zhdanov, Director R&D
> > > *GridGain Systems*
> > > www.gridgain.com
> > >
> > > 2015-02-10 3:52 GMT+03:00 Dmitriy Setrakyan <ds...@apache.org>:
> > >
> > > > +1
> > > >
> > > > On Mon, Feb 9, 2015 at 4:35 PM, Valentin Kulichenko <
> > > > valentin.kulichenko@gmail.com> wrote:
> > > >
> > > > > Igniters,
> > > > >
> > > > > Anyone knows why GridFutureAdapter implements Externalizable? This
> is
> > > > > useless from my point of view and error-prone, because it forces
> him
> > to
> > > > > have default constructor. It's invalid to use it directly in code,
> > but
> > > > for
> > > > > some reason there are 84 (!!!) usages and I just caught an
> assertion
> > > > > because of one of them.
> > > > >
> > > > > I think we should remove Externalizable from futures, remove
> default
> > > > > constructor and revisit all wrong usages.
> > > > >
> > > > > If there are no objections, I will create a ticket.
> > > > >
> > > > > Thanks!
> > > > > --
> > > > > Valentin
> > > > >
> > > >
> > >
> >
>

Re: Serializable futures?

[Yakov Zhdanov <yz...@apache.org>]

Guys,

I looked a bit closer to java8 future. It has pretty cool features, but I
suggest not to backport them. For now we have listenAsync() and chain()
methods and if anyone needs such rich functionality he can implement that.
We will definitely implement CompletableFuture as a part of java8
migration, but a bit later.

So, if there are no objections let's proceed with the issues (ticket
updated):

1. Rename listenAsync(lsnr) -> listen(lsnr)
1.1 Async listening may be implemented inside listener itself - handlong
logic will be submitted to some pool. Convenience adapters may be added.
1.2 Async chaining may be implemented in the same way.
1.3 Java8 returns futures from "listening" methods that get done when
listener completes. I don's think we should implement it now. Let's
postpone it.
2. we can remove context - it will not be used in future any more
3. remove boolean fields that manage async settings
4. remove validity check + externalizable
5. remove listeners collection, but have 1 listener and chain them if
needed. I suspect in 99% of cases we have only 1 listener.

--Yakov

2015-02-10 13:57 GMT+03:00 Dmitriy Setrakyan <ds...@apache.org>:

> My only comment is that it seems to be a big rework and should not be
> addressed in this sprint. I also suggest that we look at CompletableFuture
> from JDK8:
>
> https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html
>
> D.
>
> On Tue, Feb 10, 2015 at 1:54 AM, Yakov Zhdanov <yz...@gridgain.com>
> wrote:
>
> > Guys, I think that we should rework our future to make it more clean and
> > lightweight.
> >
> > 1. remove async notification - everyone can create listener that executes
> > in async manner
> > 2. we can remove context - it will not be used in future any more
> > 3. remove boolean fields that manage async settings
> > 4. remove validity check + externalizable
> > 5. remove listeners collection, but have 1 listener and chain them if
> > needed. I suspect in 99% of cases we have only 1 listener.
> >
> > Feel free to comment - https://issues.apache.org/jira/browse/IGNITE-216
> >
> > --
> > Yakov Zhdanov, Director R&D
> > *GridGain Systems*
> > www.gridgain.com
> >
> > 2015-02-10 3:52 GMT+03:00 Dmitriy Setrakyan <ds...@apache.org>:
> >
> > > +1
> > >
> > > On Mon, Feb 9, 2015 at 4:35 PM, Valentin Kulichenko <
> > > valentin.kulichenko@gmail.com> wrote:
> > >
> > > > Igniters,
> > > >
> > > > Anyone knows why GridFutureAdapter implements Externalizable? This is
> > > > useless from my point of view and error-prone, because it forces him
> to
> > > > have default constructor. It's invalid to use it directly in code,
> but
> > > for
> > > > some reason there are 84 (!!!) usages and I just caught an assertion
> > > > because of one of them.
> > > >
> > > > I think we should remove Externalizable from futures, remove default
> > > > constructor and revisit all wrong usages.
> > > >
> > > > If there are no objections, I will create a ticket.
> > > >
> > > > Thanks!
> > > > --
> > > > Valentin
> > > >
> > >
> >
>

Re: Serializable futures?

[Dmitriy Setrakyan <ds...@apache.org>]

My only comment is that it seems to be a big rework and should not be
addressed in this sprint. I also suggest that we look at CompletableFuture
from JDK8:
https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html

D.

On Tue, Feb 10, 2015 at 1:54 AM, Yakov Zhdanov <yz...@gridgain.com>
wrote:

> Guys, I think that we should rework our future to make it more clean and
> lightweight.
>
> 1. remove async notification - everyone can create listener that executes
> in async manner
> 2. we can remove context - it will not be used in future any more
> 3. remove boolean fields that manage async settings
> 4. remove validity check + externalizable
> 5. remove listeners collection, but have 1 listener and chain them if
> needed. I suspect in 99% of cases we have only 1 listener.
>
> Feel free to comment - https://issues.apache.org/jira/browse/IGNITE-216
>
> --
> Yakov Zhdanov, Director R&D
> *GridGain Systems*
> www.gridgain.com
>
> 2015-02-10 3:52 GMT+03:00 Dmitriy Setrakyan <ds...@apache.org>:
>
> > +1
> >
> > On Mon, Feb 9, 2015 at 4:35 PM, Valentin Kulichenko <
> > valentin.kulichenko@gmail.com> wrote:
> >
> > > Igniters,
> > >
> > > Anyone knows why GridFutureAdapter implements Externalizable? This is
> > > useless from my point of view and error-prone, because it forces him to
> > > have default constructor. It's invalid to use it directly in code, but
> > for
> > > some reason there are 84 (!!!) usages and I just caught an assertion
> > > because of one of them.
> > >
> > > I think we should remove Externalizable from futures, remove default
> > > constructor and revisit all wrong usages.
> > >
> > > If there are no objections, I will create a ticket.
> > >
> > > Thanks!
> > > --
> > > Valentin
> > >
> >
>

Re: Serializable futures?

[Yakov Zhdanov <yz...@gridgain.com>]

Guys, I think that we should rework our future to make it more clean and
lightweight.

1. remove async notification - everyone can create listener that executes
in async manner
2. we can remove context - it will not be used in future any more
3. remove boolean fields that manage async settings
4. remove validity check + externalizable
5. remove listeners collection, but have 1 listener and chain them if
needed. I suspect in 99% of cases we have only 1 listener.

Feel free to comment - https://issues.apache.org/jira/browse/IGNITE-216

--
Yakov Zhdanov, Director R&D
*GridGain Systems*
www.gridgain.com

2015-02-10 3:52 GMT+03:00 Dmitriy Setrakyan <ds...@apache.org>:

> +1
>
> On Mon, Feb 9, 2015 at 4:35 PM, Valentin Kulichenko <
> valentin.kulichenko@gmail.com> wrote:
>
> > Igniters,
> >
> > Anyone knows why GridFutureAdapter implements Externalizable? This is
> > useless from my point of view and error-prone, because it forces him to
> > have default constructor. It's invalid to use it directly in code, but
> for
> > some reason there are 84 (!!!) usages and I just caught an assertion
> > because of one of them.
> >
> > I think we should remove Externalizable from futures, remove default
> > constructor and revisit all wrong usages.
> >
> > If there are no objections, I will create a ticket.
> >
> > Thanks!
> > --
> > Valentin
> >
>

Re: Serializable futures?

[Dmitriy Setrakyan <ds...@apache.org>]

+1

On Mon, Feb 9, 2015 at 4:35 PM, Valentin Kulichenko <
valentin.kulichenko@gmail.com> wrote:

> Igniters,
>
> Anyone knows why GridFutureAdapter implements Externalizable? This is
> useless from my point of view and error-prone, because it forces him to
> have default constructor. It's invalid to use it directly in code, but for
> some reason there are 84 (!!!) usages and I just caught an assertion
> because of one of them.
>
> I think we should remove Externalizable from futures, remove default
> constructor and revisit all wrong usages.
>
> If there are no objections, I will create a ticket.
>
> Thanks!
> --
> Valentin
>