mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Re: Review Request 57358: Implemented discard behavior in process::Queue.
Date Tue, 21 Mar 2017 21:45:20 GMT


> On March 15, 2017, 3:01 p.m., Benjamin Mahler wrote:
> > 3rdparty/libprocess/include/process/queue.hpp
> > Lines 80-86 (patched)
> > <https://reviews.apache.org/r/57358/diff/3/?file=1665030#file1665030line84>
> >
> >     This looks racy:
> >     
> >     (1) `put()` hits line 48, found the promise without a discard request.
> >     
> >     (2) Caller discards future corresponding to the promise found in (1), executes
promise->discard.
> >     
> >     (3) `put()` hits line 59 and drops the `t` item on the floor.
> >     
> >     Perhaps we could implement the expensive discard approach with a TODO to optimize
it, if it's simpler? Alternatively, we need to lock this section, but we need to make sure
we don't deadlock (will let you think more about that).

I've implemented the expensive-discard.  And I think it covers both racing and deadlock cases.

To mitigate races, all operations on the internal queues happen inside the synchronized sections.
To mitigate deadlocks, any setting/discarding of Promises happens outside the synchronized
sections.


> On March 15, 2017, 3:01 p.m., Benjamin Mahler wrote:
> > 3rdparty/libprocess/src/process.cpp
> > Lines 942-947 (patched)
> > <https://reviews.apache.org/r/57358/diff/3/?file=1665031#file1665031line942>
> >
> >     Can you comment on who is doing this discard? Or why it surfaces as discarded?
The implication in your comment seems to be that this is the case where libprocess is finalizing?
Couldn't quite follow why that's the case.

Moved to https://reviews.apache.org/r/57821/


- Joseph


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


On March 21, 2017, 2:40 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57358/
> -----------------------------------------------------------
> 
> (Updated March 21, 2017, 2:40 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Greg Mann.
> 
> 
> Bugs: MESOS-6919
>     https://issues.apache.org/jira/browse/MESOS-6919
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When a consumer calls `Queue::get` on an empty Queue, the Queue creates
> a Promise to back the Future return value.  This Promise currently
> does not have a discard handler, which may cause problems if the
> Future is chained to multiple continuations.  For example, see
> the scenario in MESOS-6919.
> 
> This commit implements an (potentially expensive) discard handler
> on the Queue's Promise.  If the Future return value is discarded,
> the Queue will remove the corresponding Promise from its internal
> queue of promises.  The operation is expensive because it needs
> to reconstruct the entire internal queue of promises.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/queue.hpp ab08e30df742412f22a06202526f8b55350ed435

>   3rdparty/libprocess/src/tests/queue_tests.cpp 95b738133fa50641f8f9b83014837d2808e0e4ff

> 
> 
> Diff: https://reviews.apache.org/r/57358/diff/4/
> 
> 
> Testing
> -------
> 
> cmake .. -DENABLE_LIBEVENT=1 -DENABLE_SSL=1
> 
> make libprocess-tests
> 
> 3rdparty/libprocess/src/tests/libprocess-tests --gtest_filter="Scheme/HTTPTest.Endpoints/0"
--gtest_repeat="`ulimit -n`"
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


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