mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sergey Urbanovich <sergey.urbanov...@gmail.com>
Subject Re: Review Request 68052: Discard the queue's promises in the destructor.
Date Fri, 27 Jul 2018 20:39:22 GMT


> On July 26, 2018, 7:07 p.m., Benjamin Mahler wrote:
> > 3rdparty/libprocess/include/process/queue.hpp
> > Line 112 (original), 112-115 (patched)
> > <https://reviews.apache.org/r/68052/diff/1/?file=2063730#file2063730line112>
> >
> >     Abandonment was added to address cases like these. The TODO was written before
we introduced abandonment:
> >     
> >     https://github.com/apache/mesos/tree/master/3rdparty/libprocess#futures-and-promises-abandoned-futures
> >     
> >     Now, the futures will be abandoned during queue destruction. However, because
abandonment was added much later, it is a sub-case of pending and has to be handled with Future::recover
only for now.
> >     
> >     For discard semantics we generally only return a discarded future in response
to a request to discard from the client. That is, if the client never requested a discard,
we generally don't surface discarded futures. Whether this is the right thing to do is up
for debate (but probably not in this patch).
> >     
> >     To maintain the status quo in this patch and leverage abandonment, we would
need to whichever clients need to handle this case to use `Future::recover` until we can better
integrate abandonment:
> >     
> >     ```
> >     Future<T> f = q.get();
> >     
> >     f
> >       .recover([](const Future<T>& f) {
> >         if (f.isAbandoned()) {
> >           return Failure(...);
> >         }
> >         return f;
> >       })
> >       .onAny([](const Future<T>&) {
> >         ...
> >       });
> >     ```

Thank you very much for the review! I've updated my another review with your suggestion and
will close this one.


- Sergey


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68052/#review206512
-----------------------------------------------------------


On July 26, 2018, 5:41 a.m., Sergey Urbanovich wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68052/
> -----------------------------------------------------------
> 
> (Updated July 26, 2018, 5:41 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jie Yu.
> 
> 
> Bugs: MESOS-5647
>     https://issues.apache.org/jira/browse/MESOS-5647
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> It fixes the case when we can get forever-pending futures after the
> destruction of the queue.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/queue.hpp c364b7dc831050bc1b23f669dc1ccc1f1255a9c8

>   3rdparty/libprocess/src/tests/queue_tests.cpp cc6b3d82ce5eb5ca16e2b9378dd5b1f28ac62fe8

> 
> 
> Diff: https://reviews.apache.org/r/68052/diff/1/
> 
> 
> Testing
> -------
> 
> 3rdparty/libprocess/src/tests/libprocess-tests --verbose --gtest_filter="QueueTest*"
--gtest_break_on_failure --gtest_repeat=1000
> 
> 
> Thanks,
> 
> Sergey Urbanovich
> 
>


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