Well, IMHO it's important to understand that .then provides chaining and
.onX provides callbacks. These are different primitives.
We talked in the past about providing more forms of chaining. For example,
you can imagine that .then takes a function that takes a Future<X> rather
than a X in order to support chaining against failed or discarded results:
f.then([](Future<X>) -> Y { ... })
.then([](Future<Y>) -> Z { ... })
The suggestion of adding ordering to .onAny calls carries an implication of
adding ordering to .then calls as well (since these are built on top of
.onAny):
f.then([](Future<W>) -> X { /* (1) */ });
f.then([](Future<Y>) -> Z { /* (2) */ });
Having some implicit ordering semantics on the execution of (1) and (2)
seems to be the wrong way to do ordering to me. If the caller wants
ordering, it seems better to use chaining. However, I suspect we need to
add better chaining support (something like I suggested above, but there
are subtleties that we need to think through).
On Wed, Mar 30, 2016 at 2:25 AM, Joris Van Remoortere <jo...@mesosphere.io>
wrote:
> I think the API of future intuitively suggests parts of the chain will
> execute in a specific order. The type system even enforces some of these
> IIUC.
>
> For example:
> func().then([](T) -> X {...}).repair([](Future<X>) -> X {...}).then([](X)
> -> Y {...}).onAny([](Future<Y>){...})
>
> As you can see, the .onAny(Future<Y>) can't just start executing on
> the T returned
> by func().
>
> I think it would be confusing and error prone to remember that sibling `.
> onAny` pairs could execute in arbitrary order, but members along the chain
> such as `.then` and `.repair` could not.
>
> I'm not sure the benefits of executing multiple `.onAny` calls in parallel
> in the future (if we ever need to) outweigh the inconsistency in the API
> regarding ordered execution of certain chains (At least as the API is
> currently designed).
>
> —
> *Joris Van Remoortere*
> Mesosphere
>
> On Wed, Mar 30, 2016 at 12:25 AM, Benjamin Mahler <bmah...@apache.org>
> wrote:
>
> > I don't believe we have made this assumption often, and I'm not convinced
> > it's a problem in Future.
> >
> > Why would one make such an assumption in the first place? It seems
> brittle,
> > especially when callbacks are executed in deferred contexts that Future
> is
> > unaware of. It also limits our flexibility within Future if we add these
> > ordering semantics.
> >
> > On Tue, Mar 29, 2016 at 2:41 AM, Joris Van Remoortere <
> jo...@mesosphere.io
> > >
> > wrote:
> >
> > > Rather than saying we shouldn't make this assumption (which I assume
> has
> > > already been made in places, and will likely be made again), can we fix
> > the
> > > problem?
> > > There's a `TODO` already suggesting running the callback in a separate
> > > execution environment.
> > > We could also synchronize with thread setting the future to true.
> > >
> > > I'd rather discuss and fix the problem, than try to enforce avoiding
> this
> > > case.
> > >
> > > Thoughts?
> > >
> > > —
> > > *Joris Van Remoortere*
> > > Mesosphere
> > >
> > > On Tue, Mar 29, 2016 at 2:50 AM, Jie Yu <yujie....@gmail.com> wrote:
> > >
> > > > Hi,
> > > >
> > > > While digging a bug reported in, I realized an assumption we
> shouldn't
> > > make
> > > > in our code.
> > > > https://issues.apache.org/jira/browse/MESOS-5023
> > > >
> > > > Say you have the following code:
> > > >
> > > > void some_func()
> > > > {
> > > > future
> > > > .onAny(callback_A)
> > > > .onAny(callback_B);
> > > > }
> > > >
> > > > Question: will callback_A already be executed before callback_B?
> > > >
> > > > The answer is NO. We should never assume that. Under the following
> > > > interleaving, callback_B can be invoked first:
> > > >
> > > > Thread-1 Thread-2
> > > >
> > > > onAny(callback_A) {
> > > > onAnyCallbacks.push_back(callback_A);
> > > > }
> > > > set() {
> > > > lock()
> > > > if (state ==
> > > > PENDING) {
> > > > state =
> > READY;
> > > > result =
> > true;
> > > > }
> > > > unlock();
> > > >
> > > > onAny(callback_B) {
> > > > lock()
> > > > if (state != PENDING) {
> > > > run = true
> > > > }
> > > > unlock()
> > > >
> > > > if (run) {
> > > > callback_B()
> > > > }
> > > >
> > > > if (result)
> {
> > > >
> > > > internal::run(data->onAnyCallbacks,
> > > > *this);
> > > > }
> > > >
> > > > - Jie
> > > >
> > >
> >
>
