Re: Review Request 29736: Updated the generic filter mechanism for Resources.

[Michael Park]

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

(Updated Jan. 29, 2015, 8:08 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, and 
Vinod Kone.


Repository: mesos-git


Description
---

An alternative approach for a generic filter mechanism.


Diffs (updated)
-

  include/mesos/resources.hpp 3b57568c10233a0c692787de6464f21af5eaadf4 
  src/common/resources.cpp 68f64213c47be4700bcb22cd5b76ba6ff616960d 
  src/master/hierarchical_allocator_process.hpp 
6b4489276d9f72f3bd99066bf7e48dba5ebe537e 
  src/tests/hierarchical_allocator_tests.cpp 
f44d9e98d6d9db9621f5361cddb6134f90277180 
  src/tests/resources_tests.cpp 9fd2135a7545268e2f81915ed0a019de36f3d6e1 

Diff: https://reviews.apache.org/r/29736/diff/


Testing
---

make check


Thanks,

Michael Park

Re: Review Request 29736: Updated the generic filter mechanism for Resources.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [28697, 29742, 29736]

All tests passed.

- Mesos ReviewBot


On Jan. 29, 2015, 8:08 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29736/
> ---
> 
> (Updated Jan. 29, 2015, 8:08 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, and 
> Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> An alternative approach for a generic filter mechanism.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 3b57568c10233a0c692787de6464f21af5eaadf4 
>   src/common/resources.cpp 68f64213c47be4700bcb22cd5b76ba6ff616960d 
>   src/master/hierarchical_allocator_process.hpp 
> 6b4489276d9f72f3bd99066bf7e48dba5ebe537e 
>   src/tests/hierarchical_allocator_tests.cpp 
> f44d9e98d6d9db9621f5361cddb6134f90277180 
>   src/tests/resources_tests.cpp 9fd2135a7545268e2f81915ed0a019de36f3d6e1 
> 
> Diff: https://reviews.apache.org/r/29736/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 29736: Updated the generic filter mechanism for Resources.

[Michael Park]

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

(Updated Jan. 29, 2015, 9:15 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, and 
Vinod Kone.


Repository: mesos-git


Description
---

An alternative approach for a generic filter mechanism.


Diffs (updated)
-

  include/mesos/resources.hpp 3b57568c10233a0c692787de6464f21af5eaadf4 
  src/common/resources.cpp 68f64213c47be4700bcb22cd5b76ba6ff616960d 
  src/master/hierarchical_allocator_process.hpp 
6b4489276d9f72f3bd99066bf7e48dba5ebe537e 
  src/tests/hierarchical_allocator_tests.cpp 
f44d9e98d6d9db9621f5361cddb6134f90277180 
  src/tests/resources_tests.cpp 9fd2135a7545268e2f81915ed0a019de36f3d6e1 

Diff: https://reviews.apache.org/r/29736/diff/


Testing
---

make check


Thanks,

Michael Park

Re: Review Request 28698: Modified Resources to account for reservation type.

[Michael Park]

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

(Updated Jan. 29, 2015, 9:16 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, and 
Vinod Kone.


Repository: mesos-git


Description
---

Modified Resources to account for reservation type.


Diffs (updated)
-

  include/mesos/resources.hpp 3b57568c10233a0c692787de6464f21af5eaadf4 
  src/common/resources.cpp 68f64213c47be4700bcb22cd5b76ba6ff616960d 

Diff: https://reviews.apache.org/r/28698/diff/


Testing
---

make check


Thanks,

Michael Park

Re: Review Request 29736: Updated the generic filter mechanism for Resources.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [28697, 29742, 29736]

All tests passed.

- Mesos ReviewBot


On Jan. 29, 2015, 9:15 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29736/
> ---
> 
> (Updated Jan. 29, 2015, 9:15 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, and 
> Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> An alternative approach for a generic filter mechanism.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 3b57568c10233a0c692787de6464f21af5eaadf4 
>   src/common/resources.cpp 68f64213c47be4700bcb22cd5b76ba6ff616960d 
>   src/master/hierarchical_allocator_process.hpp 
> 6b4489276d9f72f3bd99066bf7e48dba5ebe537e 
>   src/tests/hierarchical_allocator_tests.cpp 
> f44d9e98d6d9db9621f5361cddb6134f90277180 
>   src/tests/resources_tests.cpp 9fd2135a7545268e2f81915ed0a019de36f3d6e1 
> 
> Diff: https://reviews.apache.org/r/29736/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 30402: Increased the timeout for the flaky SlaveTest.CommandExecutorGracefulShutdown test.

[Alexander Rukletsov]

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

Ship it!



src/tests/slave_tests.cpp


I think we mustn't wrap `Seconds` in `Duration`.


- Alexander Rukletsov


On Jan. 29, 2015, 4:35 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30402/
> ---
> 
> (Updated Jan. 29, 2015, 4:35 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
> 
> 
> Bugs: MESOS-2228
> https://issues.apache.org/jira/browse/MESOS-2228
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> This bumps the graceful shutdown to 10 seconds to attempt to reduce the 
> flakiness of the test.
> 
> I also cleaned up some of the naming to consistently call it the "Command 
> Executor".
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp f00b6fcc2976b210c6213e52662f18f0d0342671 
>   src/tests/slave_tests.cpp aff9e255bac596a02c3d31b7c11dd5389634be20 
> 
> Diff: https://reviews.apache.org/r/30402/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 30402: Increased the timeout for the flaky SlaveTest.CommandExecutorGracefulShutdown test.

[Alexander Rukletsov]

> On Jan. 29, 2015, 10:34 a.m., Alexander Rukletsov wrote:
> > src/tests/slave_tests.cpp, line 1680
> > 
> >
> > I think we mustn't wrap `Seconds` in `Duration`.

s/mustn't/don't have to/


- Alexander


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


On Jan. 29, 2015, 4:35 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30402/
> ---
> 
> (Updated Jan. 29, 2015, 4:35 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
> 
> 
> Bugs: MESOS-2228
> https://issues.apache.org/jira/browse/MESOS-2228
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> This bumps the graceful shutdown to 10 seconds to attempt to reduce the 
> flakiness of the test.
> 
> I also cleaned up some of the naming to consistently call it the "Command 
> Executor".
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp f00b6fcc2976b210c6213e52662f18f0d0342671 
>   src/tests/slave_tests.cpp aff9e255bac596a02c3d31b7c11dd5389634be20 
> 
> Diff: https://reviews.apache.org/r/30402/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 29927: Implemented MasterAllocatorTest::StopAllocator() method.

[Alexander Rukletsov]

> On Jan. 27, 2015, 7:55 p.m., Ben Mahler wrote:
> > src/tests/master_allocator_tests.cpp, lines 80-81
> > 
> >
> > Can we be very explicit here about why we need this, and what is 
> > causing the flakiness? How does this relate to my patches to silence the 
> > mock warnings?
> 
> Alexander Rukletsov wrote:
> The ticket https://issues.apache.org/jira/browse/MESOS-1677 is a bit 
> misleading: it says the test is flaky but shows just a GMOCK warning in the 
> log. I tried to reproduce "flakiness" without stopping allocator explicitly, 
> but without success. Let's keep refactor and this issue separate. I suggest 
> we keep this patch since it restores status quo, reopen MESOS-1677 and remove 
> this code afterwards if the issue won't be confirmed. Thoughts?
> 
> Ben Mahler wrote:
> > I suggest we keep this patch since it restores status quo
> 
> Hm.. am I correct in understanding that this patch corrects MESOS-1677? 
> If not, what is the status quo that this restores? Note that my mock warning 
> fix is committed.
> 
> I'm still a bit confused as to why you want this, given the WARNING fix. 
> :)

Currently there exists `MasterAllocatorTest::StopAllocator()` that aims to fix 
MESOS-1677. During this refactor I temporary removed it (in /r/29890/) and 
restore it here. That is what I mean under the status quo.

Now given "flaky test" in MESOS-1677 means just GMOCK warning, this code is 
safe to remove. I want to separate this change from the allocator refactor.


- Alexander


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


On Jan. 28, 2015, 5:29 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29927/
> ---
> 
> (Updated Jan. 28, 2015, 5:29 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2213
> https://issues.apache.org/jira/browse/MESOS-2213
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Several tests have been reported flaky if allocation is not ceased explicitly 
> (see MESOS-1677). To stop allocation, an allocator instance should be 
> destroyed. To facilitate this, store a pointer to a MockAllocator instance in 
> the MasterAllocatorTest fixture and let a test case destroy it.
> 
> 
> Diffs
> -
> 
>   src/tests/master_allocator_tests.cpp 2430622 
> 
> Diff: https://reviews.apache.org/r/29927/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu, OS X)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 30006: Enabled concurrent downloading into the fetcher cache

[Bernd Mathiske]

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

(Updated Jan. 29, 2015, 6:08 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, and Timothy Chen.


Changes
---

Rebased.


Bugs: MESOS-2057
https://issues.apache.org/jira/browse/MESOS-2057


Repository: mesos-git


Description
---

Enables reuse of download results in the cache when multiple fetcher runs occur 
concurrently. Optimizes for minimal number of fetcher runs, between 0 and 2, 
depending on what is in the cache and what can be obtained from concurrent runs 
by simply waiting for their already initiated downloads for the same content to 
complete.


Diffs (updated)
-

  src/launcher/fetcher.cpp fed0105946da579a38357a30e7ae56e646e05b89 
  src/slave/containerizer/fetcher.hpp 1db0eaf002c8d0eaf4e0391858e61e0912b35829 
  src/slave/containerizer/fetcher.cpp d290f95251def3952c5ee34f600e1d71467f6293 

Diff: https://reviews.apache.org/r/30006/diff/


Testing
---

make check, testing whether legacy direct fetching still works, using existing 
tests. 

Testing of cache functionality coming in MESOS-2074.


Thanks,

Bernd Mathiske

Re: Review Request 30195: Remove per-flag parsing of file:// arguments

[Alexander Rukletsov]

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


I think a general solution to workaround `--whitelist` issue is to provide 
flags with dynamic content. A consumer of such flag will subscribe to updates 
and get notified when a change occurs (currently, `WhitelistWatcher` tracks the 
file changes itslef). I can imagine that in the future we want to do something 
similar for, say, ACLs. See MESOS-2090, MESOS-2092.

- Alexander Rukletsov


On Jan. 27, 2015, 11:39 p.m., Cody Maloney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30195/
> ---
> 
> (Updated Jan. 27, 2015, 11:39 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ken Sipe.
> 
> 
> Bugs: mesos-1806
> https://issues.apache.org/jira/browse/mesos-1806
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Mostly simplifies things. There are two places where there things get
> complicated
> 
> 1. --whitelist: This code wants a file to watch for changes and update
>   the whitelist from periodically. Now you have to specify that path
>   without a 'file://' prefix since the file:// prefix would read the
>   value out of a file.
> 2. --credential, --credentials: Custom parsing logic since the
>   credentials can be given in two different formats. Once the old
>   format is removed, either teaching  how to parse json
>   to protobuf generally or taking the flag as JSON then converting it
>   to protobuf later will make this clean.
> 
> 
> Diffs
> -
> 
>   CHANGELOG f515bc937441a2b4cc7e33bb857cb48a21aedea0 
>   docs/configuration.md 22f9e3db7b0e1691018de5fe3dfea3cb908de4b9 
>   src/credentials/credentials.hpp 4cdadb1b6d5a607cee8caeb38f2cbf2e3ec5da7a 
>   src/examples/load_generator_framework.cpp 
> 01a567b424140575ae0c369d026589300f173143 
>   src/master/contender.cpp c1bf82b621d6b46afe001acafe9ee53336726406 
>   src/master/detector.cpp 700eb9dde8e71648bacc00a82766634f77cf2d15 
>   src/master/flags.hpp 6c18a1af625311ef149b5877b08f63c2b12c040d 
>   src/master/main.cpp e5e76ce16646eb0244227104038efeae5f2b 
>   src/master/master.cpp ab6d1d17367f199191b7c77bccec73ec3b112d4f 
>   src/slave/flags.hpp 0f6cc41d60a2e3bc2121cc438351135541ef99ba 
>   src/slave/slave.cpp fca83b3977b95ddda30f9830da10e124b5c605e6 
>   src/tests/credentials_tests.cpp 091b545c11cfa9deabf710366e15cfc0b966de0f 
>   src/tests/master_allocator_tests.cpp 
> 2430622d09c7ef1e020e2eb8f97444e7efc7c8ea 
>   src/tests/master_contender_detector_tests.cpp 
> d847a30d21b2a2980c6b7ceb62bbf61dc77487de 
>   src/tests/master_tests.cpp 678d27f41a2f246c714c77adb132263c0c2c61ed 
>   src/tests/mesos.cpp 5ed4df530cf1bf11eec3b29542641822e0f702b2 
>   src/watcher/whitelist_watcher.cpp b6e73d7f500931dec69f115e554f9cd7cb42c3ed 
>   src/zookeeper/url.hpp 16e711c5c0bc29b1967a20f0827238f8a7b0deaf 
> 
> Diff: https://reviews.apache.org/r/30195/diff/
> 
> 
> Testing
> ---
> 
> make distcheck
>  - Tests not hitting the file:// handling which were replaced with CHECK() 
> statements.
> 
> 
> Thanks,
> 
> Cody Maloney
> 
>

Re: Review Request 30006: Enabled concurrent downloading into the fetcher cache

[Bernd Mathiske]

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

(Updated Jan. 29, 2015, 7:17 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, and Timothy Chen.


Changes
---

Fixd bug that crept in when rebasing.


Bugs: MESOS-2057
https://issues.apache.org/jira/browse/MESOS-2057


Repository: mesos-git


Description
---

Enables reuse of download results in the cache when multiple fetcher runs occur 
concurrently. Optimizes for minimal number of fetcher runs, between 0 and 2, 
depending on what is in the cache and what can be obtained from concurrent runs 
by simply waiting for their already initiated downloads for the same content to 
complete.


Diffs (updated)
-

  src/launcher/fetcher.cpp fed0105946da579a38357a30e7ae56e646e05b89 
  src/slave/containerizer/fetcher.hpp 1db0eaf002c8d0eaf4e0391858e61e0912b35829 
  src/slave/containerizer/fetcher.cpp d290f95251def3952c5ee34f600e1d71467f6293 

Diff: https://reviews.apache.org/r/30006/diff/


Testing
---

make check, testing whether legacy direct fetching still works, using existing 
tests. 

Testing of cache functionality coming in MESOS-2074.


Thanks,

Bernd Mathiske

Re: Review Request 30394: etcd api wrapper

[Alexander Rukletsov]

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



src/etcd/url.hpp


`#include `
or, what I would prefer
`#include `
and move `operator<<` definitions into `url.cpp`



src/etcd/url.hpp


Empty line, please!



src/etcd/url.hpp


You store the path with the leading `/`, could you please reflect it here?



src/etcd/url.hpp


Maybe it makes sense to kill `public:` or s/struct/class/?



src/etcd/url.hpp


s/strm/stream/ ;
Stick & to the type, please.



src/etcd/url.cpp


Shouldn't we add ``?



src/etcd/url.cpp


Kill one line, please



src/etcd/url.cpp


Though I like range-based for loops, I don't think we have agreed to start 
using them (FWIK we still support archaic GCC). I think we use `foreach` in 
such cases.



src/etcd/url.cpp


Extra newline.


- Alexander Rukletsov


On Jan. 29, 2015, 3:58 a.m., Cody Maloney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30394/
> ---
> 
> (Updated Jan. 29, 2015, 3:58 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-1806
> https://issues.apache.org/jira/browse/MESOS-1806
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> etcd api wrapper
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 07bea1fb8f0035413f2119859e16fa4f9383f68e 
>   src/etcd/etcd.hpp PRE-CREATION 
>   src/etcd/etcd.cpp PRE-CREATION 
>   src/etcd/url.hpp PRE-CREATION 
>   src/etcd/url.cpp PRE-CREATION 
>   src/master/constants.hpp c386eab3973363e64c113f7e84452456fdd3393c 
>   src/master/constants.cpp 9ee17e9ad884da98cabfdfe316cb4a831de193b3 
> 
> Diff: https://reviews.apache.org/r/30394/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Cody Maloney
> 
>

Re: Review Request 30394: etcd api wrapper

[Alexander Rukletsov]

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


Sorry for this second part, think of it as a continuation : ).


src/etcd/etcd.cpp


What is `::URL`?



src/etcd/etcd.cpp


Let's move `URL etcd_url` to a new line and allign accordingly. Also, I 
think we use camelCase everywhere, so maybe `etcdURL` is a better name.



src/etcd/etcd.cpp


Could you please point me to `http::put()` definition?



src/etcd/etcd.cpp


See above, where does `http::get()` live?


- Alexander Rukletsov


On Jan. 29, 2015, 3:58 a.m., Cody Maloney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30394/
> ---
> 
> (Updated Jan. 29, 2015, 3:58 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-1806
> https://issues.apache.org/jira/browse/MESOS-1806
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> etcd api wrapper
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 07bea1fb8f0035413f2119859e16fa4f9383f68e 
>   src/etcd/etcd.hpp PRE-CREATION 
>   src/etcd/etcd.cpp PRE-CREATION 
>   src/etcd/url.hpp PRE-CREATION 
>   src/etcd/url.cpp PRE-CREATION 
>   src/master/constants.hpp c386eab3973363e64c113f7e84452456fdd3393c 
>   src/master/constants.cpp 9ee17e9ad884da98cabfdfe316cb4a831de193b3 
> 
> Diff: https://reviews.apache.org/r/30394/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Cody Maloney
> 
>

Re: Review Request 29288: stout: Created IP address abstraction for different protocol families

[Dominic Hamon]

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

Ship it!


- Dominic Hamon


On Jan. 28, 2015, 2:05 p.m., Evelina Dumitrescu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29288/
> ---
> 
> (Updated Jan. 28, 2015, 2:05 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Dominic Hamon, Jie Yu, Joris Van 
> Remoortere, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-1919
> https://issues.apache.org/jira/browse/MESOS-1919
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Created the inner class InAddrStorage encapsulated inside the IP class.
> The class uses a union with the in_addr and in6_addr fields.
> I considered that the The MasterInfo protobuffers should have both an ipv4 
> and an ipv6 field.
> I intend to use the same Classifiers, addition, removal and update of 
> container filters, but write different encode/decode functions for IPv4/ICMP 
> and IPv6/ICMPv6 because the processing of the protocol headers differ.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 
> a0210ea6440086246aafe632f86498abbb70719a 
>   3rdparty/libprocess/3rdparty/stout/tests/net_tests.cpp 
> 425132e5d7c3770be4a5a39feea5a2f22179b871 
> 
> Diff: https://reviews.apache.org/r/29288/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Evelina Dumitrescu
> 
>

Re: Review Request 29289: libprocess: Created IP address abstraction for different protocol families

[Dominic Hamon]

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

Ship it!


Ship It!

- Dominic Hamon


On Jan. 28, 2015, 2:04 p.m., Evelina Dumitrescu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29289/
> ---
> 
> (Updated Jan. 28, 2015, 2:04 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Dominic Hamon, Jie Yu, Joris Van 
> Remoortere, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-1919
> https://issues.apache.org/jira/browse/MESOS-1919
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Created the inner class InAddrStorage encapsulated inside the IP class.
> The class uses a union with the in_addr and in6_addr fields.
> I considered that the The MasterInfo protobuffers should have both an ipv4 
> and an ipv6 field.
> I intend to use the same Classifiers, addition, removal and update of 
> container filters, but write different encode/decode functions for IPv4/ICMP 
> and IPv6/ICMPv6 because the processing of the protocol headers differ.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/node.hpp 
> 173eb8abe0d0930c92f261be7a95e07b1d4c16af 
>   3rdparty/libprocess/include/process/pid.hpp 
> 7dccf297ea473a9b19dd450f02c7ae4fd0c60a22 
>   3rdparty/libprocess/include/process/socket.hpp 
> ddb9e365fc1e65a568bdac4973964df1ab8cc05e 
>   3rdparty/libprocess/src/http.cpp 869b205656fb73edb9f02a1856d10f79ed1349b4 
>   3rdparty/libprocess/src/pid.cpp 085e0b9abe9a9f33a63247915835decbf942274d 
>   3rdparty/libprocess/src/process.cpp 
> 67b6b3b9c13d95fa1a24b48a12c5c831c7f249bf 
>   3rdparty/libprocess/src/socket.cpp 4b0f6bec8051f938812dbc90a7312e4082ea203f 
> 
> Diff: https://reviews.apache.org/r/29289/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Evelina Dumitrescu
> 
>

Re: Review Request 29289: libprocess: Created IP address abstraction for different protocol families

[Dominic Hamon]

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



3rdparty/libprocess/include/process/pid.hpp


line is too long. i can fix before commit.


- Dominic Hamon


On Jan. 28, 2015, 2:04 p.m., Evelina Dumitrescu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29289/
> ---
> 
> (Updated Jan. 28, 2015, 2:04 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Dominic Hamon, Jie Yu, Joris Van 
> Remoortere, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-1919
> https://issues.apache.org/jira/browse/MESOS-1919
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Created the inner class InAddrStorage encapsulated inside the IP class.
> The class uses a union with the in_addr and in6_addr fields.
> I considered that the The MasterInfo protobuffers should have both an ipv4 
> and an ipv6 field.
> I intend to use the same Classifiers, addition, removal and update of 
> container filters, but write different encode/decode functions for IPv4/ICMP 
> and IPv6/ICMPv6 because the processing of the protocol headers differ.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/node.hpp 
> 173eb8abe0d0930c92f261be7a95e07b1d4c16af 
>   3rdparty/libprocess/include/process/pid.hpp 
> 7dccf297ea473a9b19dd450f02c7ae4fd0c60a22 
>   3rdparty/libprocess/include/process/socket.hpp 
> ddb9e365fc1e65a568bdac4973964df1ab8cc05e 
>   3rdparty/libprocess/src/http.cpp 869b205656fb73edb9f02a1856d10f79ed1349b4 
>   3rdparty/libprocess/src/pid.cpp 085e0b9abe9a9f33a63247915835decbf942274d 
>   3rdparty/libprocess/src/process.cpp 
> 67b6b3b9c13d95fa1a24b48a12c5c831c7f249bf 
>   3rdparty/libprocess/src/socket.cpp 4b0f6bec8051f938812dbc90a7312e4082ea203f 
> 
> Diff: https://reviews.apache.org/r/29289/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Evelina Dumitrescu
> 
>

Re: Review Request 29290: Created IP address abstraction for different protocol families

[Dominic Hamon]

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

Ship it!


Ship It!

- Dominic Hamon


On Jan. 28, 2015, 2:04 p.m., Evelina Dumitrescu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29290/
> ---
> 
> (Updated Jan. 28, 2015, 2:04 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Dominic Hamon, Jie Yu, Joris Van 
> Remoortere, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-1919
> https://issues.apache.org/jira/browse/MESOS-1919
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Created the inner class InAddrStorage encapsulated inside the IP class.
> The class uses a union with the in_addr and in6_addr fields.
> I considered that the The MasterInfo protobuffers should have both an ipv4 
> and an ipv6 field.
> I intend to use the same Classifiers, addition, removal and update of 
> container filters, but write different encode/decode functions for IPv4/ICMP 
> and IPv6/ICMPv6 because the processing of the protocol headers differ.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp a5793918a2c1bc1c13432653c4219de7283fefd1 
>   src/common/protobuf_utils.cpp c4b53a81c0426d361363c12920d67c261e381553 
>   src/linux/routing/diagnosis/diagnosis.cpp 
> 136ba379efbbe4200c0e9f794a2966ffee174fff 
>   src/linux/routing/filter/icmp.cpp 86bd67b71a590b88344adbe10fd1b44ea1b5148d 
>   src/linux/routing/filter/ip.cpp 922a732c3543a072674208b123fdfadbef2b15f2 
>   src/linux/routing/route.cpp b0eda7b662eca0ba1357e558f6f6b1474067b06d 
>   src/master/http.cpp 3981b18cb82d3b8bd974b80d27f14c304898a43c 
>   src/master/master.cpp 54f26900ac8c63e79a1f89562a988c9a2567d209 
>   src/sched/sched.cpp a822c002781b35872d3c477366775a3705343cd2 
>   src/scheduler/scheduler.cpp 44713cae36a83081c9a665d2eb73f9dbec2d4268 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> b484bbf34060ccc9debd48a3e840c03c8f95de09 
>   src/slave/slave.cpp a8b262174ab5c9a524db8318d3d1438cd75a702b 
>   src/tests/master_contender_detector_tests.cpp 
> d847a30d21b2a2980c6b7ceb62bbf61dc77487de 
>   src/tests/master_tests.cpp 678d27f41a2f246c714c77adb132263c0c2c61ed 
>   src/tests/port_mapping_tests.cpp 18d58ff310d189f7461eead445e186172cbcd101 
>   src/tests/routing_tests.cpp 962cff27652d8589dc0f7d3b1ecc6d81ef9d1f23 
> 
> Diff: https://reviews.apache.org/r/29290/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Evelina Dumitrescu
> 
>

Re: Review Request 30395: etcd master contender + detector

[Alexander Rukletsov]

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



src/master/contender.hpp


Let's expand the comment with etcd examples.



src/master/contender.cpp


You can safely omit trailing underscores.


- Alexander Rukletsov


On Jan. 29, 2015, 3:58 a.m., Cody Maloney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30395/
> ---
> 
> (Updated Jan. 29, 2015, 3:58 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-1806
> https://issues.apache.org/jira/browse/MESOS-1806
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> etcd master contender + detector
> 
> 
> Diffs
> -
> 
>   src/etcd/etcd.cpp PRE-CREATION 
>   src/master/contender.hpp 76beb5f973ae02507849233b6d73c43293669489 
>   src/master/contender.cpp c1bf82b621d6b46afe001acafe9ee53336726406 
>   src/master/detector.hpp 2905e2b3536e14e9df3570da172603e6ed81aae1 
>   src/master/detector.cpp 700eb9dde8e71648bacc00a82766634f77cf2d15 
> 
> Diff: https://reviews.apache.org/r/30395/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Cody Maloney
> 
>

Re: Review Request 30396: Add --etcd to match --zk.

[Alexander Rukletsov]

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


What if a user specifies both `--zk` and `--etcd`? Do we want to track it and 
throw at least a warning?

- Alexander Rukletsov


On Jan. 29, 2015, 3:58 a.m., Cody Maloney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30396/
> ---
> 
> (Updated Jan. 29, 2015, 3:58 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-1806
> https://issues.apache.org/jira/browse/MESOS-1806
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Eventually these will be replaced with --masters which handles both
> calculating the master group as well as leader election. Currently
> etcd can only be used for master election.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 22f9e3db7b0e1691018de5fe3dfea3cb908de4b9 
>   src/master/main.cpp e5e76ce16646eb0244227104038efeae5f2b 
>   src/slave/main.cpp 42e46c58ab4b16033ac6a73d127968d0803490aa 
> 
> Diff: https://reviews.apache.org/r/30396/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Cody Maloney
> 
>

Re: Review Request 30402: Increased the timeout for the flaky SlaveTest.CommandExecutorGracefulShutdown test.

[Ben Mahler]

> On Jan. 29, 2015, 10:34 a.m., Alexander Rukletsov wrote:
> > src/tests/slave_tests.cpp, line 1680
> > 
> >
> > I think we mustn't wrap `Seconds` in `Duration`.
> 
> Alexander Rukletsov wrote:
> s/mustn't/don't have to/

I thought so too, but it doesn't compile here as max(Duration, Seconds), 
unfortunately :)


- Ben


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


On Jan. 29, 2015, 4:35 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30402/
> ---
> 
> (Updated Jan. 29, 2015, 4:35 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
> 
> 
> Bugs: MESOS-2228
> https://issues.apache.org/jira/browse/MESOS-2228
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> This bumps the graceful shutdown to 10 seconds to attempt to reduce the 
> flakiness of the test.
> 
> I also cleaned up some of the naming to consistently call it the "Command 
> Executor".
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp f00b6fcc2976b210c6213e52662f18f0d0342671 
>   src/tests/slave_tests.cpp aff9e255bac596a02c3d31b7c11dd5389634be20 
> 
> Diff: https://reviews.apache.org/r/30402/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 30396: Add --etcd to match --zk.

[Cody Maloney]

> On Jan. 29, 2015, 5:55 p.m., Alexander Rukletsov wrote:
> > What if a user specifies both `--zk` and `--etcd`? Do we want to track it 
> > and throw at least a warning?

There isn't a good way to do this using  without rewriting a 
decent chunk of it. I could make the flags have different backing member 
variables and write the logic outside of flags / in main, but that just adds a 
lot of code + easy to get wrong / hard to test conditionals without any real 
value.

If someone specifies both on the command line, the last one to be specified 
wins. Simple enough for an admin to figure out. Note they could also have 
MESOS_ZK and MESOS_ETCD specified and we wouldn't warn/error.


- Cody


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


On Jan. 29, 2015, 3:58 a.m., Cody Maloney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30396/
> ---
> 
> (Updated Jan. 29, 2015, 3:58 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-1806
> https://issues.apache.org/jira/browse/MESOS-1806
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Eventually these will be replaced with --masters which handles both
> calculating the master group as well as leader election. Currently
> etcd can only be used for master election.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 22f9e3db7b0e1691018de5fe3dfea3cb908de4b9 
>   src/master/main.cpp e5e76ce16646eb0244227104038efeae5f2b 
>   src/slave/main.cpp 42e46c58ab4b16033ac6a73d127968d0803490aa 
> 
> Diff: https://reviews.apache.org/r/30396/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Cody Maloney
> 
>

Re: Review Request 30402: Increased the timeout for the flaky SlaveTest.CommandExecutorGracefulShutdown test.

[Vinod Kone]

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

Ship it!



src/tests/slave_tests.cpp


Can you add a comment here on why you do a max() here for future readers?


- Vinod Kone


On Jan. 29, 2015, 4:35 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30402/
> ---
> 
> (Updated Jan. 29, 2015, 4:35 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
> 
> 
> Bugs: MESOS-2228
> https://issues.apache.org/jira/browse/MESOS-2228
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> This bumps the graceful shutdown to 10 seconds to attempt to reduce the 
> flakiness of the test.
> 
> I also cleaned up some of the naming to consistently call it the "Command 
> Executor".
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp f00b6fcc2976b210c6213e52662f18f0d0342671 
>   src/tests/slave_tests.cpp aff9e255bac596a02c3d31b7c11dd5389634be20 
> 
> Diff: https://reviews.apache.org/r/30402/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 28809: Started to maintain and checkpoint persisted resource in slave.

[Jie Yu]

> On Jan. 8, 2015, 12:28 a.m., Ben Mahler wrote:
> > src/slave/slave.cpp, lines 1794-1815
> > 
> >
> > What about adding a method in slave/state.hpp for checkpointing 
> > Resources?
> > 
> > ```
> > Try checkpoint(
> > const std::string& path,
> > const Resources& resources);
> > ```

This is done in https://reviews.apache.org/r/29918/


> On Jan. 8, 2015, 12:28 a.m., Ben Mahler wrote:
> > src/slave/slave.cpp, lines 1817-1826
> > 
> >
> > Maybe a little NOTE here that we assume that messages are ordered for 
> > the releasing to be correct? And that ordering is technically not 
> > guaranteed, maybe pointing to the relevant tickets?
> > 
> > Just want to make sure we know what needs to be done to prevent this 
> > from ever biting us.

Added a TODO here explaining the details about the ordering issue.


> On Jan. 8, 2015, 12:28 a.m., Ben Mahler wrote:
> > src/slave/slave.cpp, line 1830
> > 
> >
> > "Updated persistent resources to Y"?
> > 
> > Or
> > 
> > "Updated persistent resources from X to Y"?

Fixed.


> On Jan. 8, 2015, 12:28 a.m., Ben Mahler wrote:
> > src/slave/slave.cpp, line 3700
> > 
> >
> > Shouldn't this CHECK exist in updateResources instead of here? Or in 
> > both places?
> > 
> > Otherwise the slave will create a situation where it will fail CHECKs 
> > when it next recovers, is there something else I'm missing?

Doesn't apply any more. We want to get dynamic reservation done as well.


> On Jan. 8, 2015, 12:28 a.m., Ben Mahler wrote:
> > src/slave/slave.cpp, lines 1789-1791
> > 
> >
> > Just so I understand, we won't be deleting anything, right? We'll leave 
> > volumes "dangling" only when the master fails over during the issue you 
> > described here. In that case, the master also thinks that there are no 
> > persisted resources on the slave.
> > 
> > And we won't delete the unknown volumes, we'll leave them dangling on 
> > the filesystem, right?. Can you please file a ticket to capture this issue 
> > and link it here and in the epic? Don't want to leave it unfixed.
> > 
> > If the master is up, it should re-send the persistent resources at 
> > which point the slave gets them back during re-registration, right?. During 
> > registration however, the problem still exists, right?
> > 
> > Let's document the details here and/or in a ticket!

This TODO has been resolved by using an all-or-nothing atomic checkpoint 
function.


- Jie


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


On Dec. 8, 2014, 10:19 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28809/
> ---
> 
> (Updated Dec. 8, 2014, 10:19 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-2031
> https://issues.apache.org/jira/browse/MESOS-2031
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Started to maintain and checkpoint persisted resource in slave. That includes:
> 1) responds to update resources message
> 2) checkpoint resources
> 3) recover checkpointed resources
> 4) send checkpointed resources during register/reregister
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 70bd8c1fde4ea09fa54c76aa93424a1adb0309f6 
>   src/slave/slave.cpp 9ac64589c353b2f17f538db7de01faa55b2369b9 
> 
> Diff: https://reviews.apache.org/r/28809/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 30386: Added support for CREATE operation in master.

[Ben Mahler]

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



src/master/master.cpp


Nice :)

Do you need to pass the slave here? Can you remove it for now?



src/master/master.cpp


Since we have to still check uniqueness across 'Resources', could we handle 
uniqueness through the ResourceUsage checker?

To be specific, having 2 IDs inside a task / executor will mean that it's 
using more resources than we've offered.

For create operations, we are interested in looking at duplicates within 
what is being created and compared to what's in the slave.

If there's something I'm missing, we could still keep this but split it 
into two validators:

(1) PersistenceIDValidator -> For tasks, makes sure no duplicates.
(2) PersistenceIDCreationValidator -> For CREATE, makes sure it hasn't 
already been created.

For launch, we use (1), and for create we use (1) and (2). Thoughts?



src/master/master.cpp


Any reason these checks captured in Validators?



src/master/master.cpp


What is this TODO? How could one create a volume that's already reserved?


- Ben Mahler


On Jan. 29, 2015, 12:12 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30386/
> ---
> 
> (Updated Jan. 29, 2015, 12:12 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Michael Park, and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Added support for CREATE operation in master.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 1d342e56116ad63aade43484b6899ce26f25abfd 
>   src/master/master.cpp 54f26900ac8c63e79a1f89562a988c9a2567d209 
> 
> Diff: https://reviews.apache.org/r/30386/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 29927: Implemented MasterAllocatorTest::StopAllocator() method.

[Ben Mahler]

> On Jan. 27, 2015, 7:55 p.m., Ben Mahler wrote:
> > src/tests/master_allocator_tests.cpp, lines 80-81
> > 
> >
> > Can we be very explicit here about why we need this, and what is 
> > causing the flakiness? How does this relate to my patches to silence the 
> > mock warnings?
> 
> Alexander Rukletsov wrote:
> The ticket https://issues.apache.org/jira/browse/MESOS-1677 is a bit 
> misleading: it says the test is flaky but shows just a GMOCK warning in the 
> log. I tried to reproduce "flakiness" without stopping allocator explicitly, 
> but without success. Let's keep refactor and this issue separate. I suggest 
> we keep this patch since it restores status quo, reopen MESOS-1677 and remove 
> this code afterwards if the issue won't be confirmed. Thoughts?
> 
> Ben Mahler wrote:
> > I suggest we keep this patch since it restores status quo
> 
> Hm.. am I correct in understanding that this patch corrects MESOS-1677? 
> If not, what is the status quo that this restores? Note that my mock warning 
> fix is committed.
> 
> I'm still a bit confused as to why you want this, given the WARNING fix. 
> :)
> 
> Alexander Rukletsov wrote:
> Currently there exists `MasterAllocatorTest::StopAllocator()` that aims 
> to fix MESOS-1677. During this refactor I temporary removed it (in /r/29890/) 
> and restore it here. That is what I mean under the status quo.
> 
> Now given "flaky test" in MESOS-1677 means just GMOCK warning, this code 
> is safe to remove. I want to separate this change from the allocator refactor.

Ok great! Want to go ahead and discard this patch then?


- Ben


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


On Jan. 28, 2015, 5:29 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29927/
> ---
> 
> (Updated Jan. 28, 2015, 5:29 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2213
> https://issues.apache.org/jira/browse/MESOS-2213
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Several tests have been reported flaky if allocation is not ceased explicitly 
> (see MESOS-1677). To stop allocation, an allocator instance should be 
> destroyed. To facilitate this, store a pointer to a MockAllocator instance in 
> the MasterAllocatorTest fixture and let a test case destroy it.
> 
> 
> Diffs
> -
> 
>   src/tests/master_allocator_tests.cpp 2430622 
> 
> Diff: https://reviews.apache.org/r/29927/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu, OS X)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 30396: Add --etcd to match --zk.

[Benjamin Mahler]

That sounds like it might lead to surprise, typically we handle these cross
flag interactions via manual checks (e.g. in master/main.cpp). Could you do
that here?

On Thu, Jan 29, 2015 at 10:25 AM, Cody Maloney  wrote:

>
>
> > On Jan. 29, 2015, 5:55 p.m., Alexander Rukletsov wrote:
> > > What if a user specifies both `--zk` and `--etcd`? Do we want to track
> it and throw at least a warning?
>
> There isn't a good way to do this using  without rewriting a
> decent chunk of it. I could make the flags have different backing member
> variables and write the logic outside of flags / in main, but that just
> adds a lot of code + easy to get wrong / hard to test conditionals without
> any real value.
>
> If someone specifies both on the command line, the last one to be
> specified wins. Simple enough for an admin to figure out. Note they could
> also have MESOS_ZK and MESOS_ETCD specified and we wouldn't warn/error.
>
>
> - Cody
>
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30396/#review70236
> ---
>
>
> On Jan. 29, 2015, 3:58 a.m., Cody Maloney wrote:
> >
> > ---
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/30396/
> > ---
> >
> > (Updated Jan. 29, 2015, 3:58 a.m.)
> >
> >
> > Review request for mesos and Benjamin Hindman.
> >
> >
> > Bugs: MESOS-1806
> > https://issues.apache.org/jira/browse/MESOS-1806
> >
> >
> > Repository: mesos-git
> >
> >
> > Description
> > ---
> >
> > Eventually these will be replaced with --masters which handles both
> > calculating the master group as well as leader election. Currently
> > etcd can only be used for master election.
> >
> >
> > Diffs
> > -
> >
> >   docs/configuration.md 22f9e3db7b0e1691018de5fe3dfea3cb908de4b9
> >   src/master/main.cpp e5e76ce16646eb0244227104038efeae5f2b
> >   src/slave/main.cpp 42e46c58ab4b16033ac6a73d127968d0803490aa
> >
> > Diff: https://reviews.apache.org/r/30396/diff/
> >
> >
> > Testing
> > ---
> >
> >
> > Thanks,
> >
> > Cody Maloney
> >
> >
>
>

Re: Review Request 28809: Started to maintain and checkpoint persisted resource in slave.

[Jie Yu]

> On Dec. 22, 2014, 5:43 a.m., Adam B wrote:
> > src/slave/slave.cpp, line 1787
> > 
> >
> > What is the source of truth for persistent resources? The checkpoint or 
> > the master? What if a framework is trying to launch a task on a slave with 
> > a new persistent disk resource, while at the same time, the slave is 
> > restarted with new persistent disks added by the slave operator? Assume we 
> > can update slave resources without invalidating the SlaveID, and that the 
> > master has already started processing the launchTask when the slave tries 
> > to reregister.
> > I'm guessing the slave would reject the UpdateResources call until the 
> > slave has successfully re-registered with the master, so the master would 
> > have the updated persistentResource, which it would then update with the 
> > newly launched task's persistent disk.

Good question! The source of truth is in master in the current design. There 
are several resaons for this choice. But I guess the main motivation is that 
master is in the central place of the cluster (in the middle of framework and 
slaves). Keeping source of truch in master makes it easy for us to maintain a 
consistent state for the cluster.

Since master can fail over, slave needs to checkpoint persistent volumes and 
dynamic reservations and report them back to master when master fails over. 
Also, a newly registered slave needs to tell master about it's persistent 
volumes and dynamic reservations as well since master has no idea about a new 
slave.

Now, If an operator wants to add a new persistent volume, he/she has to start 
the slave as a new slave (new SlaveID). This is the similar to changing 
SlaveInfo right now. Once we have a way to update SlaveInfo (MESOS-1739), we 
could use the similar mechanism to update persistent volumes and dynamic 
reservations.


- Jie


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


On Dec. 8, 2014, 10:19 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28809/
> ---
> 
> (Updated Dec. 8, 2014, 10:19 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-2031
> https://issues.apache.org/jira/browse/MESOS-2031
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Started to maintain and checkpoint persisted resource in slave. That includes:
> 1) responds to update resources message
> 2) checkpoint resources
> 3) recover checkpointed resources
> 4) send checkpointed resources during register/reregister
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 70bd8c1fde4ea09fa54c76aa93424a1adb0309f6 
>   src/slave/slave.cpp 9ac64589c353b2f17f538db7de01faa55b2369b9 
> 
> Diff: https://reviews.apache.org/r/28809/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 28809: Started to maintain and checkpoint persisted resource in slave.

[Jie Yu]

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

(Updated Jan. 29, 2015, 7:02 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Review comments. Rebased.


Bugs: MESOS-2031
https://issues.apache.org/jira/browse/MESOS-2031


Repository: mesos-git


Description
---

Started to maintain and checkpoint persisted resource in slave. That includes:
1) responds to update resources message
2) checkpoint resources
3) recover checkpointed resources
4) send checkpointed resources during register/reregister


Diffs (updated)
-

  src/slave/slave.hpp 70bd8c1fde4ea09fa54c76aa93424a1adb0309f6 
  src/slave/slave.cpp a8b262174ab5c9a524db8318d3d1438cd75a702b 

Diff: https://reviews.apache.org/r/28809/diff/


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 30283: Separated offer operations in Master::_accept

[Jie Yu]

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

(Updated Jan. 29, 2015, 7:03 p.m.)


Review request for mesos, Ben Mahler and Michael Park.


Changes
---

Rebased. NNFR


Repository: mesos-git


Description
---

To avoid potential conflicts.


Diffs (updated)
-

  src/master/master.cpp 54f26900ac8c63e79a1f89562a988c9a2567d209 

Diff: https://reviews.apache.org/r/30283/diff/


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 30298: Refactored resources validations into separate validators.

[Jie Yu]

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

(Updated Jan. 29, 2015, 7:03 p.m.)


Review request for mesos, Ben Mahler and Michael Park.


Changes
---

Rebased. NNFR


Repository: mesos-git


Description
---

Pull resources validation info separate validators so that we can leverage them 
to validate resources in other Offer Operations as well.


Diffs (updated)
-

  src/master/master.cpp 54f26900ac8c63e79a1f89562a988c9a2567d209 

Diff: https://reviews.apache.org/r/30298/diff/


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 30402: Increased the timeout for the flaky SlaveTest.CommandExecutorGracefulShutdown test.

[Ben Mahler]

> On Jan. 29, 2015, 6:36 p.m., Vinod Kone wrote:
> > src/tests/slave_tests.cpp, line 1676
> > 
> >
> > Can you add a comment here on why you do a max() here for future 
> > readers?

Thanks!


- Ben


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


On Jan. 29, 2015, 4:35 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30402/
> ---
> 
> (Updated Jan. 29, 2015, 4:35 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
> 
> 
> Bugs: MESOS-2228
> https://issues.apache.org/jira/browse/MESOS-2228
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> This bumps the graceful shutdown to 10 seconds to attempt to reduce the 
> flakiness of the test.
> 
> I also cleaned up some of the naming to consistently call it the "Command 
> Executor".
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp f00b6fcc2976b210c6213e52662f18f0d0342671 
>   src/tests/slave_tests.cpp aff9e255bac596a02c3d31b7c11dd5389634be20 
> 
> Diff: https://reviews.apache.org/r/30402/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 28809: Started to maintain and checkpoint persisted resource in slave.

[Jie Yu]

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

(Updated Jan. 29, 2015, 7:04 p.m.)


Review request for mesos and Ben Mahler.


Bugs: MESOS-2031
https://issues.apache.org/jira/browse/MESOS-2031


Repository: mesos-git


Description
---

Started to maintain and checkpoint persisted resource in slave. That includes:
1) responds to update resources message
2) checkpoint resources
3) recover checkpointed resources
4) send checkpointed resources during register/reregister


Diffs
-

  src/slave/slave.hpp 70bd8c1fde4ea09fa54c76aa93424a1adb0309f6 
  src/slave/slave.cpp a8b262174ab5c9a524db8318d3d1438cd75a702b 

Diff: https://reviews.apache.org/r/28809/diff/


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 28781: Maintained persisted resources in master memory.

[Jie Yu]

> On Jan. 29, 2015, 1:53 a.m., Ben Mahler wrote:
> > src/master/master.hpp, lines 762-764
> > 
> >
> > Any reason why you're not doing this in the initializer list via the 
> > Resources(vector) constructor?

Because at that time, the Resources(vector) has been added. I changed it.


- Jie


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


On Jan. 29, 2015, 12:11 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28781/
> ---
> 
> (Updated Jan. 29, 2015, 12:11 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-2030
> https://issues.apache.org/jira/browse/MESOS-2030
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Maintained persisted resources in master memory.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 1d342e56116ad63aade43484b6899ce26f25abfd 
>   src/master/master.cpp 54f26900ac8c63e79a1f89562a988c9a2567d209 
> 
> Diff: https://reviews.apache.org/r/28781/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 28781: Maintained persisted resources in master memory.

[Jie Yu]

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

(Updated Jan. 29, 2015, 7:16 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

BenM's comments.


Bugs: MESOS-2030
https://issues.apache.org/jira/browse/MESOS-2030


Repository: mesos-git


Description
---

Maintained persisted resources in master memory.


Diffs (updated)
-

  src/master/master.hpp 1d342e56116ad63aade43484b6899ce26f25abfd 
  src/master/master.cpp 54f26900ac8c63e79a1f89562a988c9a2567d209 

Diff: https://reviews.apache.org/r/28781/diff/


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 30283: Separated offer operations in Master::_accept

[Michael Park]

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



src/master/master.cpp


Just wondering, how come the braces were removed for the `default` case?


- Michael Park


On Jan. 29, 2015, 7:03 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30283/
> ---
> 
> (Updated Jan. 29, 2015, 7:03 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Michael Park.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> To avoid potential conflicts.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 54f26900ac8c63e79a1f89562a988c9a2567d209 
> 
> Diff: https://reviews.apache.org/r/30283/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 30295: Removed mesos::internal namespace.

[Kapil Arya]

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

(Updated Jan. 29, 2015, 2:32 p.m.)


Review request for mesos, Alexander Rukletsov, Jie Yu, Niklas Nielsen, Till 
Toenshoff, and Timothy Chen.


Changes
---

Added Jie to the review.


Bugs: MESOS-2272
https://issues.apache.org/jira/browse/MESOS-2272


Repository: mesos-git


Description
---

[1/2]: Remove "internal" namespace.

The namespace is redundant as it encapsulates a different namespace
in most cases.


Diffs
-

  include/mesos/executor.hpp f3cd3ccd505ed4b308a1a42b238fc21fe45cc3b3 
  include/mesos/scheduler.hpp 9d6a20577377b1b4ff14e0106796104b85bedd7a 
  include/mesos/values.hpp c61f9e87f1e240c71571793d24751fbe53ed45d2 
  src/authentication/authenticatee.hpp 361083b5e1ca5ab1a594b3108fb768626313a725 
  src/authentication/authenticator.hpp 460494a49ad7c71803507afe62005280d5929d41 
  src/authentication/cram_md5/authenticatee.hpp 
5d35598f75f4d9a8c185cfd387e1085b3bf15395 
  src/authentication/cram_md5/authenticator.hpp 
d739a02d755c2729248291db8c274ad7e7dd4c6d 
  src/authentication/cram_md5/auxprop.hpp 
b894386b946bcbc91a6bf292247c8ed815214d38 
  src/authentication/cram_md5/auxprop.cpp 
cf503a25a37585aa40a7afb6ce41e0f11793afa7 
  src/authorizer/authorizer.hpp c039d9412780aa199db169b31991bf9f45b07d0f 
  src/authorizer/authorizer.cpp 21e97e315478a4ca9442af83732665f85eb2f8fc 
  src/cli/execute.cpp 77deec91177c90a8a20ccd4fe063b82ca08cefbd 
  src/cli/resolve.cpp a99b6094dffc9f7aa44bcf63ad40121e1abb120b 
  src/common/attributes.hpp 0a043d5b5dca804c6dd215cabd2704f24df71a33 
  src/common/attributes.cpp aab114e1a5932e3f218b850e1afc7f2ef0f10e21 
  src/common/build.hpp 7d61a06da82da5bcee7457e9070ba0f9567f8692 
  src/common/build.cpp 9bef16e8d0c9908560ac7d8d18b3f5e6fc9f34e2 
  src/common/date_utils.hpp 5f14a069d9bab8b8e5e4353dd9b6b3ce35da957a 
  src/common/date_utils.cpp c5268bc7f7177efca27c64fbf4792d18d89d4570 
  src/common/factory.hpp bdddcf872eae1a4ed78ca8b35864ee59bbaecb65 
  src/common/http.hpp afce7fea334c7bfa57efc48c413bf906a2ebde32 
  src/common/http.cpp 915cba10725685da55a53b249b8b676b5e229d86 
  src/common/lock.hpp 988dff524e3d9f6c0bafa3398f6c3c258829cfe3 
  src/common/lock.cpp bb8ea3ada6b710357e6872959868d2d8a2035371 
  src/common/protobuf_utils.hpp a5793918a2c1bc1c13432653c4219de7283fefd1 
  src/common/protobuf_utils.cpp c4b53a81c0426d361363c12920d67c261e381553 
  src/common/resources.cpp 214e441fb86aa0c094c28ed5801089051468137b 
  src/common/type_utils.cpp fcc9eb0e27a9cbf53f7923f0c051a6578527ec37 
  src/common/values.cpp 597c4520ebc8d31a606f433593ae41c003683eb3 
  src/credentials/credentials.hpp 4cdadb1b6d5a607cee8caeb38f2cbf2e3ec5da7a 
  src/docker/docker.cpp 3a485a28d4b1ac3fcbce2e89c7d52b097886921a 
  src/examples/balloon_framework.cpp c2337ba7ae00e5c59dfb3734d2f314c89687c4ad 
  src/examples/load_generator_framework.cpp 
01a567b424140575ae0c369d026589300f173143 
  src/examples/low_level_scheduler_libprocess.cpp 
a0ec131fff1773280b17e97cb78a3da88afe6f7d 
  src/examples/low_level_scheduler_pthread.cpp 
f05489ae10032dcf177ce149938cd94186355351 
  src/examples/test_authentication_modules.cpp 
6d32573d4e7817ddb62f3eb7aca2cf3f06d27c65 
  src/examples/test_framework.cpp e5ec3b9f75446980ce9235209b8b74128e3e5e39 
  src/examples/test_hook_module.cpp 04fd43eb3eacae0d850dd7f4e191116d20620f10 
  src/examples/test_isolator_module.cpp 
dc107a1315271b0275c60819c4b5aaa1f527e73c 
  src/exec/exec.cpp aada24664dba9060a92230e25689c89852585443 
  src/files/files.hpp 818087b13cc787d0bd3186bb3e8a069751629bf9 
  src/files/files.cpp 7b1024059ab00c00a98bb1cdb9c281ed2a89cf06 
  src/health-check/main.cpp a4ce742ab8deff1ebd99359112670493fdaeeac3 
  src/hook/hook.hpp 9401e03192e53019b80e9a29a322195250cc019b 
  src/hook/manager.hpp aadc17aa8f56f8f6ae23ed427cd344f07f7db0c7 
  src/hook/manager.cpp 6bf1ef7bdddee78a0fab1a942aa89edfd9eb719e 
  src/java/jni/org_apache_mesos_Log.cpp 
1e1d821dcbecdc1e1e5cdf38e6c6aa61ab501075 
  src/java/jni/org_apache_mesos_state_AbstractState.cpp 
1accc8a498a68b7cfd9e39dc1f3ce01c8bfd219f 
  src/java/jni/org_apache_mesos_state_LevelDBState.cpp 
30f63092d3839485da2a8d818febfa85c81df49f 
  src/java/jni/org_apache_mesos_state_LogState.cpp 
6382b9c3ee0ed2da30be39fda7e1555716dfe394 
  src/java/jni/org_apache_mesos_state_Variable.cpp 
4d840ce0d3c4add7a0ec80229bae2188dc17ad32 
  src/java/jni/org_apache_mesos_state_ZooKeeperState.cpp 
c40e6858bc7534443e01ad189173e8cdb56b0fba 
  src/jvm/jvm.cpp d33a655dd46dc52f23905eb53de6530fa7b66a6c 
  src/launcher/executor.cpp d527fec3b9e658099571f808aa824bd6e17a6e0d 
  src/launcher/fetcher.cpp fed0105946da579a38357a30e7ae56e646e05b89 
  src/linux/cgroups.cpp 0b136e17907123e814e6011523d8e66a4d66cf98 
  src/linux/fs.hpp ac8b5f416dae0a9388a3839b6f078abdcd42edae 
  src/linux/fs.cpp b01d14c3a1b296566b2b17a7f540097bd7cc53dd 
  src/local/flags.hpp 5

Re: Review Request 28781: Maintained persisted resources in master memory.

[Ben Mahler]

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

Ship it!


Ship It!

- Ben Mahler


On Jan. 29, 2015, 7:16 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28781/
> ---
> 
> (Updated Jan. 29, 2015, 7:16 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-2030
> https://issues.apache.org/jira/browse/MESOS-2030
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Maintained persisted resources in master memory.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 1d342e56116ad63aade43484b6899ce26f25abfd 
>   src/master/master.cpp 54f26900ac8c63e79a1f89562a988c9a2567d209 
> 
> Diff: https://reviews.apache.org/r/28781/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 30386: Added support for CREATE operation in master.

[Michael Park]

> On Jan. 29, 2015, 6:49 p.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 2929-2931
> > 
> >
> > What is this TODO? How could one create a volume that's already 
> > reserved?

This is if we're creating a volume with dynamically reserved regular disk. We 
want to transition from `[(disk, 2, role, DYNAMIC)]` to `[(disk, 2, role, 
DYNAMIC, persistence=id, ...)]` not to `[(disk, 2, role, DYNAMIC), (disk, 2, 
role, DYNAMIC, persistence=id, ...)]`.


- Michael


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


On Jan. 29, 2015, 12:12 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30386/
> ---
> 
> (Updated Jan. 29, 2015, 12:12 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Michael Park, and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Added support for CREATE operation in master.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 1d342e56116ad63aade43484b6899ce26f25abfd 
>   src/master/master.cpp 54f26900ac8c63e79a1f89562a988c9a2567d209 
> 
> Diff: https://reviews.apache.org/r/30386/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 30283: Separated offer operations in Master::_accept

[Jie Yu]

> On Jan. 29, 2015, 7:17 p.m., Michael Park wrote:
> > src/master/master.cpp, lines 2937-2940
> > 
> >
> > Just wondering, how come the braces were removed for the `default` case?

We won't be adding more stuff to the default case. That's the reason I removed 
the braces:)


- Jie


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


On Jan. 29, 2015, 7:03 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30283/
> ---
> 
> (Updated Jan. 29, 2015, 7:03 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Michael Park.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> To avoid potential conflicts.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 54f26900ac8c63e79a1f89562a988c9a2567d209 
> 
> Diff: https://reviews.apache.org/r/30283/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 30423: Fixed the flaky FaultToleranceTest.eSchedulerFailoverFrameworkMessage test.

[Ben Mahler]

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

Review request for mesos and Vinod Kone.


Bugs: MESOS-2302
https://issues.apache.org/jira/browse/MESOS-2302


Repository: mesos-git


Description
---

>From MESOS-2302:

Comparing the runs, you can see that there's a race in which the driver gets 
stopped before the error reaches it:

Bad run:
```
I0123 18:50:12.034864 15688 sched.cpp:1471] Asked to stop the driver
...
I0123 18:50:12.037359 15710 sched.cpp:788] Ignoring error message because the 
driver is not running!
```

Good run:
```
I0122 19:15:01.721943  3540 sched.cpp:792] Got error 'Framework failed over'
I0122 19:15:01.721972  3540 sched.cpp:1505] Asked to abort the driver
I0122 19:15:01.722039  3540 sched.cpp:803] Scheduler::error took 26469ns
...
I0122 19:15:01.727665  3518 sched.cpp:1471] Asked to stop the driver
```


Diffs
-

  src/tests/fault_tolerance_tests.cpp e0065b3a89b9af27c5584bcfdd8c790814bf1af6 

Diff: https://reviews.apache.org/r/30423/diff/


Testing
---

100 repetitions of this test


Thanks,

Ben Mahler

Re: Review Request 30082: Cleaned up namespace hierarchy in allocation sources.

[Niklas Nielsen]

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

Ship it!


Ship It!

- Niklas Nielsen


On Jan. 28, 2015, 9:32 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30082/
> ---
> 
> (Updated Jan. 28, 2015, 9:32 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2213
> https://issues.apache.org/jira/browse/MESOS-2213
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/local/local.hpp 0aa50ef 
>   src/local/local.cpp 76e73a4 
>   src/master/allocation/allocator.hpp PRE-CREATION 
>   src/master/allocation/mesos/allocator.hpp PRE-CREATION 
>   src/master/allocation/mesos/hierarchical.hpp PRE-CREATION 
>   src/master/allocation/sorter.hpp PRE-CREATION 
>   src/master/allocation/sorters/drf.hpp PRE-CREATION 
>   src/master/allocation/sorters/drf.cpp PRE-CREATION 
>   src/master/main.cpp e5e76ce 
>   src/master/master.hpp 1d342e5 
>   src/master/master.cpp 54f2690 
>   src/tests/cluster.hpp 74cedb3 
>   src/tests/fault_tolerance_tests.cpp e0065b3 
>   src/tests/hierarchical_allocator_tests.cpp f44d9e9 
>   src/tests/master_allocator_tests.cpp 2430622 
>   src/tests/master_authorization_tests.cpp 42ffe24 
>   src/tests/master_tests.cpp 678d27f 
>   src/tests/mesos.hpp 3f4704b 
>   src/tests/mesos.cpp 5ed4df5 
>   src/tests/partition_tests.cpp fea7801 
>   src/tests/rate_limiting_tests.cpp 7f5ca25 
>   src/tests/resource_offers_tests.cpp ffad1f8 
>   src/tests/slave_recovery_tests.cpp d135856 
>   src/tests/sorter_tests.cpp 520a42e 
> 
> Diff: https://reviews.apache.org/r/30082/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 28697: Added ReservationType for dynamic reservations.

[Jie Yu]

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

Ship it!


That looks good to me!

- Jie Yu


On Jan. 29, 2015, 6:10 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28697/
> ---
> 
> (Updated Jan. 29, 2015, 6:10 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-2137
> https://issues.apache.org/jira/browse/MESOS-2137
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Adding new protobuf messages necessary to support dynamic reservations.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 540071db64961466eb75c779b3ea6863f4594437 
> 
> Diff: https://reviews.apache.org/r/28697/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 30386: Added support for CREATE operation in master.

[Ben Mahler]

> On Jan. 29, 2015, 6:49 p.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 2929-2931
> > 
> >
> > What is this TODO? How could one create a volume that's already 
> > reserved?
> 
> Michael Park wrote:
> This is if we're creating a volume with dynamically reserved regular 
> disk. We want to transition from `[(disk, 2, role, DYNAMIC)]` to `[(disk, 2, 
> role, DYNAMIC, persistence=id, ...)]` not to `[(disk, 2, role, DYNAMIC), 
> (disk, 2, role, DYNAMIC, persistence=id, ...)]`.

Gotcha, let's clarify the TODO to reflect that the volumes are not reserved.


- Ben


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


On Jan. 29, 2015, 12:12 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30386/
> ---
> 
> (Updated Jan. 29, 2015, 12:12 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Michael Park, and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Added support for CREATE operation in master.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 1d342e56116ad63aade43484b6899ce26f25abfd 
>   src/master/master.cpp 54f26900ac8c63e79a1f89562a988c9a2567d209 
> 
> Diff: https://reviews.apache.org/r/30386/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 30423: Fixed the flaky FaultToleranceTest.eSchedulerFailoverFrameworkMessage test.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [30423]

All tests passed.

- Mesos ReviewBot


On Jan. 29, 2015, 7:54 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30423/
> ---
> 
> (Updated Jan. 29, 2015, 7:54 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2302
> https://issues.apache.org/jira/browse/MESOS-2302
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> From MESOS-2302:
> 
> Comparing the runs, you can see that there's a race in which the driver gets 
> stopped before the error reaches it:
> 
> Bad run:
> ```
> I0123 18:50:12.034864 15688 sched.cpp:1471] Asked to stop the driver
> ...
> I0123 18:50:12.037359 15710 sched.cpp:788] Ignoring error message because the 
> driver is not running!
> ```
> 
> Good run:
> ```
> I0122 19:15:01.721943  3540 sched.cpp:792] Got error 'Framework failed over'
> I0122 19:15:01.721972  3540 sched.cpp:1505] Asked to abort the driver
> I0122 19:15:01.722039  3540 sched.cpp:803] Scheduler::error took 26469ns
> ...
> I0122 19:15:01.727665  3518 sched.cpp:1471] Asked to stop the driver
> ```
> 
> 
> Diffs
> -
> 
>   src/tests/fault_tolerance_tests.cpp 
> e0065b3a89b9af27c5584bcfdd8c790814bf1af6 
> 
> Diff: https://reviews.apache.org/r/30423/diff/
> 
> 
> Testing
> ---
> 
> 100 repetitions of this test
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Kill the "internal" namespace

[Kapil Arya]

As Dominic pointed out, if the framework writers are not going to use the
headers, then it doesn't make much sense to create a hierarchy for
"internal" module-specific stuff at this point.

On Tue, Jan 27, 2015 at 2:12 PM, Kapil Arya  wrote:

>
>
> On Tue, Jan 27, 2015 at 10:28 AM, Jie Yu  wrote:
>
>> I think Mesos is kind of different than other C/C++ projects like apache
>> httpd, llvm, etc. For httpd, it's clear that the public headers are for
>> module writers. For llvm, the public headers are for plugin writers.
>>
>> For Mesos, we have two types of developers: framework writers and module
>> writers. I think it'll be great if we can separate headers for these two
>> types of writers in an obvious way.
>>
>> What I am proposing is: we still keep the internal namespace in Mesos, but
>> move those internal interfaces that needed by the module to
>> include/mesos/internal. So the layout of the include/ is like the
>> following:
>>
>> include/
>>   |-- mesos/ <-- public headers for framework/executor writers
>>   |-- scheduler.hpp
>>   |-- executor.hpp
>>   ...
>>   |-- internal/  <-- headers for module writers
>>   |--- slave/
>>   |--- master/
>>
>
> I think, having "internal" in a public headers isn't a good idea.  The
> module API is supposed to be fairly stable.  This, in fact, will be the
> driving force to enable smooth upgrades even when there are some modules
> installed for master/slave.
>
>
>> Thoughts?
>>
>> - Jie
>>
>>
>> On Tue, Jan 27, 2015 at 10:04 AM, Dominic Hamon 
>> wrote:
>>
>> > +1
>> >
>> > if people write comments that say that, i'll be the first to recommend a
>> > redesign of what they're writing :)
>> >
>> >
>> >
>> > On Tue, Jan 27, 2015 at 8:47 AM, Benjamin Hindman <
>> b...@eecs.berkeley.edu>
>> > wrote:
>> >
>> > > I think the point that Jie is making is that we have sometimes wanted
>> to
>> > > have "internal" implementation details exposed in public headers.
>> This is
>> > > common in Stout because everything has to be in headers.
>> > >
>> > > With that in mind, our goal for Mesos should be to have _zero_
>> internal
>> > > implementation details in public headers. If people find themselves
>> > writing
>> > > comments that say "don't use these global variables or functions
>> because
>> > > they're internal to the implementation" then I'll be the first to
>> > recommend
>> > > that we reintroduce the 'internal' namespace.
>> > >
>> > > As for Stout I think we should keep the 'internal' namespace.
>> > >
>> > > On Mon, Jan 26, 2015 at 10:25 PM, Jie Yu  wrote:
>> > >
>> > > > One benefit of having an internal namespace is that it tells the
>> > > > framework/executor writer that those symbols/method/class are
>> internal
>> > to
>> > > > Mesos core and should not be used.
>> > > >
>> > > > If we kill all the internal namespaces and move many headers like
>> > > > isolator.hpp to include/mesos, how does the framework writer know
>> that
>> > > > he/she shouldn't use some of the headers because they are internal
>> to
>> > > Mesos
>> > > > and are subject to change?
>> > > >
>> > > > For modules, I am wondering can we separate Mesos public headers (in
>> > > > include/mesos right now) from those headers that are only for
>> building
>> > > > modules (more like internal public headers).
>> > > >
>> > > > Thoughts?
>> > > >
>> > > > - Jie
>> > > >
>> > > > On Mon, Jan 26, 2015 at 12:35 PM, Kapil Arya 
>> > > wrote:
>> > > >
>> > > > > Hi All,
>> > > > >
>> > > > > TLDR: We currently use "mesos::internal" namespace for almost
>> > > everything
>> > > > > inside src/.  However, in most cases, it is directly enclosing
>> > another
>> > > > > namespace.  This makes the "internal" namespace redundant and we
>> > should
>> > > > > kill it.
>> > > > >
>> > > > > I learned from Ben Hindman that the original motivation for
>> > introducing
>> > > > an
>> > > > > explicit internal namespace was to discourage people from exposing
>> > > > internal
>> > > > > symbols through "extern", etc.  Since we don't seem to expose
>> symbols
>> > > > > through "extern" in our codebase, I think it's safe to kill this
>> > > > namespace.
>> > > > >
>> > > > > Here is a list of files that define classes directly in the
>> > > > mesos::internal
>> > > > > namespace (i.e. without enclosing a separate namespace) [1]:
>> > > > >
>> > > > > authorizer/authorizer.hpp
>> > > > > common/http.hpp
>> > > > > common/attributes.hpp
>> > > > > common/lock.hpp
>> > > > > files/files.hpp
>> > > > > hook/manager.hpp
>> > > > > master/contender.hpp
>> > > > > master/detector.hpp
>> > > > > usage/usage.hpp
>> > > > > watcher/whitelist_watcher.hpp
>> > > > >
>> > > > > messages/messages.pb.h
>> > > > >
>> > > > > Of the above list, things like hook/manager.hpp and
>> > > > > master/{contender,detector}.hpp can be moved into their own
>> > namespaces.
>> > > > I
>> > > > > am sure, we can come up with a strategy for the rest as well.
>> > > > >
>> > > > > O

Re: Review Request 30395: etcd master contender + detector

[Michael Park]

> On Jan. 29, 2015, 5:50 p.m., Alexander Rukletsov wrote:
> > src/master/contender.cpp, line 82
> > 
> >
> > You can safely omit trailing underscores.

In this case, yes that is true. This topic has come up before and perhaps it's 
better suited for the dev list so that we can get a community-wide consensus on 
this.

In general, I think we should try to avoid shadowing. *Especially* between 
local variables (including function parameters) and member variables because 
the member variable declarations are typically lexically not in scope.

1)

```
struct Obj
{
  // Original author: "Eh, we don't need to refer to the member 'foo' so this 
is fine."
  Obj(Foo &&foo)
  : foo(std::move(foo)) {}
  
  Foo foo;
};
```

```
struct Obj
{
  Obj(Foo &&foo)
  : foo(std::move(foo)) {
// New author: "ok, let's do some stuff with member 'foo'!"
// Oops... this will segfault.
std::cout << foo << std::endl;
  }
  
  Foo foo;
};
```

2)

```
// Original author: "There's a member 'foo' but I don't need it for this 
function."
Obj::F(const Foo &foo) {
  // Do stuff with local 'foo' ...
}
```

```
Obj::F(const Foo &foo) {
  // Do stuff with local 'foo' ...
  // /* ... */
  // New author: (middle of debugging) "What state is (member) 'foo' in...?"
  // Local 'foo' gets printed instead, doesn't realize the problem until they 
look above
  // to find the shadowed variable and... fury.
  std::cout << foo << std::endl;
}
```

In short, I think we should opt to stay away from running into issues like this 
by not shadowing member variables.
It keeps us away from running into stupid bugs that waste developer's time, and 
also eliminates the need to reevaluate if the names of variables need to be 
changed when adding new code.


- Michael


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


On Jan. 29, 2015, 3:58 a.m., Cody Maloney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30395/
> ---
> 
> (Updated Jan. 29, 2015, 3:58 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-1806
> https://issues.apache.org/jira/browse/MESOS-1806
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> etcd master contender + detector
> 
> 
> Diffs
> -
> 
>   src/etcd/etcd.cpp PRE-CREATION 
>   src/master/contender.hpp 76beb5f973ae02507849233b6d73c43293669489 
>   src/master/contender.cpp c1bf82b621d6b46afe001acafe9ee53336726406 
>   src/master/detector.hpp 2905e2b3536e14e9df3570da172603e6ed81aae1 
>   src/master/detector.cpp 700eb9dde8e71648bacc00a82766634f77cf2d15 
> 
> Diff: https://reviews.apache.org/r/30395/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Cody Maloney
> 
>

Re: Review Request 30395: etcd master contender + detector

[Cody Maloney]

> On Jan. 29, 2015, 5:50 p.m., Alexander Rukletsov wrote:
> > src/master/contender.cpp, line 82
> > 
> >
> > You can safely omit trailing underscores.
> 
> Michael Park wrote:
> In this case, yes that is true. This topic has come up before and perhaps 
> it's better suited for the dev list so that we can get a community-wide 
> consensus on this.
> 
> In general, I think we should try to avoid shadowing. *Especially* 
> between local variables (including function parameters) and member variables 
> because the member variable declarations are typically lexically not in scope.
> 
> 1)
> 
> ```
> struct Obj
> {
>   // Original author: "Eh, we don't need to refer to the member 'foo' so 
> this is fine."
>   Obj(Foo &&foo)
>   : foo(std::move(foo)) {}
>   
>   Foo foo;
> };
> ```
> 
> ```
> struct Obj
> {
>   Obj(Foo &&foo)
>   : foo(std::move(foo)) {
> // New author: "ok, let's do some stuff with member 'foo'!"
> // Oops... this will segfault.
> std::cout << foo << std::endl;
>   }
>   
>   Foo foo;
> };
> ```
> 
> 2)
> 
> ```
> // Original author: "There's a member 'foo' but I don't need it for this 
> function."
> Obj::F(const Foo &foo) {
>   // Do stuff with local 'foo' ...
> }
> ```
> 
> ```
> Obj::F(const Foo &foo) {
>   // Do stuff with local 'foo' ...
>   // /* ... */
>   // New author: (middle of debugging) "What state is (member) 'foo' 
> in...?"
>   // Local 'foo' gets printed instead, doesn't realize the problem until 
> they look above
>   // to find the shadowed variable and... fury.
>   std::cout << foo << std::endl;
> }
> ```
> 
> In short, I think we should opt to stay away from running into issues 
> like this by not shadowing member variables.
> It keeps us away from running into stupid bugs that waste developer's 
> time, and also eliminates the need to reevaluate if the names of variables 
> need to be changed when adding new code.

Some examples from the codebase of existing shadowing and problems which could 
be encountered working / refactoring around them.

stout/json.hpp: 
```
inline Value convert(const picojson::value& value)
{
  if (value.is()) {
return Null();
  } else if (value.is()) {
return Boolean(value.get());
  } else if (value.is()) {
Object object;
foreachpair (const std::string& name,
 const picojson::value& value,
 value.get()) {
  object.values[name] = convert(value);
}
return object;
  } else if (value.is()) {
Array array;
foreach (const picojson::value& value,
 value.get()) {
  array.values.push_back(convert(value));
}
return array;
  } else if (value.is()) {
return Number(value.get());
  } else if (value.is()) {
return String(value.get());
  }
  return Null();
}
```
The foreachpair creates a second value which it uses in the assignment. Yes, 
this works right, but when scanning over the code if I don't catch that there 
was an additional definition embedded in the foreach...

stout/flags.hpp:
```
template 
void FlagsBase::add(
T1 Flags::*t1,
const std::string& name,
const std::string& help,
const T2& t2)
{
  Flags* flags = dynamic_cast(this);
  if (flags == NULL) {
ABORT("Attempted to add flag '" + name + "' with incompatible type");
  } else {
flags->*t1 = t2; // Set the default.
  }

  Flag flag;
  flag.name = name;
  flag.help = help;
  flag.boolean = typeid(T1) == typeid(bool);
  flag.loader = lambda::bind(
  &MemberLoader::load,
  lambda::_1,
  t1,
  lambda::function(const std::string&)>(
  lambda::bind(&parse, lambda::_1)),
  name,
  lambda::_2);
  flag.stringify = lambda::bind(
  &MemberStringifier,
  lambda::_1,
  t1);

  // Update the help string to include the default value.
  flag.help += help.size() > 0 && help.find_last_of("\n\r") != help.size() - 1
? " (default: " // On same line, add space.
: "(default: "; // On newline.
  flag.help += stringify(t2);
  flag.help += ")";

  add(flag);
}
```
If the delegated add had been written inline (flags[flag.name] = flag), then 
things wouldn't work right...


stout/os/killtre
```
inline Try > killtree(
pid_t pid,
int signal,
bool groups = false,
bool sessions = false)
{
  ...

  while (!queue.empty()) {
pid_t pid = queue.front();
queue.pop();

...

  }
}
```
If I need to modify the code inside the while loop to do something like exclude 
the original PID to resolve some sort of bug, I will have to rename a lot of 
variables for a small change in functionality. Most likely I won't notice that 
pid became even more local, and will first spend a while debugging why the code 
"Reads" right but doesn't do w

Re: Review Request 30395: etcd master contender + detector

[Alexander Rukletsov]

> On Jan. 29, 2015, 5:50 p.m., Alexander Rukletsov wrote:
> > src/master/contender.cpp, line 82
> > 
> >
> > You can safely omit trailing underscores.
> 
> Michael Park wrote:
> In this case, yes that is true. This topic has come up before and perhaps 
> it's better suited for the dev list so that we can get a community-wide 
> consensus on this.
> 
> In general, I think we should try to avoid shadowing. *Especially* 
> between local variables (including function parameters) and member variables 
> because the member variable declarations are typically lexically not in scope.
> 
> 1)
> 
> ```
> struct Obj
> {
>   // Original author: "Eh, we don't need to refer to the member 'foo' so 
> this is fine."
>   Obj(Foo &&foo)
>   : foo(std::move(foo)) {}
>   
>   Foo foo;
> };
> ```
> 
> ```
> struct Obj
> {
>   Obj(Foo &&foo)
>   : foo(std::move(foo)) {
> // New author: "ok, let's do some stuff with member 'foo'!"
> // Oops... this will segfault.
> std::cout << foo << std::endl;
>   }
>   
>   Foo foo;
> };
> ```
> 
> 2)
> 
> ```
> // Original author: "There's a member 'foo' but I don't need it for this 
> function."
> Obj::F(const Foo &foo) {
>   // Do stuff with local 'foo' ...
> }
> ```
> 
> ```
> Obj::F(const Foo &foo) {
>   // Do stuff with local 'foo' ...
>   // /* ... */
>   // New author: (middle of debugging) "What state is (member) 'foo' 
> in...?"
>   // Local 'foo' gets printed instead, doesn't realize the problem until 
> they look above
>   // to find the shadowed variable and... fury.
>   std::cout << foo << std::endl;
> }
> ```
> 
> In short, I think we should opt to stay away from running into issues 
> like this by not shadowing member variables.
> It keeps us away from running into stupid bugs that waste developer's 
> time, and also eliminates the need to reevaluate if the names of variables 
> need to be changed when adding new code.
> 
> Cody Maloney wrote:
> Some examples from the codebase of existing shadowing and problems which 
> could be encountered working / refactoring around them.
> 
> stout/json.hpp: 
> ```
> inline Value convert(const picojson::value& value)
> {
>   if (value.is()) {
> return Null();
>   } else if (value.is()) {
> return Boolean(value.get());
>   } else if (value.is()) {
> Object object;
> foreachpair (const std::string& name,
>  const picojson::value& value,
>  value.get()) {
>   object.values[name] = convert(value);
> }
> return object;
>   } else if (value.is()) {
> Array array;
> foreach (const picojson::value& value,
>  value.get()) {
>   array.values.push_back(convert(value));
> }
> return array;
>   } else if (value.is()) {
> return Number(value.get());
>   } else if (value.is()) {
> return String(value.get());
>   }
>   return Null();
> }
> ```
> The foreachpair creates a second value which it uses in the assignment. 
> Yes, this works right, but when scanning over the code if I don't catch that 
> there was an additional definition embedded in the foreach...
> 
> stout/flags.hpp:
> ```
> template 
> void FlagsBase::add(
> T1 Flags::*t1,
> const std::string& name,
> const std::string& help,
> const T2& t2)
> {
>   Flags* flags = dynamic_cast(this);
>   if (flags == NULL) {
> ABORT("Attempted to add flag '" + name + "' with incompatible type");
>   } else {
> flags->*t1 = t2; // Set the default.
>   }
> 
>   Flag flag;
>   flag.name = name;
>   flag.help = help;
>   flag.boolean = typeid(T1) == typeid(bool);
>   flag.loader = lambda::bind(
>   &MemberLoader::load,
>   lambda::_1,
>   t1,
>   lambda::function(const std::string&)>(
>   lambda::bind(&parse, lambda::_1)),
>   name,
>   lambda::_2);
>   flag.stringify = lambda::bind(
>   &MemberStringifier,
>   lambda::_1,
>   t1);
> 
>   // Update the help string to include the default value.
>   flag.help += help.size() > 0 && help.find_last_of("\n\r") != 
> help.size() - 1
> ? " (default: " // On same line, add space.
> : "(default: "; // On newline.
>   flag.help += stringify(t2);
>   flag.help += ")";
> 
>   add(flag);
> }
> ```
> If the delegated add had been written inline (flags[flag.name] = flag), 
> then things wouldn't work right...
> 
> 
> stout/os/killtre
> ```
> inline Try > killtree(
> 

Review Request 30426: Use libc++ for travis clang builds

[Dominic Hamon]

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

Review request for mesos and Cody Maloney.


Bugs: MESOS-2304
https://issues.apache.org/jira/browse/MESOS-2304


Repository: mesos-git


Description
---

Use libc++ for travis clang builds


Diffs
-

  .travis.yml 4991dd77a62b8bfc6045099ca99682b47aadaa97 

Diff: https://reviews.apache.org/r/30426/diff/


Testing
---


Thanks,

Dominic Hamon

Re: Review Request 30426: Use libc++ for travis clang builds

[Cody Maloney]

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



.travis.yml


--enable-debug / --enable-release don't do anything currently when CXXFLAGS 
has an initial value since linux distribution build systems usually set them 
specifically to get debug / release / optimized /etc. You should manually add 
"-g -O0".

I actually thing we should add a mesos configure check for this:
if compiler == clang && libc++ is installed:
use libc++

Will make clang "just work" more of the times. libstdc++ has a habit lately 
of breaking clang with new releases.


- Cody Maloney


On Jan. 29, 2015, 9:53 p.m., Dominic Hamon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30426/
> ---
> 
> (Updated Jan. 29, 2015, 9:53 p.m.)
> 
> 
> Review request for mesos and Cody Maloney.
> 
> 
> Bugs: MESOS-2304
> https://issues.apache.org/jira/browse/MESOS-2304
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Use libc++ for travis clang builds
> 
> 
> Diffs
> -
> 
>   .travis.yml 4991dd77a62b8bfc6045099ca99682b47aadaa97 
> 
> Diff: https://reviews.apache.org/r/30426/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>

Re: Review Request 30426: Use libc++ for travis clang builds

[Dominic Hamon]

> On Jan. 29, 2015, 1:59 p.m., Cody Maloney wrote:
> > .travis.yml, line 30
> > 
> >
> > --enable-debug / --enable-release don't do anything currently when 
> > CXXFLAGS has an initial value since linux distribution build systems 
> > usually set them specifically to get debug / release / optimized /etc. You 
> > should manually add "-g -O0".
> > 
> > I actually thing we should add a mesos configure check for this:
> > if compiler == clang && libc++ is installed:
> > use libc++
> > 
> > Will make clang "just work" more of the times. libstdc++ has a habit 
> > lately of breaking clang with new releases.

re: configure check: sounds reasonable.

re: enable-: that's surprising. CXXFLAGS could be set anywhere in 
the system and a developer on Mesos might not realise. Perhaps we should be 
firmer about what we mean by 'debug' and 'release' so we can just set the flags.


- Dominic


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


On Jan. 29, 2015, 1:53 p.m., Dominic Hamon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30426/
> ---
> 
> (Updated Jan. 29, 2015, 1:53 p.m.)
> 
> 
> Review request for mesos and Cody Maloney.
> 
> 
> Bugs: MESOS-2304
> https://issues.apache.org/jira/browse/MESOS-2304
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Use libc++ for travis clang builds
> 
> 
> Diffs
> -
> 
>   .travis.yml 4991dd77a62b8bfc6045099ca99682b47aadaa97 
> 
> Diff: https://reviews.apache.org/r/30426/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>

Re: Review Request 30426: Use libc++ for travis clang builds

[Cody Maloney]

> On Jan. 29, 2015, 9:59 p.m., Cody Maloney wrote:
> > .travis.yml, line 30
> > 
> >
> > --enable-debug / --enable-release don't do anything currently when 
> > CXXFLAGS has an initial value since linux distribution build systems 
> > usually set them specifically to get debug / release / optimized /etc. You 
> > should manually add "-g -O0".
> > 
> > I actually thing we should add a mesos configure check for this:
> > if compiler == clang && libc++ is installed:
> > use libc++
> > 
> > Will make clang "just work" more of the times. libstdc++ has a habit 
> > lately of breaking clang with new releases.
> 
> Dominic Hamon wrote:
> re: configure check: sounds reasonable.
> 
> re: enable-: that's surprising. CXXFLAGS could be set 
> anywhere in the system and a developer on Mesos might not realise. Perhaps we 
> should be firmer about what we mean by 'debug' and 'release' so we can just 
> set the flags.

Re: enable-

The problem is how do you pass the knowledge down to stout / libprocess / etc? 
They don't see the outer configure flags unfortunately. Initially I always set 
the flags for --enable-optimize / --enable-debug / default, but that resulted 
in builds which would get an optimized mesos but a unoptimized libprocess + 
stout + misc. other tools...

Might be able to restructure it to a "if explicitly stated then will force 
append to CXXFLAGS, otherwise we only overwrite CXXFLAGS if they were unset. 
But that will take some configure digging.


- Cody


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


On Jan. 29, 2015, 9:53 p.m., Dominic Hamon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30426/
> ---
> 
> (Updated Jan. 29, 2015, 9:53 p.m.)
> 
> 
> Review request for mesos and Cody Maloney.
> 
> 
> Bugs: MESOS-2304
> https://issues.apache.org/jira/browse/MESOS-2304
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Use libc++ for travis clang builds
> 
> 
> Diffs
> -
> 
>   .travis.yml 4991dd77a62b8bfc6045099ca99682b47aadaa97 
> 
> Diff: https://reviews.apache.org/r/30426/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>

Re: Review Request 30426: Use libc++ for travis clang builds

[Dominic Hamon]

> On Jan. 29, 2015, 1:59 p.m., Cody Maloney wrote:
> > .travis.yml, line 30
> > 
> >
> > --enable-debug / --enable-release don't do anything currently when 
> > CXXFLAGS has an initial value since linux distribution build systems 
> > usually set them specifically to get debug / release / optimized /etc. You 
> > should manually add "-g -O0".
> > 
> > I actually thing we should add a mesos configure check for this:
> > if compiler == clang && libc++ is installed:
> > use libc++
> > 
> > Will make clang "just work" more of the times. libstdc++ has a habit 
> > lately of breaking clang with new releases.
> 
> Dominic Hamon wrote:
> re: configure check: sounds reasonable.
> 
> re: enable-: that's surprising. CXXFLAGS could be set 
> anywhere in the system and a developer on Mesos might not realise. Perhaps we 
> should be firmer about what we mean by 'debug' and 'release' so we can just 
> set the flags.
> 
> Cody Maloney wrote:
> Re: enable-
> 
> The problem is how do you pass the knowledge down to stout / libprocess / 
> etc? They don't see the outer configure flags unfortunately. Initially I 
> always set the flags for --enable-optimize / --enable-debug / default, but 
> that resulted in builds which would get an optimized mesos but a unoptimized 
> libprocess + stout + misc. other tools...
> 
> Might be able to restructure it to a "if explicitly stated then will 
> force append to CXXFLAGS, otherwise we only overwrite CXXFLAGS if they were 
> unset. But that will take some configure digging.

we should probably take this to a jira but.. if you have --enable- 
in libprocess and stout, they'll pick up the flag too. it'll take some 
maintenance to ensure a debug libprocess is the same as a debug mesos (or at 
least ABI compatible) but that should 'just work'.


- Dominic


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


On Jan. 29, 2015, 1:53 p.m., Dominic Hamon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30426/
> ---
> 
> (Updated Jan. 29, 2015, 1:53 p.m.)
> 
> 
> Review request for mesos and Cody Maloney.
> 
> 
> Bugs: MESOS-2304
> https://issues.apache.org/jira/browse/MESOS-2304
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Use libc++ for travis clang builds
> 
> 
> Diffs
> -
> 
>   .travis.yml 4991dd77a62b8bfc6045099ca99682b47aadaa97 
> 
> Diff: https://reviews.apache.org/r/30426/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>

Re: Review Request 30426: Use libc++ for travis clang builds

[Cody Maloney]

> On Jan. 29, 2015, 9:59 p.m., Cody Maloney wrote:
> > .travis.yml, line 30
> > 
> >
> > --enable-debug / --enable-release don't do anything currently when 
> > CXXFLAGS has an initial value since linux distribution build systems 
> > usually set them specifically to get debug / release / optimized /etc. You 
> > should manually add "-g -O0".
> > 
> > I actually thing we should add a mesos configure check for this:
> > if compiler == clang && libc++ is installed:
> > use libc++
> > 
> > Will make clang "just work" more of the times. libstdc++ has a habit 
> > lately of breaking clang with new releases.
> 
> Dominic Hamon wrote:
> re: configure check: sounds reasonable.
> 
> re: enable-: that's surprising. CXXFLAGS could be set 
> anywhere in the system and a developer on Mesos might not realise. Perhaps we 
> should be firmer about what we mean by 'debug' and 'release' so we can just 
> set the flags.
> 
> Cody Maloney wrote:
> Re: enable-
> 
> The problem is how do you pass the knowledge down to stout / libprocess / 
> etc? They don't see the outer configure flags unfortunately. Initially I 
> always set the flags for --enable-optimize / --enable-debug / default, but 
> that resulted in builds which would get an optimized mesos but a unoptimized 
> libprocess + stout + misc. other tools...
> 
> Might be able to restructure it to a "if explicitly stated then will 
> force append to CXXFLAGS, otherwise we only overwrite CXXFLAGS if they were 
> unset. But that will take some configure digging.
> 
> Dominic Hamon wrote:
> we should probably take this to a jira but.. if you have 
> --enable- in libprocess and stout, they'll pick up the flag too. 
> it'll take some maintenance to ensure a debug libprocess is the same as a 
> debug mesos (or at least ABI compatible) but that should 'just work'.

It wasn't when I was working on restructuring the flags which is why I ended up 
with what I did. Note there are some bundled libraries even still which get 
built with different debug/release flags then the overarching mesos...


- Cody


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


On Jan. 29, 2015, 9:53 p.m., Dominic Hamon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30426/
> ---
> 
> (Updated Jan. 29, 2015, 9:53 p.m.)
> 
> 
> Review request for mesos and Cody Maloney.
> 
> 
> Bugs: MESOS-2304
> https://issues.apache.org/jira/browse/MESOS-2304
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Use libc++ for travis clang builds
> 
> 
> Diffs
> -
> 
>   .travis.yml 4991dd77a62b8bfc6045099ca99682b47aadaa97 
> 
> Diff: https://reviews.apache.org/r/30426/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>

Review Request 30428: Added hidden field to labels

[Niklas Nielsen]

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

Review request for mesos, Ben Mahler and Kapil Arya.


Bugs: MESOS-2259
https://issues.apache.org/jira/browse/MESOS-2259


Repository: mesos-git


Description
---

Task labels are used by label-decorator hooks to insert hook-specific labels. 
Some hook implementation might not want to expose their label for security 
reasons.
We should allow certain labels to be marked as "hidden", such that they are 
invisible from the "web ui" to start with.


Diffs
-

  include/mesos/mesos.proto 26003fada2e4c4be0e9ebc4663f7ebab858cc48d 
  src/common/http.cpp 915cba10725685da55a53b249b8b676b5e229d86 
  src/tests/master_tests.cpp 678d27f41a2f246c714c77adb132263c0c2c61ed 
  src/tests/slave_tests.cpp e7e2af63da785644f3f7e6e23607c02be962a2c6 

Diff: https://reviews.apache.org/r/30428/diff/


Testing
---

make check with two new tests


Thanks,

Niklas Nielsen

Review Request 30430: Fixed a bug in Resources::contains.

[Jie Yu]

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

Review request for mesos and Ben Mahler.


Repository: mesos-git


Description
---

Fixed a bug in Resources::contains.


Diffs
-

  src/common/resources.cpp 68f64213c47be4700bcb22cd5b76ba6ff616960d 
  src/tests/resources_tests.cpp 9fd2135a7545268e2f81915ed0a019de36f3d6e1 

Diff: https://reviews.apache.org/r/30430/diff/


Testing
---

make check.


Thanks,

Jie Yu

Re: Review Request 30430: Fixed a bug in Resources::contains.

[Ben Mahler]

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

Ship it!


Thanks for testing!


src/tests/resources_tests.cpp


Maybe s/Subset/Contains/ and we can add a normal (non-duplicate) contains 
test in here?


- Ben Mahler


On Jan. 29, 2015, 11:08 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30430/
> ---
> 
> (Updated Jan. 29, 2015, 11:08 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Fixed a bug in Resources::contains.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 68f64213c47be4700bcb22cd5b76ba6ff616960d 
>   src/tests/resources_tests.cpp 9fd2135a7545268e2f81915ed0a019de36f3d6e1 
> 
> Diff: https://reviews.apache.org/r/30430/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 30432: Fixed the flaky MasterAuthorizationTest.FrameworkRemovedBeforeReregistration test.

[Ben Mahler]

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

Review request for mesos and Vinod Kone.


Bugs: MESOS-2306
https://issues.apache.org/jira/browse/MESOS-2306


Repository: mesos-git


Description
---

>From MESOS-2306:

There is a race here where the scheduler retries registration an extra time in 
the failing test. This then messes up the mock expectations:

```
  // Return a pending future from authorizer after first attempt.
  Future authorize2;
  Promise promise2;
  EXPECT_CALL(authorizer, authorize(An()))
.WillOnce(Return(true))
.WillOnce(DoAll(FutureSatisfy(&authorize2),
Return(promise2.future(
.WillRepeatedly(Return(true)); // Authorize subsequent registration retries.
```

We want to simulate a master detection and capture the authorization, but when 
there is a spurious retry, our simulated retry falls into the WillRepeatedly 
case and is authorized immediately.


Diffs
-

  src/tests/master_authorization_tests.cpp 
42ffe24685b859fb16132d22a932f6637fe9b5c3 

Diff: https://reviews.apache.org/r/30432/diff/


Testing
---

500 iterations of this test


Thanks,

Ben Mahler

Re: Review Request 30428: Added hidden field to labels

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [30428]

All tests passed.

- Mesos ReviewBot


On Jan. 29, 2015, 10:30 p.m., Niklas Nielsen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30428/
> ---
> 
> (Updated Jan. 29, 2015, 10:30 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Kapil Arya.
> 
> 
> Bugs: MESOS-2259
> https://issues.apache.org/jira/browse/MESOS-2259
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Task labels are used by label-decorator hooks to insert hook-specific labels. 
> Some hook implementation might not want to expose their label for security 
> reasons.
> We should allow certain labels to be marked as "hidden", such that they are 
> invisible from the "web ui" to start with.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 26003fada2e4c4be0e9ebc4663f7ebab858cc48d 
>   src/common/http.cpp 915cba10725685da55a53b249b8b676b5e229d86 
>   src/tests/master_tests.cpp 678d27f41a2f246c714c77adb132263c0c2c61ed 
>   src/tests/slave_tests.cpp e7e2af63da785644f3f7e6e23607c02be962a2c6 
> 
> Diff: https://reviews.apache.org/r/30428/diff/
> 
> 
> Testing
> ---
> 
> make check with two new tests
> 
> 
> Thanks,
> 
> Niklas Nielsen
> 
>

Re: Review Request 30423: Fixed the flaky FaultToleranceTest.eSchedulerFailoverFrameworkMessage test.

[Vinod Kone]

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

Ship it!



src/tests/fault_tolerance_tests.cpp


I think this could be moved up, right after driver2.start() ?


- Vinod Kone


On Jan. 29, 2015, 7:54 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30423/
> ---
> 
> (Updated Jan. 29, 2015, 7:54 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2302
> https://issues.apache.org/jira/browse/MESOS-2302
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> From MESOS-2302:
> 
> Comparing the runs, you can see that there's a race in which the driver gets 
> stopped before the error reaches it:
> 
> Bad run:
> ```
> I0123 18:50:12.034864 15688 sched.cpp:1471] Asked to stop the driver
> ...
> I0123 18:50:12.037359 15710 sched.cpp:788] Ignoring error message because the 
> driver is not running!
> ```
> 
> Good run:
> ```
> I0122 19:15:01.721943  3540 sched.cpp:792] Got error 'Framework failed over'
> I0122 19:15:01.721972  3540 sched.cpp:1505] Asked to abort the driver
> I0122 19:15:01.722039  3540 sched.cpp:803] Scheduler::error took 26469ns
> ...
> I0122 19:15:01.727665  3518 sched.cpp:1471] Asked to stop the driver
> ```
> 
> 
> Diffs
> -
> 
>   src/tests/fault_tolerance_tests.cpp 
> e0065b3a89b9af27c5584bcfdd8c790814bf1af6 
> 
> Diff: https://reviews.apache.org/r/30423/diff/
> 
> 
> Testing
> ---
> 
> 100 repetitions of this test
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 30432: Fixed the flaky MasterAuthorizationTest.FrameworkRemovedBeforeReregistration test.

[Vinod Kone]

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

Ship it!



src/tests/master_authorization_tests.cpp


Maybe just say the same thing that was said in the earlier test (#910) for 
consistency?

// Pause the clock to avoid scheduler registration retries.


- Vinod Kone


On Jan. 29, 2015, 11:13 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30432/
> ---
> 
> (Updated Jan. 29, 2015, 11:13 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2306
> https://issues.apache.org/jira/browse/MESOS-2306
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> From MESOS-2306:
> 
> There is a race here where the scheduler retries registration an extra time 
> in the failing test. This then messes up the mock expectations:
> 
> ```
>   // Return a pending future from authorizer after first attempt.
>   Future authorize2;
>   Promise promise2;
>   EXPECT_CALL(authorizer, authorize(An mesos::ACL::RegisterFramework&>()))
> .WillOnce(Return(true))
> .WillOnce(DoAll(FutureSatisfy(&authorize2),
> Return(promise2.future(
> .WillRepeatedly(Return(true)); // Authorize subsequent registration 
> retries.
> ```
> 
> We want to simulate a master detection and capture the authorization, but 
> when there is a spurious retry, our simulated retry falls into the 
> WillRepeatedly case and is authorized immediately.
> 
> 
> Diffs
> -
> 
>   src/tests/master_authorization_tests.cpp 
> 42ffe24685b859fb16132d22a932f6637fe9b5c3 
> 
> Diff: https://reviews.apache.org/r/30432/diff/
> 
> 
> Testing
> ---
> 
> 500 iterations of this test
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 30432: Fixed the flaky MasterAuthorizationTest.FrameworkRemovedBeforeReregistration test.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [30432]

All tests passed.

- Mesos ReviewBot


On Jan. 29, 2015, 11:13 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30432/
> ---
> 
> (Updated Jan. 29, 2015, 11:13 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2306
> https://issues.apache.org/jira/browse/MESOS-2306
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> From MESOS-2306:
> 
> There is a race here where the scheduler retries registration an extra time 
> in the failing test. This then messes up the mock expectations:
> 
> ```
>   // Return a pending future from authorizer after first attempt.
>   Future authorize2;
>   Promise promise2;
>   EXPECT_CALL(authorizer, authorize(An mesos::ACL::RegisterFramework&>()))
> .WillOnce(Return(true))
> .WillOnce(DoAll(FutureSatisfy(&authorize2),
> Return(promise2.future(
> .WillRepeatedly(Return(true)); // Authorize subsequent registration 
> retries.
> ```
> 
> We want to simulate a master detection and capture the authorization, but 
> when there is a spurious retry, our simulated retry falls into the 
> WillRepeatedly case and is authorized immediately.
> 
> 
> Diffs
> -
> 
>   src/tests/master_authorization_tests.cpp 
> 42ffe24685b859fb16132d22a932f6637fe9b5c3 
> 
> Diff: https://reviews.apache.org/r/30432/diff/
> 
> 
> Testing
> ---
> 
> 500 iterations of this test
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Review Request 30439: Increased the default AWAIT timeout.

[Ben Mahler]

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

Review request for mesos and Vinod Kone.


Repository: mesos-git


Description
---

I've observed a number of CI VM stalls that lead to test failures, this is an 
attempt to mitigate some of the flaky tests in the face of VM stalls.


Diffs
-

  3rdparty/libprocess/include/process/gtest.hpp 
10f991d4e6a6ec8479f342ee812faa3f5c3870bb 

Diff: https://reviews.apache.org/r/30439/diff/


Testing
---

make check


Thanks,

Ben Mahler

Re: Review Request 30439: Increased the default AWAIT timeout.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [30439]

All tests passed.

- Mesos ReviewBot


On Jan. 30, 2015, 2:12 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30439/
> ---
> 
> (Updated Jan. 30, 2015, 2:12 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> I've observed a number of CI VM stalls that lead to test failures, this is an 
> attempt to mitigate some of the flaky tests in the face of VM stalls.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/gtest.hpp 
> 10f991d4e6a6ec8479f342ee812faa3f5c3870bb 
> 
> Diff: https://reviews.apache.org/r/30439/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Review Request 30441: Query string encode / decode

[Cody Maloney]

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

Review request for mesos and Benjamin Hindman.


Repository: mesos-git


Description
---

Query string encode / decode


Diffs
-

  3rdparty/libprocess/include/process/http.hpp 
9cf05acbb724ab9af8010d1788621d37a0e48e86 
  3rdparty/libprocess/src/decoder.hpp 816846436cd8710c476902146c4df06f09aa9da1 
  3rdparty/libprocess/src/http.cpp 869b205656fb73edb9f02a1856d10f79ed1349b4 
  3rdparty/libprocess/src/tests/http_tests.cpp 
c71a91d516214d20d3df4c220e4a39a070db3260 

Diff: https://reviews.apache.org/r/30441/diff/


Testing
---

make distcheck on Ubuntu 14.04 for the series


Thanks,

Cody Maloney

Review Request 30442: Update mesos for query string encode/decode

[Cody Maloney]

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

Review request for mesos and Benjamin Hindman.


Repository: mesos-git


Description
---

Update mesos for query string encode/decode


Diffs
-

  src/master/http.cpp 3981b18cb82d3b8bd974b80d27f14c304898a43c 
  src/tests/repair_tests.cpp 5235058263d661dfa6445b0b443af6290e2ea016 

Diff: https://reviews.apache.org/r/30442/diff/


Testing
---

make distcheck on Ubuntu 14.04


Thanks,

Cody Maloney