Re: Review Request 39325: Fixed race between coordinator election and recovery in replicated log.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39325]

All tests passed.

- Mesos ReviewBot


On Oct. 15, 2015, 1:21 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39325/
> ---
> 
> (Updated Oct. 15, 2015, 1:21 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Joris Van Remoortere, and Timothy Chen.
> 
> 
> Bugs: MESOS-3280
> https://issues.apache.org/jira/browse/MESOS-3280
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3280. The basic problem is that replicas silently ignore inbound Promise
> and Write requests if they have not finished the recovery protocol yet 
> (because
> they can't safely vote on such requests). Hence, if we try to do a Paxos round
> while a quorum of nodes have not finished recovering, the Paxos round will 
> never
> complete. In particular, this might happen during coordinator election:
> coordinator election (which is implemented as performing a full Paxos round)
> starts as soon as the candidate coordinator replica has finished the recovery
> protocol. If several nodes start concurrently, a quorum of those nodes might
> still be executing the recovery protocol, and hence the coordinator will never
> be elected.
> 
> To address this, add "ignored" responses to the Promise and Write 
> sub-protocols:
> if a proposer sees a quorum of "ignored" responses to a promise or write 
> request
> it has issued, it knows the request will never succeed.  When used for
> coordinator election, the current coding will retry immediately (without a
> backoff).
> 
> Note that replicas will still silently drop promise/write requests if another
> kind of problem occurs (e.g., an I/O error prevents reading/writing log
> data). We might consider changing this, although it will require some thought:
> e.g., if a replica's disk is broken, sending an "ignored" message on every
> request might flood the network.
> 
> CODE REVIEW TO DISCUSS / FIX:
> 
> * Test mock is incredibly ugly: it works, but we clearly need a better 
> approach
>   before committing this. I've been chatting with @tnachen to find a better
>   approach but haven't got anything that works yet.
> 
> * Should we add a backoff when retrying after a failed coordinator election?
> 
> * Should we also send back an "ignored" response if an I/O error occurs?
> 
> 
> Diffs
> -
> 
>   src/log/consensus.cpp 59f80d02d1d3c11683631f3fc5f6e923b5ebdf96 
>   src/log/coordinator.cpp 5500bca77f3020e0051010c5c178a20a3c7ad44a 
>   src/log/replica.hpp 33d3f1d9e89035936c67739898e73a06b391fcd0 
>   src/log/replica.cpp 75d39ff56822bf00fce9daf5c1e3befb75f2e039 
>   src/messages/log.proto d73b33f865963292af580945659ad0e800f2a204 
>   src/state/log.cpp a75a605a4b0edb8863a3378e2133df7d6eb1cc3d 
>   src/tests/log_tests.cpp f2dd47cfbe73fb18c360a637db009b7d391a782e 
>   src/tests/slave_tests.cpp 10a4fa7eaa8e868ccc6d60ac220d66a4f0a523b4 
> 
> Diff: https://reviews.apache.org/r/39325/diff/
> 
> 
> Testing
> ---
> 
> "make check" passes, including a new test that uses a newly constructed mock 
> to ensure we're testing the message schedule described above.
> 
> I also wrote a script stops and starts mesos-master in a loop, removing the 
> replicated log each time. Without the patch, this occasionally fails with a 
> "registry fetch" timeout; with the patch, you can observe several scenarios 
> where coordinator election is reborted and retried because a quorum of 
> ignored responses is seen. Note that in some cases, we need to retry 
> coordinator election up to ~70 times (!), because we don't currently use a 
> backoff; that should probably be fixed, per comments above. But the important 
> point is that election eventually succeeds and we don't hang.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 39323: Fixed syntax error in sed usage on OSX.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39323]

All tests passed.

- Mesos ReviewBot


On Oct. 14, 2015, 11:41 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39323/
> ---
> 
> (Updated Oct. 14, 2015, 11:41 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3734
> https://issues.apache.org/jira/browse/MESOS-3734
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> 5d246ce4e2d40b adopted sed syntax that works on Linux but does not work on
> OSX. Apparently there is not a portable way to instruct sed not to make a 
> backup
> file; hence, we now create a backup file and then immediately delete it.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 2bf40f27733b4362d64679bf594e15e82f47155d 
> 
> Diff: https://reviews.apache.org/r/39323/diff/
> 
> 
> Testing
> ---
> 
> Successfully built on OSX.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 39312: Hierarchical Allocator: Replaced Polymorphic factory with functions.

[Michael Park]

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

Ship it!


Ship It!

- Michael Park


On Oct. 14, 2015, 9:15 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39312/
> ---
> 
> (Updated Oct. 14, 2015, 9:15 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Michael Park.
> 
> 
> Bugs: MESOS-3554
> https://issues.apache.org/jira/browse/MESOS-3554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> e468b5ae61e828bf0d1d670582dcf8a6c9fad5a8 
>   src/master/allocator/mesos/hierarchical.cpp 
> 0a6f8a67b9a8a0e6d17e4aa0bc3b55cf4787cf01 
> 
> Diff: https://reviews.apache.org/r/39312/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 39325: Fixed race between coordinator election and recovery in replicated log.

[Neil Conway]

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

(Updated Oct. 15, 2015, 1:21 a.m.)


Review request for mesos, Jie Yu, Joris Van Remoortere, and Timothy Chen.


Changes
---

Fixed Linux compile errors in test code.


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


Repository: mesos


Description
---

MESOS-3280. The basic problem is that replicas silently ignore inbound Promise
and Write requests if they have not finished the recovery protocol yet (because
they can't safely vote on such requests). Hence, if we try to do a Paxos round
while a quorum of nodes have not finished recovering, the Paxos round will never
complete. In particular, this might happen during coordinator election:
coordinator election (which is implemented as performing a full Paxos round)
starts as soon as the candidate coordinator replica has finished the recovery
protocol. If several nodes start concurrently, a quorum of those nodes might
still be executing the recovery protocol, and hence the coordinator will never
be elected.

To address this, add "ignored" responses to the Promise and Write sub-protocols:
if a proposer sees a quorum of "ignored" responses to a promise or write request
it has issued, it knows the request will never succeed.  When used for
coordinator election, the current coding will retry immediately (without a
backoff).

Note that replicas will still silently drop promise/write requests if another
kind of problem occurs (e.g., an I/O error prevents reading/writing log
data). We might consider changing this, although it will require some thought:
e.g., if a replica's disk is broken, sending an "ignored" message on every
request might flood the network.

CODE REVIEW TO DISCUSS / FIX:

* Test mock is incredibly ugly: it works, but we clearly need a better approach
  before committing this. I've been chatting with @tnachen to find a better
  approach but haven't got anything that works yet.

* Should we add a backoff when retrying after a failed coordinator election?

* Should we also send back an "ignored" response if an I/O error occurs?


Diffs (updated)
-

  src/log/consensus.cpp 59f80d02d1d3c11683631f3fc5f6e923b5ebdf96 
  src/log/coordinator.cpp 5500bca77f3020e0051010c5c178a20a3c7ad44a 
  src/log/replica.hpp 33d3f1d9e89035936c67739898e73a06b391fcd0 
  src/log/replica.cpp 75d39ff56822bf00fce9daf5c1e3befb75f2e039 
  src/messages/log.proto d73b33f865963292af580945659ad0e800f2a204 
  src/state/log.cpp a75a605a4b0edb8863a3378e2133df7d6eb1cc3d 
  src/tests/log_tests.cpp f2dd47cfbe73fb18c360a637db009b7d391a782e 
  src/tests/slave_tests.cpp 10a4fa7eaa8e868ccc6d60ac220d66a4f0a523b4 

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


Testing
---

"make check" passes, including a new test that uses a newly constructed mock to 
ensure we're testing the message schedule described above.

I also wrote a script stops and starts mesos-master in a loop, removing the 
replicated log each time. Without the patch, this occasionally fails with a 
"registry fetch" timeout; with the patch, you can observe several scenarios 
where coordinator election is reborted and retried because a quorum of ignored 
responses is seen. Note that in some cases, we need to retry coordinator 
election up to ~70 times (!), because we don't currently use a backoff; that 
should probably be fixed, per comments above. But the important point is that 
election eventually succeeds and we don't hang.


Thanks,

Neil Conway

Re: Review Request 39331: Support docker local store pull image simultaneously

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39331]

All tests passed.

- Mesos ReviewBot


On Oct. 14, 2015, 10:54 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39331/
> ---
> 
> (Updated Oct. 14, 2015, 10:54 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3736
> https://issues.apache.org/jira/browse/MESOS-3736
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support docker local store pull image simultaneously
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/metadata_manager.cpp 
> 2b2de5245bccbd01a856b214ac6525278d794537 
>   src/slave/containerizer/provisioner/docker/store.cpp 
> 637c97c0973bda492826803a962c99635647692d 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 9c3c45a81be6398722a37911788e347a4e91cce8 
> 
> Diff: https://reviews.apache.org/r/39331/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 39325: Fixed race between coordinator election and recovery in replicated log.

[Jie Yu]

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


Haven't looked into details yet. Some high level questions below.


src/log/replica.cpp (line 209)


We typically do the cleanup in a separate patch to reduce the size for the 
main patch (easiser for reviewers to review).



src/log/replica.cpp (lines 363 - 364)


Given that we want to handle this in the future, does it make sense to use 
a 'type' field in the response message and deprecate 'okay' field?

Having two boolean field 'okay' and 'ignore' becomes semantically wiered 
when okay = true and ignore = true.

```
message PromiseResponse {
  // To be deprecated.
  required bool okay;
  
  message Type {
ACCEPT,
REJECT,
IGNORE,
ERROR,
  }
  optional Type type;
}
```



src/messages/log.proto (line 148)


I am just thinking about the rolling upgrade case. What happens if the old 
coordinator receives a response from a new replica. According to the current 
semantics, it'll be treated as a NACK. Is that OK? Will that unnecessarily 
demote the old coordinator?

If you think that's OK, please add a comment about why it is OK (e.g., 
it'll eventually succeed when all replicas/coordinator are updated). If that's 
the case, we definitely need to call out this in upgrades.md.

Ditto for the 'ignored' field in WriteResponse.


- Jie Yu


On Oct. 14, 2015, 8:29 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39325/
> ---
> 
> (Updated Oct. 14, 2015, 8:29 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Joris Van Remoortere, and Timothy Chen.
> 
> 
> Bugs: MESOS-3280
> https://issues.apache.org/jira/browse/MESOS-3280
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3280. The basic problem is that replicas silently ignore inbound Promise
> and Write requests if they have not finished the recovery protocol yet 
> (because
> they can't safely vote on such requests). Hence, if we try to do a Paxos round
> while a quorum of nodes have not finished recovering, the Paxos round will 
> never
> complete. In particular, this might happen during coordinator election:
> coordinator election (which is implemented as performing a full Paxos round)
> starts as soon as the candidate coordinator replica has finished the recovery
> protocol. If several nodes start concurrently, a quorum of those nodes might
> still be executing the recovery protocol, and hence the coordinator will never
> be elected.
> 
> To address this, add "ignored" responses to the Promise and Write 
> sub-protocols:
> if a proposer sees a quorum of "ignored" responses to a promise or write 
> request
> it has issued, it knows the request will never succeed.  When used for
> coordinator election, the current coding will retry immediately (without a
> backoff).
> 
> Note that replicas will still silently drop promise/write requests if another
> kind of problem occurs (e.g., an I/O error prevents reading/writing log
> data). We might consider changing this, although it will require some thought:
> e.g., if a replica's disk is broken, sending an "ignored" message on every
> request might flood the network.
> 
> CODE REVIEW TO DISCUSS / FIX:
> 
> * Test mock is incredibly ugly: it works, but we clearly need a better 
> approach
>   before committing this. I've been chatting with @tnachen to find a better
>   approach but haven't got anything that works yet.
> 
> * Should we add a backoff when retrying after a failed coordinator election?
> 
> * Should we also send back an "ignored" response if an I/O error occurs?
> 
> 
> Diffs
> -
> 
>   src/log/consensus.cpp 59f80d02d1d3c11683631f3fc5f6e923b5ebdf96 
>   src/log/coordinator.cpp 5500bca77f3020e0051010c5c178a20a3c7ad44a 
>   src/log/replica.hpp 33d3f1d9e89035936c67739898e73a06b391fcd0 
>   src/log/replica.cpp 75d39ff56822bf00fce9daf5c1e3befb75f2e039 
>   src/messages/log.proto d73b33f865963292af580945659ad0e800f2a204 
>   src/state/log.cpp a75a605a4b0edb8863a3378e2133df7d6eb1cc3d 
>   src/tests/log_tests.cpp f2dd47cfbe73fb18c360a637db009b7d391a782e 
>   src/tests/slave_tests.cpp 10a4fa7eaa8e868ccc6d60ac220d66a4f0a523b4 
> 
> Diff: https://reviews.apache.org/r/39325/diff/
> 
> 
> Testing
> ---
> 
> "make check" passes, including a new test that uses a newly constructed mock 
> to ensure we're testing the message schedule described above.
> 
> I also wrote a script stops and starts mesos-master in a loop, removing

Re: Review Request 39330: libprocess: Replaced usage of "volatile" with std::atomic.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39330]

All tests passed.

- Mesos ReviewBot


On Oct. 14, 2015, 10:29 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39330/
> ---
> 
> (Updated Oct. 14, 2015, 10:29 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3326
> https://issues.apache.org/jira/browse/MESOS-3326
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See MESOS-3326. We adopted std::atomic in most of the code base earlier 
> (commits
> 87de003c6e8a, 4b938052b6af, and 4a01850c5540), but a few places were omitted;
> those locations are fixed by this commit.
> 
> There's one remaining place to improve: we use the GCC intrinsic
> __sync_synchronize() in 3rdparty/libprocess/include/process/logging.h. Because
> that is used to protect modifications to the FLAGS_v variable defined by glog,
> we can't easily adapt it to use std::atomic.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/owned.hpp 
> bc5b527152c8864544ad58070c0bfc81639056da 
>   3rdparty/libprocess/include/process/shared.hpp 
> 021807b961bb55f11c9e04327135bd83f4d86c21 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> e5277de5b5bdea4a44606cda7fbf69a559aeebbe 
> 
> Diff: https://reviews.apache.org/r/39330/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 39323: Fixed syntax error in sed usage on OSX.

[James Peach]

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

Ship it!


Ship It!

- James Peach


On Oct. 14, 2015, 11:41 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39323/
> ---
> 
> (Updated Oct. 14, 2015, 11:41 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3734
> https://issues.apache.org/jira/browse/MESOS-3734
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> 5d246ce4e2d40b adopted sed syntax that works on Linux but does not work on
> OSX. Apparently there is not a portable way to instruct sed not to make a 
> backup
> file; hence, we now create a backup file and then immediately delete it.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 2bf40f27733b4362d64679bf594e15e82f47155d 
> 
> Diff: https://reviews.apache.org/r/39323/diff/
> 
> 
> Testing
> ---
> 
> Successfully built on OSX.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 39323: Fixed syntax error in sed usage on OSX.

[James Peach]

> On Oct. 14, 2015, 11:24 p.m., James Peach wrote:
> > Automake detects sed, so I think that you should do something like this:
> > 
> > SED_I = $(SED) -i.orig
> > 
> > foo: bar
> > $(AM_V_GEN)$(SED_I) 's/mesos.mesos_pb2/mesos_pb2/' $@
> 
> Neil Conway wrote:
> I suppose there's value in using $(SED) instead of "sed". Using 
> $(AM_V_GEN) when running some commands but not others (e.g., protoc) seems 
> inconsistent to me, though.
> 
> I'm inclined to replace "sed" with $(SED) and leave it at that (since it 
> would be nice to unbreak the build); we can consider further cleanup 
> separately.

That's fair enough. Just for fun, here's a diff with pretty PROTOC output 
http://fpaste.org/279307/48670631/ :)


- James


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


On Oct. 14, 2015, 11:41 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39323/
> ---
> 
> (Updated Oct. 14, 2015, 11:41 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3734
> https://issues.apache.org/jira/browse/MESOS-3734
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> 5d246ce4e2d40b adopted sed syntax that works on Linux but does not work on
> OSX. Apparently there is not a portable way to instruct sed not to make a 
> backup
> file; hence, we now create a backup file and then immediately delete it.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 2bf40f27733b4362d64679bf594e15e82f47155d 
> 
> Diff: https://reviews.apache.org/r/39323/diff/
> 
> 
> Testing
> ---
> 
> Successfully built on OSX.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 39323: Fixed syntax error in sed usage on OSX.

[James Peach]

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

Ship it!


Ship It!

- James Peach


On Oct. 14, 2015, 11:41 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39323/
> ---
> 
> (Updated Oct. 14, 2015, 11:41 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3734
> https://issues.apache.org/jira/browse/MESOS-3734
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> 5d246ce4e2d40b adopted sed syntax that works on Linux but does not work on
> OSX. Apparently there is not a portable way to instruct sed not to make a 
> backup
> file; hence, we now create a backup file and then immediately delete it.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 2bf40f27733b4362d64679bf594e15e82f47155d 
> 
> Diff: https://reviews.apache.org/r/39323/diff/
> 
> 
> Testing
> ---
> 
> Successfully built on OSX.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 38734: Cleaned up function signatures to use Option.

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Sept. 24, 2015, 9:27 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38734/
> ---
> 
> (Updated Sept. 24, 2015, 9:27 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also make use of delegating constructors to reduce redundancy in the master
> detector and master contender.
> 
> 
> Diffs
> -
> 
>   src/master/contender.hpp 927601c73f3ba56cafb366502a5f58a71c581837 
>   src/master/contender.cpp 67562f19754bd57e293b62d0b7547ff2e6cf18ad 
>   src/master/detector.hpp d31ce532e0b26569e521b21893ef91d771fc20bc 
>   src/master/detector.cpp d0d10e58cc32f5ba9da1bd35a66dcbf5c204 
>   src/master/main.cpp bafc605d6c20bd264b932e44ee80373a3f692734 
> 
> Diff: https://reviews.apache.org/r/38734/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 39312: Hierarchical Allocator: Replaced Polymorphic factory with functions.

[Guangya Liu]

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

Ship it!


Ship It!

- Guangya Liu


On Oct. 14, 2015, 9:15 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39312/
> ---
> 
> (Updated Oct. 14, 2015, 9:15 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Michael Park.
> 
> 
> Bugs: MESOS-3554
> https://issues.apache.org/jira/browse/MESOS-3554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> e468b5ae61e828bf0d1d670582dcf8a6c9fad5a8 
>   src/master/allocator/mesos/hierarchical.cpp 
> 0a6f8a67b9a8a0e6d17e4aa0bc3b55cf4787cf01 
> 
> Diff: https://reviews.apache.org/r/39312/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 39323: Fixed syntax error in sed usage on OSX.

[Neil Conway]

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

(Updated Oct. 14, 2015, 11:41 p.m.)


Review request for mesos and Joris Van Remoortere.


Changes
---

Replace "sed" with $(SED)


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


Repository: mesos


Description
---

5d246ce4e2d40b adopted sed syntax that works on Linux but does not work on
OSX. Apparently there is not a portable way to instruct sed not to make a backup
file; hence, we now create a backup file and then immediately delete it.


Diffs (updated)
-

  src/Makefile.am 2bf40f27733b4362d64679bf594e15e82f47155d 

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


Testing
---

Successfully built on OSX.


Thanks,

Neil Conway

Re: Review Request 39323: Fixed syntax error in sed usage on OSX.

[Neil Conway]

> On Oct. 14, 2015, 11:24 p.m., James Peach wrote:
> > Automake detects sed, so I think that you should do something like this:
> > 
> > SED_I = $(SED) -i.orig
> > 
> > foo: bar
> > $(AM_V_GEN)$(SED_I) 's/mesos.mesos_pb2/mesos_pb2/' $@

I suppose there's value in using $(SED) instead of "sed". Using $(AM_V_GEN) 
when running some commands but not others (e.g., protoc) seems inconsistent to 
me, though.

I'm inclined to replace "sed" with $(SED) and leave it at that (since it would 
be nice to unbreak the build); we can consider further cleanup separately.


- Neil


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


On Oct. 14, 2015, 6:26 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39323/
> ---
> 
> (Updated Oct. 14, 2015, 6:26 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3734
> https://issues.apache.org/jira/browse/MESOS-3734
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> 5d246ce4e2d40b adopted sed syntax that works on Linux but does not work on
> OSX. Apparently there is not a portable way to instruct sed not to make a 
> backup
> file; hence, we now create a backup file and then immediately delete it.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 2bf40f27733b4362d64679bf594e15e82f47155d 
> 
> Diff: https://reviews.apache.org/r/39323/diff/
> 
> 
> Testing
> ---
> 
> Successfully built on OSX.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 39197: Provider tests: minor style fixes.

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On Oct. 9, 2015, 11:49 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39197/
> ---
> 
> (Updated Oct. 9, 2015, 11:49 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provider tests: minor style fixes.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/39197/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 37955: Remove hashmap::existsValue since it is never called

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Aug. 31, 2015, 3:42 p.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37955/
> ---
> 
> (Updated Aug. 31, 2015, 3:42 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3329
> https://issues.apache.org/jira/browse/MESOS-3329
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Remove hashmap::existsValue since it is never called
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp 
> f09bea125035aa3621402b83608b233e42877559 
> 
> Diff: https://reviews.apache.org/r/37955/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 39323: Fixed syntax error in sed usage on OSX.

[James Peach]

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


Automake detects sed, so I think that you should do something like this:

SED_I = $(SED) -i.orig

foo: bar
$(AM_V_GEN)$(SED_I) 's/mesos.mesos_pb2/mesos_pb2/' $@

- James Peach


On Oct. 14, 2015, 6:26 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39323/
> ---
> 
> (Updated Oct. 14, 2015, 6:26 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3734
> https://issues.apache.org/jira/browse/MESOS-3734
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> 5d246ce4e2d40b adopted sed syntax that works on Linux but does not work on
> OSX. Apparently there is not a portable way to instruct sed not to make a 
> backup
> file; hence, we now create a backup file and then immediately delete it.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 2bf40f27733b4362d64679bf594e15e82f47155d 
> 
> Diff: https://reviews.apache.org/r/39323/diff/
> 
> 
> Testing
> ---
> 
> Successfully built on OSX.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 39018: Added JSON parsing for Resources.

[Michael Park]

> On Oct. 13, 2015, 5 p.m., Michael Park wrote:
> > src/common/resources.cpp, line 361
> > 
> >
> > I think it would be better to only support one way of doing things 
> > here. I would opt for the array format. Is supporting both formats 
> > explicitly a feature?
> 
> Greg Mann wrote:
> No, it hasn't been explicitly called out as a feature. Why the desire to 
> allow only one format? My thought was that any valid JSON string describing 
> one or more Resource objects should return successfully, but I suppose there 
> isn't too much need to accommodate a single resource object, since at the 
> command line users will always be specifying multiple resources.
> 
> Michael Park wrote:
> > Why the desire to allow only one format?
> 
> (1) Generally speaking, it's good to limit the number of ways to do the 
> same thing so that there's less to remember as the user
> (2) In this specific case, it can actually be confusing/ambiguous.
> That is, does this expect a single `Resource` object: `{ "name" : 
> "cpus", ... }`, or
> does it expect a `Credentials`-like object where we specify something 
> like: `{ "resources": { "name" : "cpus", ... }, { "name" : "disk", ... }, ... 
> }`?
> (I know now that it does the first one, but at first glance I 
> actually thought it does the second)
> 
> Greg Mann wrote:
> Yea, I see your point with #1. Regarding #2, that is interesting... I 
> went with a JSON array in this implementation due to the discussion on the 
> JIRA ticket, but honestly I was unaware of the format of the `Credentials` 
> JSON input. In order to make the formats more consistent, we could use the 
> same form here: `{"resources": [{"name": "cpus", ...}, {"name": "disk"}, 
> ...]}`. This would seem to be more in line with your point #1, keeping the 
> interface simple for the user. Thoughts?

Yeah, except I think we're moving away from the `Credentials`-like JSON input. 
The `JSON::Array` parse feature was only recently introduced.
For example, Joseph initially introduced `MachineIDs` but got rid of it once 
the `JSON::Array` parse function was introduced. 
[r38011](https://reviews.apache.org/r/38011)


- Michael


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


On Oct. 14, 2015, 3:50 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated Oct. 14, 2015, 3:50 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes code changes necessary for JSON parsing of Resources. 
> Documentation changes will be posted soon in another review 
> (https://reviews.apache.org/r/39102/).
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
>   include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39018/diff/
> 
> 
> Testing
> ---
> 
> New code was added to `ResourcesTest.ParsingFromJSON`, and two new tests were 
> added: `ResourcesTest.ParsingFromJSONWithRoles` and 
> `ResourcesTest.ParsingFromJSONError`. These attempt to test common error 
> scenarios that might show up in user-supplied JSON.
> 
> `make check` was used to confirm that all tests pass, with one notable 
> failure (ResourcesTest.ParsingFromJSONError) related to a picojson issue 
> tracked here: https://issues.apache.org/jira/browse/MESOS-3698
> 
> The original resources parsing format is used throughout many other tests 
> (check `src/tests/sorter_tests.cpp`, for example), so it's clear that the 
> original parsing continues to work correctly.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 39331: Support docker local store pull image simultaneously

[Gilbert Song]

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

Review request for mesos, Anand Mazumdar, Jojy Varghese, and Timothy Chen.


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


Repository: mesos


Description
---

Support docker local store pull image simultaneously


Diffs
-

  src/slave/containerizer/provisioner/docker/metadata_manager.cpp 
2b2de5245bccbd01a856b214ac6525278d794537 
  src/slave/containerizer/provisioner/docker/store.cpp 
637c97c0973bda492826803a962c99635647692d 
  src/tests/containerizer/provisioner_docker_tests.cpp 
9c3c45a81be6398722a37911788e347a4e91cce8 

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


Testing
---

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song

Re: Review Request 29551: Added "tests" target for make.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [29550, 29551]

All tests passed.

- Mesos ReviewBot


On Oct. 14, 2015, 6:31 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29551/
> ---
> 
> (Updated Oct. 14, 2015, 6:31 p.m.)
> 
> 
> Review request for mesos, Cody Maloney, Joris Van Remoortere, Timothy Chen, 
> and Timothy St. Clair.
> 
> 
> Bugs: MESOS-2273
> https://issues.apache.org/jira/browse/MESOS-2273
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> 'make check' allows one to build and run the test suite. However,
> 'make tests' allows one to build the test suite without running it.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 7c244a053044ae347f9505cb78a71a6636e26586 
>   Makefile.am cb289b4f21516c3027429d8f8ba72d799f975bd4 
>   docs/submitting-a-patch.md cbd99fe3fd16270316596700e3fc292243a9 
>   src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 
> 
> Diff: https://reviews.apache.org/r/29551/diff/
> 
> 
> Testing
> ---
> 
> Ran 'make tests'.
> 
> For added test, ran "make clean; make; make tests" followed by "make check" 
> and verified that no additional target are build in the "make check" step.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Review Request 39330: libprocess: Replaced usage of "volatile" with std::atomic.

[Neil Conway]

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

Review request for mesos and Joris Van Remoortere.


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


Repository: mesos


Description
---

See MESOS-3326. We adopted std::atomic in most of the code base earlier (commits
87de003c6e8a, 4b938052b6af, and 4a01850c5540), but a few places were omitted;
those locations are fixed by this commit.

There's one remaining place to improve: we use the GCC intrinsic
__sync_synchronize() in 3rdparty/libprocess/include/process/logging.h. Because
that is used to protect modifications to the FLAGS_v variable defined by glog,
we can't easily adapt it to use std::atomic.


Diffs
-

  3rdparty/libprocess/include/process/owned.hpp 
bc5b527152c8864544ad58070c0bfc81639056da 
  3rdparty/libprocess/include/process/shared.hpp 
021807b961bb55f11c9e04327135bd83f4d86c21 
  3rdparty/libprocess/src/tests/process_tests.cpp 
e5277de5b5bdea4a44606cda7fbf69a559aeebbe 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 39323: Fixed syntax error in sed usage on OSX.

[Gilbert Song]

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

Ship it!


Ship It!

- Gilbert Song


On Oct. 14, 2015, 11:26 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39323/
> ---
> 
> (Updated Oct. 14, 2015, 11:26 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3734
> https://issues.apache.org/jira/browse/MESOS-3734
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> 5d246ce4e2d40b adopted sed syntax that works on Linux but does not work on
> OSX. Apparently there is not a portable way to instruct sed not to make a 
> backup
> file; hence, we now create a backup file and then immediately delete it.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 2bf40f27733b4362d64679bf594e15e82f47155d 
> 
> Diff: https://reviews.apache.org/r/39323/diff/
> 
> 
> Testing
> ---
> 
> Successfully built on OSX.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 39258: Add filesystem isolator with command executor test.

[Jie Yu]

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



src/slave/containerizer/provisioner/provisioner.hpp (line 101)


Can you move this comment up at the class level.

```
// Expose this class for testing only.
```



src/tests/containerizer/filesystem_isolator_tests.cpp (lines 100 - 102)


Instead of doing that, can we combine two 'createContainerizer' methods 
into one? In other words, let's always use the real provisioner with TestStore. 
Let's still keep the existing createContainerizer interface (i.e., taking a 
hashmap from image name to rootfs path).



src/tests/containerizer/filesystem_isolator_tests.cpp (lines 280 - 306)


You should be able to get rid of the logics here by doing what I suggested 
above.


- Jie Yu


On Oct. 13, 2015, 3:40 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39258/
> ---
> 
> (Updated Oct. 13, 2015, 3:40 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add filesystem isolator with command executor test.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 
>   src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 912fc5abadb1219fc4baec1a751010522bc7a7d0 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 30a85a52ee5986c6e1652edfd08ae881280b23ab 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 1e332e0d36ac2812456514aa030f995b3a07dca1 
>   src/tests/containerizer/store.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39258/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 39324: Updated protobuf::parse to handle JSON "null" values.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39324]

All tests passed.

- Mesos ReviewBot


On Oct. 14, 2015, 6:31 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39324/
> ---
> 
> (Updated Oct. 14, 2015, 6:31 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows us to handle JSON objects that have some fields set to "null".
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
> 2285ce9eaba668d5215c108849055fe92163da4d 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
> 68328a2d9bb61cb3b832c1136dfe9a77f8421768 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 
> 8ebb798f06f89f89f412da0490615c8ae03c90b6 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 
> 34eb6d04822ad043a3f5aebe531b34c0a9809155 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto 
> 920f5c98b74513c374dac4673fcb445e30beef61 
> 
> Diff: https://reviews.apache.org/r/39324/diff/
> 
> 
> Testing
> ---
> 
> Added a new tests in stout::protobuf_tests and ran make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 39323: Fixed syntax error in sed usage on OSX.

[Isabel Jimenez]

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

Ship it!


Ship It!

- Isabel Jimenez


On Oct. 14, 2015, 6:26 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39323/
> ---
> 
> (Updated Oct. 14, 2015, 6:26 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3734
> https://issues.apache.org/jira/browse/MESOS-3734
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> 5d246ce4e2d40b adopted sed syntax that works on Linux but does not work on
> OSX. Apparently there is not a portable way to instruct sed not to make a 
> backup
> file; hence, we now create a backup file and then immediately delete it.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 2bf40f27733b4362d64679bf594e15e82f47155d 
> 
> Diff: https://reviews.apache.org/r/39323/diff/
> 
> 
> Testing
> ---
> 
> Successfully built on OSX.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 38747: Adding digest utilities

[Jojy Varghese]

> On Oct. 13, 2015, 12:54 a.m., Jiang Yan Xu wrote:
> > 3rdparty/libprocess/include/process/digest.hpp, line 71
> > 
> >
> > I imagine `fn` is expensive, after all, it's  cryptography. :)
> > 
> > Serially doing read and compute seems to be suboptimal but I simply 
> > parallelize read and compute can cause OOM when read is faster than 
> > compute. Of couse this is a classic producer and consumer problem and we 
> > can use a queue. We probably don't want to make things too complicated here 
> > in a single review but again, this is the issue of reimplemting a generic 
> > and widely used utility: we don't get to leverage state of the art for 
> > free...
> > 
> > Just a comment.

Added comment on preliminary performance test.


- Jojy


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


On Oct. 13, 2015, 7:21 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> ---
> 
> (Updated Oct. 13, 2015, 7:21 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Gilbert Song, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added SHA256, SHA512 implementation.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am eb7393904988afc0bc5e9f7dadd545e0d6c04e5f 
>   3rdparty/libprocess/include/Makefile.am 
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
>   3rdparty/libprocess/include/process/digest.hpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> a14b5b8fe22d3e75bef3c716ae7865ddaf132927 
>   3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38747/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 39325: Fixed race between coordinator election and recovery in replicated log.

[Neil Conway]

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

(Updated Oct. 14, 2015, 8:29 p.m.)


Review request for mesos, Jie Yu, Joris Van Remoortere, and Timothy Chen.


Changes
---

Address code review nit.


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


Repository: mesos


Description
---

MESOS-3280. The basic problem is that replicas silently ignore inbound Promise
and Write requests if they have not finished the recovery protocol yet (because
they can't safely vote on such requests). Hence, if we try to do a Paxos round
while a quorum of nodes have not finished recovering, the Paxos round will never
complete. In particular, this might happen during coordinator election:
coordinator election (which is implemented as performing a full Paxos round)
starts as soon as the candidate coordinator replica has finished the recovery
protocol. If several nodes start concurrently, a quorum of those nodes might
still be executing the recovery protocol, and hence the coordinator will never
be elected.

To address this, add "ignored" responses to the Promise and Write sub-protocols:
if a proposer sees a quorum of "ignored" responses to a promise or write request
it has issued, it knows the request will never succeed.  When used for
coordinator election, the current coding will retry immediately (without a
backoff).

Note that replicas will still silently drop promise/write requests if another
kind of problem occurs (e.g., an I/O error prevents reading/writing log
data). We might consider changing this, although it will require some thought:
e.g., if a replica's disk is broken, sending an "ignored" message on every
request might flood the network.

CODE REVIEW TO DISCUSS / FIX:

* Test mock is incredibly ugly: it works, but we clearly need a better approach
  before committing this. I've been chatting with @tnachen to find a better
  approach but haven't got anything that works yet.

* Should we add a backoff when retrying after a failed coordinator election?

* Should we also send back an "ignored" response if an I/O error occurs?


Diffs (updated)
-

  src/log/consensus.cpp 59f80d02d1d3c11683631f3fc5f6e923b5ebdf96 
  src/log/coordinator.cpp 5500bca77f3020e0051010c5c178a20a3c7ad44a 
  src/log/replica.hpp 33d3f1d9e89035936c67739898e73a06b391fcd0 
  src/log/replica.cpp 75d39ff56822bf00fce9daf5c1e3befb75f2e039 
  src/messages/log.proto d73b33f865963292af580945659ad0e800f2a204 
  src/state/log.cpp a75a605a4b0edb8863a3378e2133df7d6eb1cc3d 
  src/tests/log_tests.cpp f2dd47cfbe73fb18c360a637db009b7d391a782e 
  src/tests/slave_tests.cpp 10a4fa7eaa8e868ccc6d60ac220d66a4f0a523b4 

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


Testing
---

"make check" passes, including a new test that uses a newly constructed mock to 
ensure we're testing the message schedule described above.

I also wrote a script stops and starts mesos-master in a loop, removing the 
replicated log each time. Without the patch, this occasionally fails with a 
"registry fetch" timeout; with the patch, you can observe several scenarios 
where coordinator election is reborted and retried because a quorum of ignored 
responses is seen. Note that in some cases, we need to retry coordinator 
election up to ~70 times (!), because we don't currently use a backoff; that 
should probably be fixed, per comments above. But the important point is that 
election eventually succeeds and we don't hang.


Thanks,

Neil Conway

Re: Review Request 39323: Fixed syntax error in sed usage on OSX.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39323]

All tests passed.

- Mesos ReviewBot


On Oct. 14, 2015, 6:26 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39323/
> ---
> 
> (Updated Oct. 14, 2015, 6:26 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3734
> https://issues.apache.org/jira/browse/MESOS-3734
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> 5d246ce4e2d40b adopted sed syntax that works on Linux but does not work on
> OSX. Apparently there is not a portable way to instruct sed not to make a 
> backup
> file; hence, we now create a backup file and then immediately delete it.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 2bf40f27733b4362d64679bf594e15e82f47155d 
> 
> Diff: https://reviews.apache.org/r/39323/diff/
> 
> 
> Testing
> ---
> 
> Successfully built on OSX.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 39325: Fixed race between coordinator election and recovery in replicated log.

[Artem Harutyunyan]

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



src/tests/log_tests.cpp (line 730)


micro-nit: period is missing.


- Artem Harutyunyan


On Oct. 14, 2015, 11:33 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39325/
> ---
> 
> (Updated Oct. 14, 2015, 11:33 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Joris Van Remoortere, and Timothy Chen.
> 
> 
> Bugs: MESOS-3280
> https://issues.apache.org/jira/browse/MESOS-3280
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3280. The basic problem is that replicas silently ignore inbound Promise
> and Write requests if they have not finished the recovery protocol yet 
> (because
> they can't safely vote on such requests). Hence, if we try to do a Paxos round
> while a quorum of nodes have not finished recovering, the Paxos round will 
> never
> complete. In particular, this might happen during coordinator election:
> coordinator election (which is implemented as performing a full Paxos round)
> starts as soon as the candidate coordinator replica has finished the recovery
> protocol. If several nodes start concurrently, a quorum of those nodes might
> still be executing the recovery protocol, and hence the coordinator will never
> be elected.
> 
> To address this, add "ignored" responses to the Promise and Write 
> sub-protocols:
> if a proposer sees a quorum of "ignored" responses to a promise or write 
> request
> it has issued, it knows the request will never succeed.  When used for
> coordinator election, the current coding will retry immediately (without a
> backoff).
> 
> Note that replicas will still silently drop promise/write requests if another
> kind of problem occurs (e.g., an I/O error prevents reading/writing log
> data). We might consider changing this, although it will require some thought:
> e.g., if a replica's disk is broken, sending an "ignored" message on every
> request might flood the network.
> 
> CODE REVIEW TO DISCUSS / FIX:
> 
> * Test mock is incredibly ugly: it works, but we clearly need a better 
> approach
>   before committing this. I've been chatting with @tnachen to find a better
>   approach but haven't got anything that works yet.
> 
> * Should we add a backoff when retrying after a failed coordinator election?
> 
> * Should we also send back an "ignored" response if an I/O error occurs?
> 
> 
> Diffs
> -
> 
>   src/log/consensus.cpp 59f80d02d1d3c11683631f3fc5f6e923b5ebdf96 
>   src/log/coordinator.cpp 5500bca77f3020e0051010c5c178a20a3c7ad44a 
>   src/log/replica.hpp 33d3f1d9e89035936c67739898e73a06b391fcd0 
>   src/log/replica.cpp 75d39ff56822bf00fce9daf5c1e3befb75f2e039 
>   src/messages/log.proto d73b33f865963292af580945659ad0e800f2a204 
>   src/state/log.cpp a75a605a4b0edb8863a3378e2133df7d6eb1cc3d 
>   src/tests/log_tests.cpp f2dd47cfbe73fb18c360a637db009b7d391a782e 
>   src/tests/slave_tests.cpp 10a4fa7eaa8e868ccc6d60ac220d66a4f0a523b4 
> 
> Diff: https://reviews.apache.org/r/39325/diff/
> 
> 
> Testing
> ---
> 
> "make check" passes, including a new test that uses a newly constructed mock 
> to ensure we're testing the message schedule described above.
> 
> I also wrote a script stops and starts mesos-master in a loop, removing the 
> replicated log each time. Without the patch, this occasionally fails with a 
> "registry fetch" timeout; with the patch, you can observe several scenarios 
> where coordinator election is reborted and retried because a quorum of 
> ignored responses is seen. Note that in some cases, we need to retry 
> coordinator election up to ~70 times (!), because we don't currently use a 
> backoff; that should probably be fixed, per comments above. But the important 
> point is that election eventually succeeds and we don't hang.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 38900: Update command executor to support rootfs.

[Jie Yu]

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


Only one issue (regarding user). See my detailed comments below.


src/launcher/executor.cpp (line 187)


Could you please add some comments about what we are doing here:

```
// If 'sandbox_diretory' is specified, that means the user
// task specifies a root filesystem, and that root filesystem has
// already been prepared at COMMAND_EXECUTOR_ROOTFS_CONTAINER_PATH.
// The command executor is reponsible for mounting the sandbox
// into the root filesystem, chrooting into it and changing the
// user before exec-ing the user process.
```



src/launcher/executor.cpp (lines 203 - 212)


This loop looks like unnecessary. Can you simply do:
```
rootfs = path::join(os::getcwd(), COMMAND_EXECUTOR_ROOTFS_CONTAINER_PATH);
```



src/launcher/executor.cpp (line 218)


Failed to create sandbox mount point at '...': 



src/launcher/executor.cpp (line 224)


Please add a NOTE here saying that this is a non-recursive bind mount. You 
don't want to recursively mount root filesystem into root filesystem since the 
root filesystem is under sandbox as well.



src/launcher/executor.cpp (line 227)


this should be 'sandbox'?



src/launcher/executor.cpp (line 306)


Hum, looks like no one will use 'rootfs' variable on non linux platforms. 
WIll that trigger compiler warning/error?

So should you put #ifdef within the if block and abort if it's not linux?



src/slave/containerizer/mesos/containerizer.cpp (lines 592 - 597)


WHy the change here? Can you do that in a separate patch?



src/slave/containerizer/mesos/containerizer.cpp 


Ditto here.



src/slave/slave.cpp (lines 3390 - 3393)


Hum... Does that mean that if the user task does not specify a user (e.g., 
rely on framework.user), we are going to launch the task under root?


- Jie Yu


On Oct. 13, 2015, 3:39 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38900/
> ---
> 
> (Updated Oct. 13, 2015, 3:39 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3428
> https://issues.apache.org/jira/browse/MESOS-3428
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update command executor to support rootfs.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
>   src/slave/constants.hpp 1197268576ee2ec37601db75ea9536ec09882886 
>   src/slave/constants.cpp 96dadce6d28aa585410f50a2509a34445f8dcf82 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 4aad8a3be43b331efc6b8157b2fae090df16c1b4 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d1fc5a460e7313828014eea999cf4e63dde01921 
>   src/slave/slave.cpp 6b25b49458c163097a5292843134363c4d0f5e6f 
> 
> Diff: https://reviews.apache.org/r/38900/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38978: Add post-rewrite hook.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38978]

All tests passed.

- Mesos ReviewBot


On Oct. 14, 2015, 5:10 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38978/
> ---
> 
> (Updated Oct. 14, 2015, 5:10 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2953
> https://issues.apache.org/jira/browse/MESOS-2953
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add post-rewrite hook.
> 
> 
> Diffs
> -
> 
>   bootstrap 779b33cdcb88b2417769bb046a17b47cd6042f2d 
>   support/hooks/post-rewrite PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38978/diff/
> 
> 
> Testing
> ---
> 
> test manually
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 29550: Libprocess: Added "tests" target for make.

[Kapil Arya]

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

(Updated Oct. 14, 2015, 2:31 p.m.)


Review request for mesos, Cody Maloney, Joris Van Remoortere, Timothy Chen, and 
Timothy St. Clair.


Changes
---

Fixed dependency order for libprocess-tests.


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


Repository: mesos


Description
---

'make check' allows one to build and run the test suite. However,
'make tests' allows one to build the test suite without running it.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/Makefile.am 
e64e3d9e958fab3c7c25fad17dcc2b279d255500 
  3rdparty/libprocess/Makefile.am eb7393904988afc0bc5e9f7dadd545e0d6c04e5f 

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


Testing
---

Ran make tests.

For added test, ran "make clean; make; make tests" followed by "make check" and 
verified that no additional target are build in the "make check" step.


Thanks,

Kapil Arya

Review Request 39325: Fixed race between coordinator election and recovery in replicated log.

[Neil Conway]

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

Review request for mesos, Jie Yu, Joris Van Remoortere, and Timothy Chen.


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


Repository: mesos


Description
---

MESOS-3280. The basic problem is that replicas silently ignore inbound Promise
and Write requests if they have not finished the recovery protocol yet (because
they can't safely vote on such requests). Hence, if we try to do a Paxos round
while a quorum of nodes have not finished recovering, the Paxos round will never
complete. In particular, this might happen during coordinator election:
coordinator election (which is implemented as performing a full Paxos round)
starts as soon as the candidate coordinator replica has finished the recovery
protocol. If several nodes start concurrently, a quorum of those nodes might
still be executing the recovery protocol, and hence the coordinator will never
be elected.

To address this, add "ignored" responses to the Promise and Write sub-protocols:
if a proposer sees a quorum of "ignored" responses to a promise or write request
it has issued, it knows the request will never succeed.  When used for
coordinator election, the current coding will retry immediately (without a
backoff).

Note that replicas will still silently drop promise/write requests if another
kind of problem occurs (e.g., an I/O error prevents reading/writing log
data). We might consider changing this, although it will require some thought:
e.g., if a replica's disk is broken, sending an "ignored" message on every
request might flood the network.

CODE REVIEW TO DISCUSS / FIX:

* Test mock is incredibly ugly: it works, but we clearly need a better approach
  before committing this. I've been chatting with @tnachen to find a better
  approach but haven't got anything that works yet.

* Should we add a backoff when retrying after a failed coordinator election?

* Should we also send back an "ignored" response if an I/O error occurs?


Diffs
-

  src/log/consensus.cpp 59f80d02d1d3c11683631f3fc5f6e923b5ebdf96 
  src/log/coordinator.cpp 5500bca77f3020e0051010c5c178a20a3c7ad44a 
  src/log/replica.hpp 33d3f1d9e89035936c67739898e73a06b391fcd0 
  src/log/replica.cpp 75d39ff56822bf00fce9daf5c1e3befb75f2e039 
  src/messages/log.proto d73b33f865963292af580945659ad0e800f2a204 
  src/state/log.cpp a75a605a4b0edb8863a3378e2133df7d6eb1cc3d 
  src/tests/log_tests.cpp f2dd47cfbe73fb18c360a637db009b7d391a782e 
  src/tests/slave_tests.cpp 10a4fa7eaa8e868ccc6d60ac220d66a4f0a523b4 

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


Testing
---

"make check" passes, including a new test that uses a newly constructed mock to 
ensure we're testing the message schedule described above.

I also wrote a script stops and starts mesos-master in a loop, removing the 
replicated log each time. Without the patch, this occasionally fails with a 
"registry fetch" timeout; with the patch, you can observe several scenarios 
where coordinator election is reborted and retried because a quorum of ignored 
responses is seen. Note that in some cases, we need to retry coordinator 
election up to ~70 times (!), because we don't currently use a backoff; that 
should probably be fixed, per comments above. But the important point is that 
election eventually succeeds and we don't hang.


Thanks,

Neil Conway

Re: Review Request 39312: Hierarchical Allocator: Replaced Polymorphic factory with functions.

[Ben Mahler]

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

Ship it!


Thank you! Yes, this seems simpler to me :)

- Ben Mahler


On Oct. 14, 2015, 9:15 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39312/
> ---
> 
> (Updated Oct. 14, 2015, 9:15 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Michael Park.
> 
> 
> Bugs: MESOS-3554
> https://issues.apache.org/jira/browse/MESOS-3554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> e468b5ae61e828bf0d1d670582dcf8a6c9fad5a8 
>   src/master/allocator/mesos/hierarchical.cpp 
> 0a6f8a67b9a8a0e6d17e4aa0bc3b55cf4787cf01 
> 
> Diff: https://reviews.apache.org/r/39312/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Review Request 39324: Updated protobuf::parse to handle JSON "null" values.

[Kapil Arya]

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

Review request for mesos, Ben Mahler and Jie Yu.


Repository: mesos


Description
---

This allows us to handle JSON objects that have some fields set to "null".


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
2285ce9eaba668d5215c108849055fe92163da4d 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
68328a2d9bb61cb3b832c1136dfe9a77f8421768 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 
8ebb798f06f89f89f412da0490615c8ae03c90b6 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 
34eb6d04822ad043a3f5aebe531b34c0a9809155 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto 
920f5c98b74513c374dac4673fcb445e30beef61 

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


Testing
---

Added a new tests in stout::protobuf_tests and ran make check.


Thanks,

Kapil Arya

Re: Review Request 29551: Added "tests" target for make.

[Kapil Arya]

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

(Updated Oct. 14, 2015, 2:31 p.m.)


Review request for mesos, Cody Maloney, Joris Van Remoortere, Timothy Chen, and 
Timothy St. Clair.


Changes
---

rebased


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


Repository: mesos


Description
---

'make check' allows one to build and run the test suite. However,
'make tests' allows one to build the test suite without running it.


Diffs (updated)
-

  3rdparty/Makefile.am 7c244a053044ae347f9505cb78a71a6636e26586 
  Makefile.am cb289b4f21516c3027429d8f8ba72d799f975bd4 
  docs/submitting-a-patch.md cbd99fe3fd16270316596700e3fc292243a9 
  src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 

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


Testing
---

Ran 'make tests'.

For added test, ran "make clean; make; make tests" followed by "make check" and 
verified that no additional target are build in the "make check" step.


Thanks,

Kapil Arya

Re: Review Request 39310: Enable /master/state-summary endpoint include active flag for framework

[Ben Mahler]

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

Ship it!


Ship It!

- Ben Mahler


On Oct. 14, 2015, 8:03 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39310/
> ---
> 
> (Updated Oct. 14, 2015, 8:03 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3721
> https://issues.apache.org/jira/browse/MESOS-3721
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enable /master/state-summary endpoint include active flag for framework
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 3ca3725302cc831b067a2f69edbda95cd30c2520 
> 
> Diff: https://reviews.apache.org/r/39310/diff/
> 
> 
> Testing
> ---
> 
> gyliu@devstack007:~$ curl "http://9.111.242.187:5050/master/state-summary"; 
> 2>/dev/null| jq .
> {
>   "slaves": [
> {
>   "used_resources": {
> "mem": 256,
> "disk": 0,
> "cpus": 1
>   },
>   "unreserved_resources": {
> "ports": "[31000-32000]",
> "mem": 2808,
> "disk": 1399183,
> "cpus": 4
>   },
>   "resources": {
> "ports": "[31000-32000]",
> "mem": 2808,
> "disk": 1399183,
> "cpus": 4
>   },
>   "reserved_resources": {},
>   "reregistered_time": 1444809313.03239,
>   "TASK_STARTING": 0,
>   "TASK_STAGING": 0,
>   "TASK_RUNNING": 1,
>   "TASK_LOST": 0,
>   "TASK_KILLED": 0,
>   "TASK_FINISHED": 0,
>   "TASK_FAILED": 0,
>   "TASK_ERROR": 0,
>   "active": true,
>   "attributes": {},
>   "framework_ids": [
> "ec7faf83-1838-4909-9a96-4e30a6757967-"
>   ],
>   "hostname": "devstack007.cn.ibm.com",
>   "id": "2a7e51b2-20d2-4476-a3e3-8c3207e7427a-S0",
>   "offered_resources": {
> "mem": 0,
> "disk": 0,
> "cpus": 0
>   },
>   "pid": "slave(1)@9.111.242.187:5051",
>   "registered_time": 1444809313.03176
> }
>   ],
>   "hostname": "devstack007.cn.ibm.com",
>   "frameworks": [
> {
>   "webui_url": "",
>   "used_resources": {
> "mem": 256,
> "disk": 0,
> "cpus": 1
>   },
>   "TASK_STARTING": 0,
>   "TASK_STAGING": 0,
>   "TASK_RUNNING": 1,
>   "TASK_LOST": 0,
>   "TASK_KILLED": 0,
>   "TASK_FINISHED": 0,
>   "TASK_FAILED": 0,
>   "TASK_ERROR": 0,
>   "active": true,  Here
>   "capabilities": [],
>   "hostname": "devstack007.cn.ibm.com",
>   "id": "ec7faf83-1838-4909-9a96-4e30a6757967-",
>   "name": "",
>   "offered_resources": {
> "mem": 0,
> "disk": 0,
> "cpus": 0
>   },
>   "pid": 
> "scheduler-b68990a9-aed9-4182-bacb-456cbb7302d6@9.111.242.187:34782",
>   "slave_ids": [
> "2a7e51b2-20d2-4476-a3e3-8c3207e7427a-S0"
>   ]
> }
>   ]
> }
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 39323: Fixed syntax error in sed usage on OSX.

[Neil Conway]

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

(Updated Oct. 14, 2015, 6:26 p.m.)


Review request for mesos and Joris Van Remoortere.


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


Repository: mesos


Description (updated)
---

5d246ce4e2d40b adopted sed syntax that works on Linux but does not work on
OSX. Apparently there is not a portable way to instruct sed not to make a backup
file; hence, we now create a backup file and then immediately delete it.


Diffs
-

  src/Makefile.am 2bf40f27733b4362d64679bf594e15e82f47155d 

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


Testing
---

Successfully built on OSX.


Thanks,

Neil Conway

Re: Review Request 39323: Fixed syntax error in sed usage on OSX.

[Neil Conway]

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

(Updated Oct. 14, 2015, 6:25 p.m.)


Review request for mesos and Joris Van Remoortere.


Changes
---

Fix typo


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


Repository: mesos


Description
---

5d246ce4e2d40b adopted sed syntax that works on Linux but does not work on
OSX. Apparently there is not a portable way to instruct sed not to make a backup
file; hence, we know create a backup file and then immediately delete it.


Diffs (updated)
-

  src/Makefile.am 2bf40f27733b4362d64679bf594e15e82f47155d 

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


Testing
---

Successfully built on OSX.


Thanks,

Neil Conway

Re: Review Request 39323: Fixed syntax error in sed usage on OSX.

[haosdent huang]

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

Ship it!


Ship It!

- haosdent huang


On Oct. 14, 2015, 6:14 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39323/
> ---
> 
> (Updated Oct. 14, 2015, 6:14 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3734
> https://issues.apache.org/jira/browse/MESOS-3734
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> 5d246ce4e2d40b adopted sed syntax that works on Linux but does not work on
> OSX. Apparently there is not a portable way to instruct sed not to make a 
> backup
> file; hence, we know create a backup file and then immediately delete it.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 2bf40f27733b4362d64679bf594e15e82f47155d 
> 
> Diff: https://reviews.apache.org/r/39323/diff/
> 
> 
> Testing
> ---
> 
> Successfully built on OSX.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 39323: Fixed syntax error in sed usage on OSX.

[Neil Conway]

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

Review request for mesos and Joris Van Remoortere.


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


Repository: mesos


Description
---

5d246ce4e2d40b adopted sed syntax that works on Linux but does not work on
OSX. Apparently there is not a portable way to instruct sed not to make a backup
file; hence, we know create a backup file and then immediately delete it.


Diffs
-

  src/Makefile.am 2bf40f27733b4362d64679bf594e15e82f47155d 

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


Testing
---

Successfully built on OSX.


Thanks,

Neil Conway

Re: Review Request 39289: [WIP] Quota: Added authorization of quota requests.

[Jan Schlicht]

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

(Updated Oct. 14, 2015, 8:04 p.m.)


Review request for mesos and Alexander Rukletsov.


Changes
---

Rebased.


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


Repository: mesos


Description
---

[WIP] Added authorization of quota requests.


Diffs (updated)
-

  docs/authorization.md 2a5e74782754e446184a297f91112e9f94077896 
  include/mesos/authorizer/authorizer.hpp 
d667a52f90f970a313580446a5a006cec4b5e25b 
  include/mesos/authorizer/authorizer.proto 
86bbb45f9d91b4098a262e3e50a793f3bb39497e 
  include/mesos/quota/quota.proto d2e3a45735e4ebcf257682556aff5075e6e3bf79 
  src/authorizer/local/authorizer.hpp 32de102fd588f029882ef121ca83a7410c65 
  src/authorizer/local/authorizer.cpp 6d7da87731a438c2180cf91003e09d4aa5a1c773 
  src/master/flags.cpp 0285ce70cefca09e81ef7137968d024e297fec87 
  src/master/master.hpp e7b16fdd21a8caa77a39956a8520cf1381186598 
  src/master/quota_handler.cpp PRE-CREATION 
  src/tests/mesos.hpp 3e58b454c75a2ab9f8b4a29785fa823afefd0c8a 

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


Testing
---


Thanks,

Jan Schlicht

Re: Review Request 39288: [WIP] Quota: Added authentication of quota requests.

[Jan Schlicht]

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

(Updated Oct. 14, 2015, 8:03 p.m.)


Review request for mesos and Alexander Rukletsov.


Changes
---

Rebased.


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


Repository: mesos


Description
---

[WIP] Added authentication of quota requests.


Diffs (updated)
-

  src/master/master.hpp e7b16fdd21a8caa77a39956a8520cf1381186598 
  src/master/quota_handler.cpp PRE-CREATION 

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


Testing
---


Thanks,

Jan Schlicht

Re: Review Request 39320: Speeded up the test by reducing the allocation timeout.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [39320]

Failed command: ./support/apply-review.sh -n -r 39320

Error:
 2015-10-14 17:52:55 URL:https://reviews.apache.org/r/39320/diff/raw/ [751/751] 
-> "39320.patch" [1]
error: patch failed: src/tests/fault_tolerance_tests.cpp:716
error: src/tests/fault_tolerance_tests.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Oct. 14, 2015, 4:29 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39320/
> ---
> 
> (Updated Oct. 14, 2015, 4:29 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3732
> https://issues.apache.org/jira/browse/MESOS-3732
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/fault_tolerance_tests.cpp 
> c97bc4691f9bac4a8677e6d2247be96ee9674b57 
> 
> Diff: https://reviews.apache.org/r/39320/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39018: Added JSON parsing for Resources.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39211, 39018]

All tests passed.

- Mesos ReviewBot


On Oct. 14, 2015, 3:50 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated Oct. 14, 2015, 3:50 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes code changes necessary for JSON parsing of Resources. 
> Documentation changes will be posted soon in another review 
> (https://reviews.apache.org/r/39102/).
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
>   include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39018/diff/
> 
> 
> Testing
> ---
> 
> New code was added to `ResourcesTest.ParsingFromJSON`, and two new tests were 
> added: `ResourcesTest.ParsingFromJSONWithRoles` and 
> `ResourcesTest.ParsingFromJSONError`. These attempt to test common error 
> scenarios that might show up in user-supplied JSON.
> 
> `make check` was used to confirm that all tests pass, with one notable 
> failure (ResourcesTest.ParsingFromJSONError) related to a picojson issue 
> tracked here: https://issues.apache.org/jira/browse/MESOS-3698
> 
> The original resources parsing format is used throughout many other tests 
> (check `src/tests/sorter_tests.cpp`, for example), so it's clear that the 
> original parsing continues to work correctly.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 38978: Add post-rewrite hook.

[haosdent huang]

> On Oct. 14, 2015, 1:27 p.m., Joris Van Remoortere wrote:
> > support/hooks/post-rewrite, line 14
> > 
> >
> > How come you chose not to follow the whitespace check in the existing 
> > pre-commit hook?
> 
> haosdent huang wrote:
> I seem mesos-style.py already check whitespace, so I don't add it. Should 
> I add it back?

Have already add it. :-)


- haosdent


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


On Oct. 14, 2015, 5:10 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38978/
> ---
> 
> (Updated Oct. 14, 2015, 5:10 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2953
> https://issues.apache.org/jira/browse/MESOS-2953
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add post-rewrite hook.
> 
> 
> Diffs
> -
> 
>   bootstrap 779b33cdcb88b2417769bb046a17b47cd6042f2d 
>   support/hooks/post-rewrite PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38978/diff/
> 
> 
> Testing
> ---
> 
> test manually
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38978: Add post-rewrite hook.

[haosdent huang]

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

(Updated Oct. 14, 2015, 5:10 p.m.)


Review request for mesos and Joris Van Remoortere.


Changes
---

Add whitespace check from pre-commit, and test again.


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


Repository: mesos


Description
---

Add post-rewrite hook.


Diffs (updated)
-

  bootstrap 779b33cdcb88b2417769bb046a17b47cd6042f2d 
  support/hooks/post-rewrite PRE-CREATION 

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


Testing
---

test manually


Thanks,

haosdent huang

Re: Review Request 38978: Add post-rewrite hook.

[haosdent huang]

> On Oct. 14, 2015, 1:27 p.m., Joris Van Remoortere wrote:
> > support/hooks/post-rewrite, line 14
> > 
> >
> > How come you chose not to follow the whitespace check in the existing 
> > pre-commit hook?

I seem mesos-style.py already check whitespace, so I don't add it. Should I add 
it back?


- haosdent


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


On Oct. 14, 2015, 4:55 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38978/
> ---
> 
> (Updated Oct. 14, 2015, 4:55 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2953
> https://issues.apache.org/jira/browse/MESOS-2953
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add post-rewrite hook.
> 
> 
> Diffs
> -
> 
>   bootstrap 779b33cdcb88b2417769bb046a17b47cd6042f2d 
>   support/hooks/post-rewrite PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38978/diff/
> 
> 
> Testing
> ---
> 
> test manually
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38978: Add post-rewrite hook.

[haosdent huang]

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

(Updated Oct. 14, 2015, 4:55 p.m.)


Review request for mesos and Joris Van Remoortere.


Changes
---

Add comments for @~..@


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


Repository: mesos


Description
---

Add post-rewrite hook.


Diffs (updated)
-

  bootstrap 779b33cdcb88b2417769bb046a17b47cd6042f2d 
  support/hooks/post-rewrite PRE-CREATION 

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


Testing
---

test manually


Thanks,

haosdent huang

Re: Review Request 38570: Change documentation image links to absolute paths.

[Joseph Wu]

> On Oct. 6, 2015, 2:29 p.m., Ben Mahler wrote:
> > docs/external-containerizer.md, line 92
> > 
> >
> > How will this work when we add multiple versions of documents to the 
> > website? Now this hardcodes "latest" as the version?
> 
> Joseph Wu wrote:
> I'm not sure.
> 
> From what I've seen of the website generation code, we currently wipe out 
> older versions of the docs during generation.  We'd first have to change the 
> site's Rakefile (and probably the release process) to keep older versions.
> 
> After that, we'd still have to deal with the problem of relative URLs and 
> trailing slashes:
> 
> http://stackoverflow.com/questions/5457885/relative-urls-and-trailing-slashes
> 
> Perhaps we could change the markdown parser to replace image links with a 
> versioned absolute path.  (I'm not sure what that would entail.)
> 
> OR
> 
> We could change all the image links to relative links (i.e. 
> `images/ec_launch_seqdiag.png`) and remove trailing slashes from all 
> documentation links.  (Maybe add redirects from trailing slash pages to 
> non-trailing slash pages.)
> 
> 
> What do you think?
> 
> Niklas Nielsen wrote:
> BenM: ping ^^
> 
> Ben Mahler wrote:
> Removing the trailing slashes on documentation links sounds good to me. 
> Also, why did you remove raw=true, was it a no-op, or?
> 
> Joseph Wu wrote:
> I'll sync with Dave L. or Jake F. about the trailing slashes.
> 
> The `raw=true` isn't necessary anymore (I believe).  In the past, I think 
> it was necessary for images to show up on the Github previews, but now images 
> show with or without the `raw=true`.
> (For example, the newer docs with images do not have `raw=true`, but they 
> show up on the previews).
> 
> Niklas Nielsen wrote:
> @joseph; any update?

I'm going to discard this while we (Dave + Jake + I) figure out the changes to 
the website.
Most likely, we'll want to keep the existing relative paths.


- Joseph


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


On Oct. 6, 2015, 9:45 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38570/
> ---
> 
> (Updated Oct. 6, 2015, 9:45 a.m.)
> 
> 
> Review request for mesos, Adam B, Dave Lester, Artem Harutyunyan, Kapil Arya, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3183
> https://issues.apache.org/jira/browse/MESOS-3183
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also removes extraneous `?raw=true` from the links.
> 
> 
> Diffs
> -
> 
>   docs/external-containerizer.md 96932189b74dce1ae28e0bd73d5543d1afaffb0b 
>   docs/fetcher-cache-internals.md e8a68d1230420d1afca61f92c1ab6be12a70dbf2 
>   docs/maintenance.md a5831ffa092a9ea6decbe2e640bae637c759a308 
>   docs/networking-for-mesos-managed-containers.md 
> 33568a8233523ff3805f962371c94346e608208b 
>   docs/oversubscription.md 5a31b1ff7f003307817732a71f3b0a7c7d60cd24 
> 
> Diff: https://reviews.apache.org/r/38570/diff/
> 
> 
> Testing
> ---
> 
> Patched `Rakefile` to include images in the website (See JIRA for the patch).
> Then rendered with: https://github.com/mesosphere/mesos-website-container
> 
> Confirmed that images show up on the modified docs.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 38883: Removed calls to apply-review.sh script. Added support for amending commit messages.

[Marco Massenzio]

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


Again lots of nit-picking, feel free to ignore what you disagree with strongly 
:)

Also, instead of using your homemade `shell()` method and `Subprocess`, given 
the usage pattern I've noticed, you may want to consider the `sh` module:
http://amoffat.github.io/sh/

once you start using it, it gets pretty awesome!


support/apply-reviews.py (line 29)


always best to prefer `str.join()` to join strings; in this case, however, 
as you need to terminate with the `/` it's best to use `format()`:
```
return "{url}/{user}/".format(url=USER_URL, user=user)
```
of course:
```
'/'.join([USER_URL, user, ''])
```
would have also been acceptable.



support/apply-reviews.py (line 34)


same here, please



support/apply-reviews.py (line 40)


(setting aside for a second that we should use `requests.get()` instead :)

can you please that you get a 200 OK code and that there is actually JSON 
content?

with `requests` this is as easy as:
```
import requests

...
try:
headers = {'AcceptContent': "application/json"}
r = requests.get(url, timeout=50, headers=headers)
if 200 <= r.status_code < 300 and r.headers.get('content-type') == 
'application/json':
return r.json()
else:
# do something with error
pass
except requestsTimeoutError:
# log the error and probably give up, the server is down
pass
```



support/apply-reviews.py (line 59)


this will throw a KeyError if either of the keys are missing (and the user 
will be none the wiser):
```
if 'review_request' in json_obj and not 
json_obj['review_request].get('depends_on'):
  ... do something
```

(also see my comments in the other review about the lines above this)



support/apply-reviews.py (line 78)


```
from __future__ import print_function:

...

print(output)
```
(this way it'll all work also for those us using a modern Python 
interpreter :) )

thanks!



support/apply-reviews.py (line 86)


please don't do this - prefer the use of `**kwargs` or, in this case, it's 
much easier to do something like:
```
def remove_patch(review_id, options=None):
options = options or {'dry_run': False}
...
```



support/apply-reviews.py (line 89)


consider using the `sh` module:
```
import sh

cmd = "{rev_id}.patch".format(rev_id=review_id)
sh.rm("-f", cmd)

```



support/apply-reviews.py (line 93)


can you please explains what is `options` supposed to be / contain?
I think you expect a `dict` here, it would be great to have something 
like...

```
""" blah blah

@param options: these are the fuz bits that god the baz, in a map that 
can contain the following keys: [`dry_run`, `verbose`, `bitz`] and if not 
specified assumes `dry_run` to be False.
@type options: dict
"""
```



support/apply-reviews.py (line 107)


prefer the use of `str.format()` instead
(here and everywhere else)



support/apply-reviews.py (lines 126 - 130)


I really really don't like this 'leading underscore' and same-naming: makes 
reading the code really difficult and, thanks to Python happily ignoring 
different types, the likelihood of bugs super-high.

also, seems to me that you never use `_review` anywhere else, so that's 
what I'd do:
```
data = url_to_json(review_url(review_id))
if data:
review = data.get('review_request')
else:
# something bad happened bail
```

same goes for `_user` too.



support/apply-reviews.py (lines 133 - 139)


more pythonic:
```
review_data = {
'summary': review['summary'],
'description': review['description'],
'url': review_url(review_id),
'author': user['fullname'],
'email': user['email']
'message': '\n\n'.join([review['summary'],
review['description'],
'Review : {}'.format(review_data['url'])])
}
```


- Marco Massenzio


On Oct. 8, 2015, 6:28 p.m., Ar

Re: Review Request 38570: Change documentation image links to absolute paths.

[Niklas Nielsen]

> On Oct. 6, 2015, 2:29 p.m., Ben Mahler wrote:
> > docs/external-containerizer.md, line 92
> > 
> >
> > How will this work when we add multiple versions of documents to the 
> > website? Now this hardcodes "latest" as the version?
> 
> Joseph Wu wrote:
> I'm not sure.
> 
> From what I've seen of the website generation code, we currently wipe out 
> older versions of the docs during generation.  We'd first have to change the 
> site's Rakefile (and probably the release process) to keep older versions.
> 
> After that, we'd still have to deal with the problem of relative URLs and 
> trailing slashes:
> 
> http://stackoverflow.com/questions/5457885/relative-urls-and-trailing-slashes
> 
> Perhaps we could change the markdown parser to replace image links with a 
> versioned absolute path.  (I'm not sure what that would entail.)
> 
> OR
> 
> We could change all the image links to relative links (i.e. 
> `images/ec_launch_seqdiag.png`) and remove trailing slashes from all 
> documentation links.  (Maybe add redirects from trailing slash pages to 
> non-trailing slash pages.)
> 
> 
> What do you think?
> 
> Niklas Nielsen wrote:
> BenM: ping ^^
> 
> Ben Mahler wrote:
> Removing the trailing slashes on documentation links sounds good to me. 
> Also, why did you remove raw=true, was it a no-op, or?
> 
> Joseph Wu wrote:
> I'll sync with Dave L. or Jake F. about the trailing slashes.
> 
> The `raw=true` isn't necessary anymore (I believe).  In the past, I think 
> it was necessary for images to show up on the Github previews, but now images 
> show with or without the `raw=true`.
> (For example, the newer docs with images do not have `raw=true`, but they 
> show up on the previews).

@joseph; any update?


- Niklas


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


On Oct. 6, 2015, 9:45 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38570/
> ---
> 
> (Updated Oct. 6, 2015, 9:45 a.m.)
> 
> 
> Review request for mesos, Adam B, Dave Lester, Artem Harutyunyan, Kapil Arya, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3183
> https://issues.apache.org/jira/browse/MESOS-3183
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also removes extraneous `?raw=true` from the links.
> 
> 
> Diffs
> -
> 
>   docs/external-containerizer.md 96932189b74dce1ae28e0bd73d5543d1afaffb0b 
>   docs/fetcher-cache-internals.md e8a68d1230420d1afca61f92c1ab6be12a70dbf2 
>   docs/maintenance.md a5831ffa092a9ea6decbe2e640bae637c759a308 
>   docs/networking-for-mesos-managed-containers.md 
> 33568a8233523ff3805f962371c94346e608208b 
>   docs/oversubscription.md 5a31b1ff7f003307817732a71f3b0a7c7d60cd24 
> 
> Diff: https://reviews.apache.org/r/38570/diff/
> 
> 
> Testing
> ---
> 
> Patched `Rakefile` to include images in the website (See JIRA for the patch).
> Then rendered with: https://github.com/mesosphere/mesos-website-container
> 
> Confirmed that images show up on the modified docs.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 39320: Speeded up the test by reducing the allocation timeout.

[Bernd Mathiske]

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



src/tests/fault_tolerance_tests.cpp (line 719)


It may be obvious to many how this speeds up the test, but it would be nice 
to expain the gist in about 1 sentence for everybody else, especially newcomers 
who read such code for the first time.


- Bernd Mathiske


On Oct. 14, 2015, 9:29 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39320/
> ---
> 
> (Updated Oct. 14, 2015, 9:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3732
> https://issues.apache.org/jira/browse/MESOS-3732
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/fault_tolerance_tests.cpp 
> c97bc4691f9bac4a8677e6d2247be96ee9674b57 
> 
> Diff: https://reviews.apache.org/r/39320/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39276]

All tests passed.

- Mesos ReviewBot


On Oct. 14, 2015, 3:35 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39276/
> ---
> 
> (Updated Oct. 14, 2015, 3:35 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Bernd Mathiske, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3705
> https://issues.apache.org/jira/browse/MESOS-3705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When using the same socket to send multiple HTTP requests to different 
> actors. If the actor responsible for handling the first request is stuck 
> handling another event while a subsequent request can reply immediatly, the 
> order of the responses is altered violating HTTP Pipelining.
> 
> This patch fixes that problem enforcing that every response is sent in the 
> order the corresponding request arrived. It also adds a test to reproduce the 
> issue and verify the fix works.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/event.hpp 
> 16ddbd77afa6efdf6bad201aa497ee102aa863ae 
>   3rdparty/libprocess/src/process.cpp 
> 0454554e7b6a39f94cfea02f08ca51ef6b35859a 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d13d3888abbf3db552df4a9f83e54667e598ded9 
> 
> Diff: https://reviews.apache.org/r/39276/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38952: Replaced `> >` with `>>` in fault tolerance tests.

[Bernd Mathiske]

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

Ship it!


Ship It!

- Bernd Mathiske


On Oct. 14, 2015, 8:51 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38952/
> ---
> 
> (Updated Oct. 14, 2015, 8:51 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/fault_tolerance_tests.cpp 
> c97bc4691f9bac4a8677e6d2247be96ee9674b57 
> 
> Diff: https://reviews.apache.org/r/38952/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Review Request 39320: Speeded up the test by reducing the allocation timeout.

[Alexander Rukletsov]

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

Review request for mesos, Benjamin Bannier, Bernd Mathiske, and Till Toenshoff.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/fault_tolerance_tests.cpp c97bc4691f9bac4a8677e6d2247be96ee9674b57 

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


Testing
---

make check


Thanks,

Alexander Rukletsov

Re: Review Request 39068: RegistryClient refactor: Renamed fsLayerInfoList

[Jojy Varghese]

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

(Updated Oct. 14, 2015, 4:07 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased.


Repository: mesos


Description
---

RegistryClient refactor: renamed fsLayerInfoList


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
1d3377e7462908246dfac90aa0c56314406529c9 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
471783d88b73b62afacac3d7952ebb5d5f442097 
  src/tests/containerizer/provisioner_docker_tests.cpp 
9c3c45a81be6398722a37911788e347a4e91cce8 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 39014: RegistryClient refactor: renamed ManifestResponse

[Jojy Varghese]

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

(Updated Oct. 14, 2015, 4:06 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased.


Repository: mesos


Description
---

RegistryClient refactor: renamed ManifestResponse as per review comments.


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
1d3377e7462908246dfac90aa0c56314406529c9 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
471783d88b73b62afacac3d7952ebb5d5f442097 
  src/tests/containerizer/provisioner_docker_tests.cpp 
9c3c45a81be6398722a37911788e347a4e91cce8 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38579: Refactored registry client

[Jojy Varghese]

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

(Updated Oct. 14, 2015, 4:06 p.m.)


Review request for mesos, Ben Mahler and Timothy Chen.


Changes
---

rebased.


Repository: mesos


Description
---

- Cleanup and broke big methods into smaller chunks.


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.cpp 
471783d88b73b62afacac3d7952ebb5d5f442097 
  src/tests/containerizer/provisioner_docker_tests.cpp 
9c3c45a81be6398722a37911788e347a4e91cce8 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 39155: RegistryClient refactor: removed nested namespace references

[Jojy Varghese]

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

(Updated Oct. 14, 2015, 4:05 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased


Repository: mesos


Description
---

RegistryClient refactor: removed nested namespace references


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.cpp 
471783d88b73b62afacac3d7952ebb5d5f442097 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 39156: RegistryClient refactor: changed getManifest interface

[Jojy Varghese]

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

(Updated Oct. 14, 2015, 4:05 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased


Repository: mesos


Description
---

RegistryClient refactor: changed getManifest interface


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
1d3377e7462908246dfac90aa0c56314406529c9 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
471783d88b73b62afacac3d7952ebb5d5f442097 
  src/tests/containerizer/provisioner_docker_tests.cpp 
9c3c45a81be6398722a37911788e347a4e91cce8 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 39018: Added JSON parsing for Resources.

[Greg Mann]

> On Oct. 13, 2015, 5 p.m., Michael Park wrote:
> > src/common/resources.cpp, line 1198
> > 
> >
> > Why the move here?
> 
> Greg Mann wrote:
> I was moving all of the private methods into an appropriately-labeled 
> section. I think I will move some of the previous methods into the 
> implementation as you suggested, so perhaps I will just return this instance 
> of `find()` to its original location.

I went ahead and left `find()` and `_contains()` in this new "Private member 
functions" section, since it matches the organization of the rest of 
resources.cpp. This does move one of the `find()` implementations away from the 
other one; let me know if you think this is an issue.


- Greg


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


On Oct. 14, 2015, 3:50 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated Oct. 14, 2015, 3:50 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes code changes necessary for JSON parsing of Resources. 
> Documentation changes will be posted soon in another review 
> (https://reviews.apache.org/r/39102/).
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
>   include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39018/diff/
> 
> 
> Testing
> ---
> 
> New code was added to `ResourcesTest.ParsingFromJSON`, and two new tests were 
> added: `ResourcesTest.ParsingFromJSONWithRoles` and 
> `ResourcesTest.ParsingFromJSONError`. These attempt to test common error 
> scenarios that might show up in user-supplied JSON.
> 
> `make check` was used to confirm that all tests pass, with one notable 
> failure (ResourcesTest.ParsingFromJSONError) related to a picojson issue 
> tracked here: https://issues.apache.org/jira/browse/MESOS-3698
> 
> The original resources parsing format is used throughout many other tests 
> (check `src/tests/sorter_tests.cpp`, for example), so it's clear that the 
> original parsing continues to work correctly.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39018: Added JSON parsing for Resources.

[Greg Mann]

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

(Updated Oct. 14, 2015, 3:50 p.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael Park.


Changes
---

Addressed comments: moved helper functions to resources.cpp, removed helper to 
convert JSON::Object to Resources.


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


Repository: mesos


Description
---

This includes code changes necessary for JSON parsing of Resources. 
Documentation changes will be posted soon in another review 
(https://reviews.apache.org/r/39102/).


Diffs (updated)
-

  include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
  include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
  src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
  src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
  src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 

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


Testing
---

New code was added to `ResourcesTest.ParsingFromJSON`, and two new tests were 
added: `ResourcesTest.ParsingFromJSONWithRoles` and 
`ResourcesTest.ParsingFromJSONError`. These attempt to test common error 
scenarios that might show up in user-supplied JSON.

`make check` was used to confirm that all tests pass, with one notable failure 
(ResourcesTest.ParsingFromJSONError) related to a picojson issue tracked here: 
https://issues.apache.org/jira/browse/MESOS-3698

The original resources parsing format is used throughout many other tests 
(check `src/tests/sorter_tests.cpp`, for example), so it's clear that the 
original parsing continues to work correctly.


Thanks,

Greg Mann

Re: Review Request 38161: Replaced a hard-coded number for registration backoff with a proper constant.

[Bernd Mathiske]

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

Ship it!


Ship It!

- Bernd Mathiske


On Oct. 14, 2015, 8:51 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38161/
> ---
> 
> (Updated Oct. 14, 2015, 8:51 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prepend a constant regulating the backoff factor with DEFAULT_ prefix for 
> clarity, use it in tests where appropriate and extend comments for posterity.
> 
> 
> Diffs
> -
> 
>   src/sched/constants.hpp ac497b2abc9fbeab713b312cf37963d7de28ed50 
>   src/sched/constants.cpp 517ca5cb0dad49d775d65c9ffabc849ff85347a0 
>   src/sched/flags.hpp 4e0d56f61b6877254161112b288ae4a9c579e846 
>   src/slave/constants.hpp 1197268576ee2ec37601db75ea9536ec09882886 
>   src/slave/constants.cpp 96dadce6d28aa585410f50a2509a34445f8dcf82 
>   src/slave/flags.cpp ac68e1bc51874c7d3bb7e08ba4b5329e8ece0d0e 
>   src/tests/fault_tolerance_tests.cpp 
> c97bc4691f9bac4a8677e6d2247be96ee9674b57 
> 
> Diff: https://reviews.apache.org/r/38161/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39018: Added JSON parsing for Resources.

[Greg Mann]

> On Oct. 13, 2015, 3:35 a.m., Guangya Liu wrote:
> > include/mesos/resources.hpp, lines 114-118
> > 
> >
> > Can this be removed or merged to the under comments?
> 
> Greg Mann wrote:
> I put the comment here so that it wouldn't interfere with the Doxygen 
> documentation, which should be placed directly before the member declaration. 
> I don't want to remove it, because I think the TODO is still valid. I'm open 
> to moving it if there is a better place for it; do you have one in mind?
> 
> Guangya Liu wrote:
> What about moving those comments to /**/ section?
> 
> Greg Mann wrote:
> But then it will show up in the documentation, correct? My motivation for 
> leaving it out of the /**/ section was to exclude this comment from the docs.
> 
> Guangya Liu wrote:
> I think it is ok to show up some TODO in document, thoughts? Thanks.

Yea now that I think of it, the increased visibility of the TODO might even be 
a good thing :-) Moved.


- Greg


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


On Oct. 14, 2015, 3:50 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated Oct. 14, 2015, 3:50 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes code changes necessary for JSON parsing of Resources. 
> Documentation changes will be posted soon in another review 
> (https://reviews.apache.org/r/39102/).
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
>   include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39018/diff/
> 
> 
> Testing
> ---
> 
> New code was added to `ResourcesTest.ParsingFromJSON`, and two new tests were 
> added: `ResourcesTest.ParsingFromJSONWithRoles` and 
> `ResourcesTest.ParsingFromJSONError`. These attempt to test common error 
> scenarios that might show up in user-supplied JSON.
> 
> `make check` was used to confirm that all tests pass, with one notable 
> failure (ResourcesTest.ParsingFromJSONError) related to a picojson issue 
> tracked here: https://issues.apache.org/jira/browse/MESOS-3698
> 
> The original resources parsing format is used throughout many other tests 
> (check `src/tests/sorter_tests.cpp`, for example), so it's clear that the 
> original parsing continues to work correctly.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 38161: Replaced a hard-coded number for registration backoff with a proper constant.

[Alexander Rukletsov]

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

(Updated Oct. 14, 2015, 3:51 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

BenM's comment.


Repository: mesos


Description
---

Prepend a constant regulating the backoff factor with DEFAULT_ prefix for 
clarity, use it in tests where appropriate and extend comments for posterity.


Diffs (updated)
-

  src/sched/constants.hpp ac497b2abc9fbeab713b312cf37963d7de28ed50 
  src/sched/constants.cpp 517ca5cb0dad49d775d65c9ffabc849ff85347a0 
  src/sched/flags.hpp 4e0d56f61b6877254161112b288ae4a9c579e846 
  src/slave/constants.hpp 1197268576ee2ec37601db75ea9536ec09882886 
  src/slave/constants.cpp 96dadce6d28aa585410f50a2509a34445f8dcf82 
  src/slave/flags.cpp ac68e1bc51874c7d3bb7e08ba4b5329e8ece0d0e 
  src/tests/fault_tolerance_tests.cpp c97bc4691f9bac4a8677e6d2247be96ee9674b57 

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


Testing
---

make check (Mac OS 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 38160: Documented how to expedite event firing.

[Alexander Rukletsov]

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

(Updated Oct. 14, 2015, 3:51 p.m.)


Review request for mesos, Bernd Mathiske and Ben Mahler.


Changes
---

Rebased.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  docs/testing-patterns.md d0d92ff569012677043a03412462cf4384096ba9 

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


Testing
---

doc rendered in Markdown


Thanks,

Alexander Rukletsov

Re: Review Request 38952: Replaced `> >` with `>>` in fault tolerance tests.

[Alexander Rukletsov]

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

(Updated Oct. 14, 2015, 3:51 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Rebased.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/fault_tolerance_tests.cpp c97bc4691f9bac4a8677e6d2247be96ee9674b57 

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


Testing
---

make check (Mac OS 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

[Alexander Rojas]

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

(Updated Oct. 14, 2015, 5:35 p.m.)


Review request for mesos, Anand Mazumdar, Bernd Mathiske, and Till Toenshoff.


Changes
---

Updates test.
Fixed some of ben's issues.


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


Repository: mesos


Description
---

When using the same socket to send multiple HTTP requests to different actors. 
If the actor responsible for handling the first request is stuck handling 
another event while a subsequent request can reply immediatly, the order of the 
responses is altered violating HTTP Pipelining.

This patch fixes that problem enforcing that every response is sent in the 
order the corresponding request arrived. It also adds a test to reproduce the 
issue and verify the fix works.


Diffs (updated)
-

  3rdparty/libprocess/include/process/event.hpp 
16ddbd77afa6efdf6bad201aa497ee102aa863ae 
  3rdparty/libprocess/src/process.cpp 0454554e7b6a39f94cfea02f08ca51ef6b35859a 
  3rdparty/libprocess/src/tests/http_tests.cpp 
d13d3888abbf3db552df4a9f83e54667e598ded9 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 36913: Added /quota HTTP Endpoint for Quota handling.

[Joerg Schad]

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

(Updated Oct. 14, 2015, 2:16 p.m.)


Review request for mesos, Alexander Rukletsov, Bernd Mathiske, Alex Clemmer, 
and Joris Van Remoortere.


Changes
---

Rebased on top of 38218.


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


Repository: mesos


Description
---

Added /quota HTTP Endpoint for Quota handling.


Diffs (updated)
-

  src/CMakeLists.txt 98e76cee81ab206f3ffe7989711abc38f49c4352 
  src/Makefile.am 2bf40f27733b4362d64679bf594e15e82f47155d 
  src/master/http.cpp 3ca3725302cc831b067a2f69edbda95cd30c2520 
  src/master/master.hpp e7b16fdd21a8caa77a39956a8520cf1381186598 
  src/master/master.cpp ba12a83b64e9fd02dd4ee7de27c44e0c8d4f03a9 
  src/master/quota_handler.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 36913: Added /quota HTTP Endpoint for Quota handling.

[Joerg Schad]

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

(Updated Oct. 14, 2015, 2:07 p.m.)


Review request for mesos, Alexander Rukletsov, Bernd Mathiske, Alex Clemmer, 
and Joris Van Remoortere.


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


Repository: mesos


Description
---

Added /quota HTTP Endpoint for Quota handling.


Diffs
-

  src/CMakeLists.txt 98e76cee81ab206f3ffe7989711abc38f49c4352 
  src/Makefile.am 4a0eeb89eb2f3fa5e9fff46e4316e39209884343 
  src/master/http.cpp 3ca3725302cc831b067a2f69edbda95cd30c2520 
  src/master/master.hpp e7b16fdd21a8caa77a39956a8520cf1381186598 
  src/master/master.cpp ba12a83b64e9fd02dd4ee7de27c44e0c8d4f03a9 
  src/master/quota_handler.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 36913: Added /quota HTTP Endpoint for Quota handling.

[Guangya Liu]

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



src/master/master.hpp (lines 863 - 878)


I see that there are comments for renaming those functions to add, update, 
delete or addQuota, updateQuta, deleteQuota, I think that add/update/delete may 
be better as all of the functions are already in QuotaHandler, thoughts? Thanks.


- Guangya Liu


On 十月 14, 2015, 1:28 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36913/
> ---
> 
> (Updated 十月 14, 2015, 1:28 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, Alex Clemmer, 
> and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3073
> https://issues.apache.org/jira/browse/MESOS-3073
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added /quota HTTP Endpoint for Quota handling.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 98e76cee81ab206f3ffe7989711abc38f49c4352 
>   src/Makefile.am 4a0eeb89eb2f3fa5e9fff46e4316e39209884343 
>   src/master/http.cpp 3ca3725302cc831b067a2f69edbda95cd30c2520 
>   src/master/master.hpp e7b16fdd21a8caa77a39956a8520cf1381186598 
>   src/master/master.cpp ba12a83b64e9fd02dd4ee7de27c44e0c8d4f03a9 
>   src/master/quota_handler.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36913/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 36913: Added /quota HTTP Endpoint for Quota handling.

[Joerg Schad]

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

(Updated Oct. 14, 2015, 1:28 p.m.)


Review request for mesos, Alexander Rukletsov, Bernd Mathiske, Alex Clemmer, 
and Joris Van Remoortere.


Changes
---

Adressed comments.


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


Repository: mesos


Description
---

Added /quota HTTP Endpoint for Quota handling.


Diffs (updated)
-

  src/CMakeLists.txt 98e76cee81ab206f3ffe7989711abc38f49c4352 
  src/Makefile.am 4a0eeb89eb2f3fa5e9fff46e4316e39209884343 
  src/master/http.cpp 3ca3725302cc831b067a2f69edbda95cd30c2520 
  src/master/master.hpp e7b16fdd21a8caa77a39956a8520cf1381186598 
  src/master/master.cpp ba12a83b64e9fd02dd4ee7de27c44e0c8d4f03a9 
  src/master/quota_handler.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 38978: Add post-rewrite hook.

[Joris Van Remoortere]

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

Ship it!


This looks good!
Tried it out locally.


support/hooks/post-rewrite (line 14)


How come you chose not to follow the whitespace check in the existing 
pre-commit hook?



support/hooks/post-rewrite (lines 15 - 19)


Can you add a comment explaining what the `@~..@` is doing here?


- Joris Van Remoortere


On Oct. 4, 2015, 8:08 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38978/
> ---
> 
> (Updated Oct. 4, 2015, 8:08 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2953
> https://issues.apache.org/jira/browse/MESOS-2953
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add post-rewrite hook.
> 
> 
> Diffs
> -
> 
>   bootstrap 779b33cdcb88b2417769bb046a17b47cd6042f2d 
>   support/hooks/post-rewrite PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38978/diff/
> 
> 
> Testing
> ---
> 
> test manually
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 39018: Added JSON parsing for Resources.

[Guangya Liu]

> On 十月 13, 2015, 3:35 a.m., Guangya Liu wrote:
> > include/mesos/resources.hpp, lines 114-118
> > 
> >
> > Can this be removed or merged to the under comments?
> 
> Greg Mann wrote:
> I put the comment here so that it wouldn't interfere with the Doxygen 
> documentation, which should be placed directly before the member declaration. 
> I don't want to remove it, because I think the TODO is still valid. I'm open 
> to moving it if there is a better place for it; do you have one in mind?
> 
> Guangya Liu wrote:
> What about moving those comments to /**/ section?
> 
> Greg Mann wrote:
> But then it will show up in the documentation, correct? My motivation for 
> leaving it out of the /**/ section was to exclude this comment from the docs.

I think it is ok to show up some TODO in document, thoughts? Thanks.


- Guangya


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


On 十月 13, 2015, 7:34 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated 十月 13, 2015, 7:34 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes code changes necessary for JSON parsing of Resources. 
> Documentation changes will be posted soon in another review 
> (https://reviews.apache.org/r/39102/).
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
>   include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39018/diff/
> 
> 
> Testing
> ---
> 
> New code was added to `ResourcesTest.ParsingFromJSON`, and two new tests were 
> added: `ResourcesTest.ParsingFromJSONWithRoles` and 
> `ResourcesTest.ParsingFromJSONError`. These attempt to test common error 
> scenarios that might show up in user-supplied JSON.
> 
> `make check` was used to confirm that all tests pass, with one notable 
> failure (ResourcesTest.ParsingFromJSONError) related to a picojson issue 
> tracked here: https://issues.apache.org/jira/browse/MESOS-3698
> 
> The original resources parsing format is used throughout many other tests 
> (check `src/tests/sorter_tests.cpp`, for example), so it's clear that the 
> original parsing continues to work correctly.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 36913: Added /quota HTTP Endpoint for Quota handling.

[Joerg Schad]

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

(Updated Oct. 14, 2015, 1:07 p.m.)


Review request for mesos, Alexander Rukletsov, Bernd Mathiske, Alex Clemmer, 
and Joris Van Remoortere.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Added /quota HTTP Endpoint for Quota handling.


Diffs (updated)
-

  src/CMakeLists.txt 98e76cee81ab206f3ffe7989711abc38f49c4352 
  src/Makefile.am 4a0eeb89eb2f3fa5e9fff46e4316e39209884343 
  src/master/http.cpp 3ca3725302cc831b067a2f69edbda95cd30c2520 
  src/master/master.hpp e7b16fdd21a8caa77a39956a8520cf1381186598 
  src/master/master.cpp ba12a83b64e9fd02dd4ee7de27c44e0c8d4f03a9 
  src/master/quota_handler.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 29550: Libprocess: Added "tests" target for make.

[Joris Van Remoortere]

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


I ran `make tests` from `3rdparty/libprocess` and ran in to:
`make: *** No rule to make target '3rdparty/libgmock.la', needed by 
'libprocess-tests'.  Stop.`

- Joris Van Remoortere


On Oct. 8, 2015, 7:29 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29550/
> ---
> 
> (Updated Oct. 8, 2015, 7:29 p.m.)
> 
> 
> Review request for mesos, Cody Maloney, Joris Van Remoortere, Timothy Chen, 
> and Timothy St. Clair.
> 
> 
> Bugs: MESOS-2273
> https://issues.apache.org/jira/browse/MESOS-2273
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> 'make check' allows one to build and run the test suite. However,
> 'make tests' allows one to build the test suite without running it.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> e64e3d9e958fab3c7c25fad17dcc2b279d255500 
>   3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1 
> 
> Diff: https://reviews.apache.org/r/29550/diff/
> 
> 
> Testing
> ---
> 
> Ran make tests.
> 
> For added test, ran "make clean; make; make tests" followed by "make check" 
> and verified that no additional target are build in the "make check" step.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Marco Massenzio]

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



support/apply-reviews.py (line 7)


uhm... could we use `requests` instead?
much more modern API and widespread use.



support/apply-reviews.py (line 11)


nit: you are 'masking' the global builtin `id()` here - that's a PEP8 
violation, could you please rename to something like `rid`?



support/apply-reviews.py (line 13)


don't concatenate string, prefer the use of os.path.join():
```
import os

os.path.join(REVIEW_URL, rid)
```
(why do you need the trailing slash?)



support/apply-reviews.py (line 19)


you should check for a non 4xx/5xx return code (or just check for a 2xx if 
you know for sure what the API returns).

as you expect JSON, you should probably specify that in the 
`Accept-content` header too?



support/apply-reviews.py (line 24)


if you are doing this often enough, it's much better to compile this to a 
constant Pattern and then use that instead.



support/apply-reviews.py (line 29)


you should not call a param with the name of a type (`list` is a type in 
Python).
also, it's bad practice to initialize it as you do there.

BTW - can you please use the @param to explain what the method expects?

Prefer:
```
def parent_review(rid, revs_list=None):
  """Returns a list with reversed chain of review requests for a given
  Review ID.
  """
  result = revs_list or []
  json_str = review_json(review_url(rid))
  json_obj = json.loads(json_str)

  # Stop as soon as we stumble upon a submitted request.
  rr = json_obj.get('review_request')
  if rr and rr.get('status') == "submitted":
return result

  for deps in rr.get('summary'):
  ...
  
```

Also, worth always using `get()` instead of accessor (`[]`) for data that 
you are not sure may or may not be there (it's an API response after all) - the 
latter will throw a KeyError if the key is missing; `get` will just give you 
back None (or you can pass a second optional default return value if you want).



support/apply-reviews.py (line 40)


is this a recursion inside an iteration?
can you please add a comment? I'm sure folks who may one day try to 
fix/augment this script will be just as confused...



support/apply-reviews.py (line 45)


is this right?
you return after the first iteration?

if so, why not just getting the first item in `depends_on`?



support/apply-reviews.py (line 56)


if you do this, this script will be probably useful to folks who use Python 
3 too :)

```
from __future__ import print_function

...

print('foo bar')
```
Also (but that's just something a bunch of us insisted upon) I prefer the 
use of `string.format()` to `%`:
```
print("Applying review {}: {}".format(review_id, summary))
```
(same also below to build `cmd`)



support/apply-reviews.py (line 90)


nit: `if r not in applied:` is more pythonic :)

(also, PEP8 doesn't really like 1- and 2-letter variables)



support/apply-reviews.py (line 91)


do you need a counter or something like that, or a flag would suffice?
I'm not sure how you use `applied` as a dict (looks like you are just using 
it as a `set`?):
```
applied = set()
for review, summary in reviews:
if review not in applied:
applied.add(review)
print(apply_review(...))
```


- Marco Massenzio


On Oct. 8, 2015, 6:26 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Oct. 8, 2015, 6:26 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> -

Re: Review Request 39018: Added JSON parsing for Resources.

[Greg Mann]

> On Oct. 13, 2015, 3:35 a.m., Guangya Liu wrote:
> > include/mesos/resources.hpp, lines 114-118
> > 
> >
> > Can this be removed or merged to the under comments?
> 
> Greg Mann wrote:
> I put the comment here so that it wouldn't interfere with the Doxygen 
> documentation, which should be placed directly before the member declaration. 
> I don't want to remove it, because I think the TODO is still valid. I'm open 
> to moving it if there is a better place for it; do you have one in mind?
> 
> Guangya Liu wrote:
> What about moving those comments to /**/ section?

But then it will show up in the documentation, correct? My motivation for 
leaving it out of the /**/ section was to exclude this comment from the docs.


- Greg


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


On Oct. 13, 2015, 7:34 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated Oct. 13, 2015, 7:34 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes code changes necessary for JSON parsing of Resources. 
> Documentation changes will be posted soon in another review 
> (https://reviews.apache.org/r/39102/).
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
>   include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39018/diff/
> 
> 
> Testing
> ---
> 
> New code was added to `ResourcesTest.ParsingFromJSON`, and two new tests were 
> added: `ResourcesTest.ParsingFromJSONWithRoles` and 
> `ResourcesTest.ParsingFromJSONError`. These attempt to test common error 
> scenarios that might show up in user-supplied JSON.
> 
> `make check` was used to confirm that all tests pass, with one notable 
> failure (ResourcesTest.ParsingFromJSONError) related to a picojson issue 
> tracked here: https://issues.apache.org/jira/browse/MESOS-3698
> 
> The original resources parsing format is used throughout many other tests 
> (check `src/tests/sorter_tests.cpp`, for example), so it's clear that the 
> original parsing continues to work correctly.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39018: Added JSON parsing for Resources.

[Greg Mann]

> On Oct. 13, 2015, 5 p.m., Michael Park wrote:
> > src/common/resources.cpp, line 361
> > 
> >
> > I think it would be better to only support one way of doing things 
> > here. I would opt for the array format. Is supporting both formats 
> > explicitly a feature?
> 
> Greg Mann wrote:
> No, it hasn't been explicitly called out as a feature. Why the desire to 
> allow only one format? My thought was that any valid JSON string describing 
> one or more Resource objects should return successfully, but I suppose there 
> isn't too much need to accommodate a single resource object, since at the 
> command line users will always be specifying multiple resources.
> 
> Michael Park wrote:
> > Why the desire to allow only one format?
> 
> (1) Generally speaking, it's good to limit the number of ways to do the 
> same thing so that there's less to remember as the user
> (2) In this specific case, it can actually be confusing/ambiguous.
> That is, does this expect a single `Resource` object: `{ "name" : 
> "cpus", ... }`, or
> does it expect a `Credentials`-like object where we specify something 
> like: `{ "resources": { "name" : "cpus", ... }, { "name" : "disk", ... }, ... 
> }`?
> (I know now that it does the first one, but at first glance I 
> actually thought it does the second)

Yea, I see your point with #1. Regarding #2, that is interesting... I went with 
a JSON array in this implementation due to the discussion on the JIRA ticket, 
but honestly I was unaware of the format of the `Credentials` JSON input. In 
order to make the formats more consistent, we could use the same form here: 
`{"resources": [{"name": "cpus", ...}, {"name": "disk"}, ...]}`. This would 
seem to be more in line with your point #1, keeping the interface simple for 
the user. Thoughts?


- Greg


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


On Oct. 13, 2015, 7:34 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated Oct. 13, 2015, 7:34 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes code changes necessary for JSON parsing of Resources. 
> Documentation changes will be posted soon in another review 
> (https://reviews.apache.org/r/39102/).
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
>   include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39018/diff/
> 
> 
> Testing
> ---
> 
> New code was added to `ResourcesTest.ParsingFromJSON`, and two new tests were 
> added: `ResourcesTest.ParsingFromJSONWithRoles` and 
> `ResourcesTest.ParsingFromJSONError`. These attempt to test common error 
> scenarios that might show up in user-supplied JSON.
> 
> `make check` was used to confirm that all tests pass, with one notable 
> failure (ResourcesTest.ParsingFromJSONError) related to a picojson issue 
> tracked here: https://issues.apache.org/jira/browse/MESOS-3698
> 
> The original resources parsing format is used throughout many other tests 
> (check `src/tests/sorter_tests.cpp`, for example), so it's clear that the 
> original parsing continues to work correctly.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39276]

All tests passed.

- Mesos ReviewBot


On Oct. 14, 2015, 12:04 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39276/
> ---
> 
> (Updated Oct. 14, 2015, 12:04 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Bernd Mathiske, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3705
> https://issues.apache.org/jira/browse/MESOS-3705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When using the same socket to send multiple HTTP requests to different 
> actors. If the actor responsible for handling the first request is stuck 
> handling another event while a subsequent request can reply immediatly, the 
> order of the responses is altered violating HTTP Pipelining.
> 
> This patch fixes that problem enforcing that every response is sent in the 
> order the corresponding request arrived. It also adds a test to reproduce the 
> issue and verify the fix works.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/event.hpp 
> 16ddbd77afa6efdf6bad201aa497ee102aa863ae 
>   3rdparty/libprocess/src/process.cpp 
> 0454554e7b6a39f94cfea02f08ca51ef6b35859a 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d13d3888abbf3db552df4a9f83e54667e598ded9 
> 
> Diff: https://reviews.apache.org/r/39276/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 39002: Fixed sed command for Python protos generation.

[Joris Van Remoortere]

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

Ship it!


Ship It!

- Joris Van Remoortere


On Oct. 5, 2015, 5:48 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39002/
> ---
> 
> (Updated Oct. 5, 2015, 5:48 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Joris Van Remoortere, and Marco 
> Massenzio.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current command didn't complete the protobuf generation and resulted in 
> an empty file.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e698927 
> 
> Diff: https://reviews.apache.org/r/39002/diff/
> 
> 
> Testing
> ---
> 
> make check and verified that protobufs were properly generated
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 39097: CMake:[2/2] remove `__WINDOWS__` flag definition from Stout config.

[Joris Van Remoortere]

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

Ship it!


- Joris Van Remoortere


On Oct. 13, 2015, 6:26 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39097/
> ---
> 
> (Updated Oct. 13, 2015, 6:26 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake:[2/2] remove `__WINDOWS__` flag definition from Stout config.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/cmake/StoutTestsConfigure.cmake 
> c4d1b7b3f896f8037d922fafcdbdd0e31a70e140 
> 
> Diff: https://reviews.apache.org/r/39097/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39096: CMake:[1/2] Moved `__WINDOWS__` flag definition to CompilationConfigure.

[Joris Van Remoortere]

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

Ship it!


cleaned up the comment a little.

- Joris Van Remoortere


On Oct. 13, 2015, 6:26 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39096/
> ---
> 
> (Updated Oct. 13, 2015, 6:26 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake:[1/2] Moved `__WINDOWS__` flag definition to CompilationConfigure.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake d1d75098d0ad655a7dcd74628bffd7ba8547b454 
> 
> Diff: https://reviews.apache.org/r/39096/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39092: CMake:[2/3] prepared process library tests to run with `make check`.

[Joris Van Remoortere]

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

Ship it!


- Joris Van Remoortere


On Oct. 13, 2015, 6:25 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39092/
> ---
> 
> (Updated Oct. 13, 2015, 6:25 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3597
> https://issues.apache.org/jira/browse/MESOS-3597
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Right now running the Mesos tests from the default tests target will (1)
> aggregate all our tests into a semi-useless report that hides all the
> errors, and (2) strip all our colors when you do pass the verbosity flag
> to get rid of the "report" structure.
> 
> This commit will prepare us to run the tests with with color, without
> this default reporting structure. We do this by moving the tests target
> to a `CACHE STRING` which can be referenced by the tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> ea6db09e1a1aa01450aee93814e07f09feae7ac9 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> a14b5b8fe22d3e75bef3c716ae7865ddaf132927 
> 
> Diff: https://reviews.apache.org/r/39092/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39093: CMake:[3/3] Add `make check` target.

[Joris Van Remoortere]

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

Ship it!


- Joris Van Remoortere


On Oct. 13, 2015, 6:25 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39093/
> ---
> 
> (Updated Oct. 13, 2015, 6:25 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3597
> https://issues.apache.org/jira/browse/MESOS-3597
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake:[3/3] Add `make check` target.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt a5a66c13496a95bb7095504a4645909d887fb49f 
> 
> Diff: https://reviews.apache.org/r/39093/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39091: CMake:[1/3] Prepared stout tests to run with `make check`.

[Joris Van Remoortere]

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

Ship it!


- Joris Van Remoortere


On Oct. 13, 2015, 6:25 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39091/
> ---
> 
> (Updated Oct. 13, 2015, 6:25 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3597
> https://issues.apache.org/jira/browse/MESOS-3597
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Right now running the Mesos tests from the default tests target will
> (1) aggregate all our tests into a semi-useless report that hides all
> the errors, and (2) strip all our colors when you do pass the verbosity
> flag to get rid of the "report" structure.
> 
> This commit will prepare us to run the tests with with color, without
> this default reporting structure. We do this by moving the tests target
> to a `CACHE STRING` which can be referenced by the tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/cmake/StoutTestsConfigure.cmake 
> c4d1b7b3f896f8037d922fafcdbdd0e31a70e140 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 94292f8a46ec31bbaf6e52f48109322bbe123f70 
> 
> Diff: https://reviews.apache.org/r/39091/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39076: CMake: Added ability of Windows builds to include protobuf headers.

[Joris Van Remoortere]

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

Ship it!



3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake (line 109)


missing space after `endif`


- Joris Van Remoortere


On Oct. 14, 2015, 11:47 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39076/
> ---
> 
> (Updated Oct. 14, 2015, 11:47 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Added ability of Windows builds to include protobuf headers.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> ee1c74d31e28136bf289f4100d79a8ce568cd3af 
> 
> Diff: https://reviews.apache.org/r/39076/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

[Benjamin Hindman]

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


Going to look at the test now, but wanted to provide this feedback first.


3rdparty/libprocess/include/process/event.hpp (line 128)


CHECK_NOTNULL(promise);

And you might as well do this for the request too!

CHECK_NOTNULL(request);
CHECK_NOTNULL(promise);



3rdparty/libprocess/src/process.cpp (line 2345)


Promise* promise = new Promise();

Or:

auto promise = new Promise();



3rdparty/libprocess/src/process.cpp (line 2354)


Let's clean this up a little bit. IIUC this is the only place in the code 
where we dispatch to HttpProxy::handle, and it's not clear that we need a 
pointer to the future (it's a shared_ptr under the covers anyway). So how about 
we do something like:

HttpProxy::handle(Future&& future, Request&& request)
{
  items.push(new Item(move(future), move(request)));
  ...;
}

Then at the call site:

// Dispatch to the proxy needs to be done at this point ...
// ...
// Also note that we need to pass a copy of  'request' because 
HttpProxy::handle takes ownership.
dispatch(proxy, &HttpProxy::handle, promise->future(), Request(*request));

Sound good? Or am I missing something?


- Benjamin Hindman


On Oct. 14, 2015, 12:04 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39276/
> ---
> 
> (Updated Oct. 14, 2015, 12:04 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Bernd Mathiske, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3705
> https://issues.apache.org/jira/browse/MESOS-3705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When using the same socket to send multiple HTTP requests to different 
> actors. If the actor responsible for handling the first request is stuck 
> handling another event while a subsequent request can reply immediatly, the 
> order of the responses is altered violating HTTP Pipelining.
> 
> This patch fixes that problem enforcing that every response is sent in the 
> order the corresponding request arrived. It also adds a test to reproduce the 
> issue and verify the fix works.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/event.hpp 
> 16ddbd77afa6efdf6bad201aa497ee102aa863ae 
>   3rdparty/libprocess/src/process.cpp 
> 0454554e7b6a39f94cfea02f08ca51ef6b35859a 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d13d3888abbf3db552df4a9f83e54667e598ded9 
> 
> Diff: https://reviews.apache.org/r/39276/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

[Alexander Rojas]

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

(Updated Oct. 14, 2015, 2:04 p.m.)


Review request for mesos, Anand Mazumdar, Bernd Mathiske, and Till Toenshoff.


Changes
---

Rewrites the test.


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


Repository: mesos


Description
---

When using the same socket to send multiple HTTP requests to different actors. 
If the actor responsible for handling the first request is stuck handling 
another event while a subsequent request can reply immediatly, the order of the 
responses is altered violating HTTP Pipelining.

This patch fixes that problem enforcing that every response is sent in the 
order the corresponding request arrived. It also adds a test to reproduce the 
issue and verify the fix works.


Diffs (updated)
-

  3rdparty/libprocess/include/process/event.hpp 
16ddbd77afa6efdf6bad201aa497ee102aa863ae 
  3rdparty/libprocess/src/process.cpp 0454554e7b6a39f94cfea02f08ca51ef6b35859a 
  3rdparty/libprocess/src/tests/http_tests.cpp 
d13d3888abbf3db552df4a9f83e54667e598ded9 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 39076: CMake: Added ability of Windows builds to include protobuf headers.

[Alex Clemmer]

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

(Updated Oct. 14, 2015, 11:47 a.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Repository: mesos


Description
---

CMake: Added ability of Windows builds to include protobuf headers.


Diffs
-

  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
ee1c74d31e28136bf289f4100d79a8ce568cd3af 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 39312: Hierarchical Allocator: Replaced Polymorphic factory with functions.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39312]

All tests passed.

- Mesos ReviewBot


On Oct. 14, 2015, 9:15 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39312/
> ---
> 
> (Updated Oct. 14, 2015, 9:15 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Michael Park.
> 
> 
> Bugs: MESOS-3554
> https://issues.apache.org/jira/browse/MESOS-3554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> e468b5ae61e828bf0d1d670582dcf8a6c9fad5a8 
>   src/master/allocator/mesos/hierarchical.cpp 
> 0a6f8a67b9a8a0e6d17e4aa0bc3b55cf4787cf01 
> 
> Diff: https://reviews.apache.org/r/39312/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 39310: Enable /master/state-summary endpoint include active flag for framework

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39310]

All tests passed.

- Mesos ReviewBot


On Oct. 14, 2015, 8:03 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39310/
> ---
> 
> (Updated Oct. 14, 2015, 8:03 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3721
> https://issues.apache.org/jira/browse/MESOS-3721
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enable /master/state-summary endpoint include active flag for framework
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 3ca3725302cc831b067a2f69edbda95cd30c2520 
> 
> Diff: https://reviews.apache.org/r/39310/diff/
> 
> 
> Testing
> ---
> 
> gyliu@devstack007:~$ curl "http://9.111.242.187:5050/master/state-summary"; 
> 2>/dev/null| jq .
> {
>   "slaves": [
> {
>   "used_resources": {
> "mem": 256,
> "disk": 0,
> "cpus": 1
>   },
>   "unreserved_resources": {
> "ports": "[31000-32000]",
> "mem": 2808,
> "disk": 1399183,
> "cpus": 4
>   },
>   "resources": {
> "ports": "[31000-32000]",
> "mem": 2808,
> "disk": 1399183,
> "cpus": 4
>   },
>   "reserved_resources": {},
>   "reregistered_time": 1444809313.03239,
>   "TASK_STARTING": 0,
>   "TASK_STAGING": 0,
>   "TASK_RUNNING": 1,
>   "TASK_LOST": 0,
>   "TASK_KILLED": 0,
>   "TASK_FINISHED": 0,
>   "TASK_FAILED": 0,
>   "TASK_ERROR": 0,
>   "active": true,
>   "attributes": {},
>   "framework_ids": [
> "ec7faf83-1838-4909-9a96-4e30a6757967-"
>   ],
>   "hostname": "devstack007.cn.ibm.com",
>   "id": "2a7e51b2-20d2-4476-a3e3-8c3207e7427a-S0",
>   "offered_resources": {
> "mem": 0,
> "disk": 0,
> "cpus": 0
>   },
>   "pid": "slave(1)@9.111.242.187:5051",
>   "registered_time": 1444809313.03176
> }
>   ],
>   "hostname": "devstack007.cn.ibm.com",
>   "frameworks": [
> {
>   "webui_url": "",
>   "used_resources": {
> "mem": 256,
> "disk": 0,
> "cpus": 1
>   },
>   "TASK_STARTING": 0,
>   "TASK_STAGING": 0,
>   "TASK_RUNNING": 1,
>   "TASK_LOST": 0,
>   "TASK_KILLED": 0,
>   "TASK_FINISHED": 0,
>   "TASK_FAILED": 0,
>   "TASK_ERROR": 0,
>   "active": true,  Here
>   "capabilities": [],
>   "hostname": "devstack007.cn.ibm.com",
>   "id": "ec7faf83-1838-4909-9a96-4e30a6757967-",
>   "name": "",
>   "offered_resources": {
> "mem": 0,
> "disk": 0,
> "cpus": 0
>   },
>   "pid": 
> "scheduler-b68990a9-aed9-4182-bacb-456cbb7302d6@9.111.242.187:34782",
>   "slave_ids": [
> "2a7e51b2-20d2-4476-a3e3-8c3207e7427a-S0"
>   ]
> }
>   ]
> }
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38627: Adds an overload of ModuleManager::create() allowing overriding parameters programatically

[Alexander Rojas]

> On Oct. 12, 2015, 2:54 p.m., Marco Massenzio wrote:
> > Would it be possible to add a few unit tests, also to show usage patterns? 
> > especially given the absence of any documentation, it's kinda difficult to 
> > figure out how is this "intended to work" and, without tests, whether it 
> > works at all.
> 
> Niklas Nielsen wrote:
> Alex: Ping ^^

Sure, I just haven't had time to look into this yet.


- Alexander


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


On Oct. 2, 2015, 10:11 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38627/
> ---
> 
> (Updated Oct. 2, 2015, 10:11 p.m.)
> 
> 
> Review request for mesos, Adam B, Bernd Mathiske, Niklas Nielsen, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allows developers to provide their own parameters when loading modules 
> instead of using the ones provided by the user when loading Mesos. This helps 
> to deal with default modules (those used when the user doesn't provide any), 
> and for testing of the modules.
> 
> 
> Diffs
> -
> 
>   src/module/manager.hpp 302eb409fb8ef53b9cef8d2ecbe7b7f452b095ef 
>   src/tests/module.hpp 0820978441aede18dae6d1701433bff705b8c3c2 
> 
> Diff: https://reviews.apache.org/r/38627/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Review Request 39312: Hierarchical Allocator: Replaced Polymorphic factory with functions.

[Joris Van Remoortere]

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

Review request for mesos, Ben Mahler and Michael Park.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/master/allocator/mesos/hierarchical.hpp 
e468b5ae61e828bf0d1d670582dcf8a6c9fad5a8 
  src/master/allocator/mesos/hierarchical.cpp 
0a6f8a67b9a8a0e6d17e4aa0bc3b55cf4787cf01 

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


Testing
---

make check.


Thanks,

Joris Van Remoortere

Re: Review Request 39088: Made shell test locale-independent.

[Till Toenshoff]

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

Ship it!


Ship It!

- Till Toenshoff


On Oct. 8, 2015, 1:27 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39088/
> ---
> 
> (Updated Oct. 8, 2015, 1:27 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Jan Schlicht, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3517
> https://issues.apache.org/jira/browse/MESOS-3517
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The test environment does not enforce that we run with a defined locale,
> so instead enforce it for our particular use case (here: potentially
> localized error messages from shell).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> e6d36ec1bf414b52d0899f0edf83e0ad8910dd0e 
> 
> Diff: https://reviews.apache.org/r/39088/diff/
> 
> 
> Testing
> ---
> 
> With German locale installed:
> 
> % LANG=de_DE.UTF-8 make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 39300: Fixed flaky ReservationEndpointsTest.AvailableResources test.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39300]

All tests passed.

- Mesos ReviewBot


On Oct. 14, 2015, 7:38 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39300/
> ---
> 
> (Updated Oct. 14, 2015, 7:38 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3411
> https://issues.apache.org/jira/browse/MESOS-3411
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The following failure was encountered on buildbot: 
> https://builds.apache.org/job/Mesos/COMPILER=gcc,CONFIGURATION=--verbose,OS=ubuntu:14.04,label_exp=docker%7C%7CHadoop/921/console
> 
> ```
> I1013 09:54:08.882694 29149 master.cpp:5559] Processing TEARDOWN call for 
> framework 4b0845cd-7ce9-4e7a-b5d1-bcf1c413ca39- (default) at 
> scheduler-62c161d7-60e9-4361-aae1-6431e60035f6@172.17.5.161:57074
> I1013 09:54:08.882822 29149 master.cpp:5571] Removing framework 
> 4b0845cd-7ce9-4e7a-b5d1-bcf1c413ca39- (default) at 
> scheduler-62c161d7-60e9-4361-aae1-6431e60035f6@172.17.5.161:57074
> ../../src/tests/reservation_endpoints_tests.cpp:184: Failure
> Mock function called more times than expected - taking default action 
> specified at:
> ../../src/tests/mesos.hpp:1518:
> Function call: recoverResources(@0x2b1be400cc10 
> 4b0845cd-7ce9-4e7a-b5d1-bcf1c413ca39-, @0x2b1be4018f40 
> 4b0845cd-7ce9-4e7a-b5d1-bcf1c413ca39-S0, @0x2b1bc22fc390 { cpus(*):2, 
> mem(*):1024, disk(*):1024, ports(*):[31000-32000] }, @0x2b1bc22fc3d0 40-byte 
> object <01-00 00-00 1C-2B 00-00 0B-00 00-00 00-00 00-00 00-00 00-00 00-00 
> 00-00 00-00 00-00 00-00 00-00 00-00 80-3F 00-00 00-00>)
>  Expected: to be called once
>Actual: called twice - over-saturated and active
> I1013 09:54:08.884042 29149 hierarchical.hpp:599] Deactivated framework 
> 4b0845cd-7ce9-4e7a-b5d1-bcf1c413ca39-
> I1013 09:54:08.884371 29149 hierarchical.hpp:1103] Recovered cpus(*):2; 
> mem(*):1024; disk(*):1024; ports(*):[31000-32000] (total: cpus(*):2; 
> mem(*):1024; disk(*):1024; ports(*):[31000-32000], allocated: ) on slave 
> 4b0845cd-7ce9-4e7a-b5d1-bcf1c413ca39-S0 from framework 
> 4b0845cd-7ce9-4e7a-b5d1-bcf1c413ca39-
> I1013 09:54:08.884469 29149 hierarchical.hpp:552] Removed framework 
> 4b0845cd-7ce9-4e7a-b5d1-bcf1c413ca39-
> ```
> 
> In `ReservationEndpointsTest.AvailableResources`, the test ends with the 
> framework holding an offer.
> The issue is that there is only a `WillOnce` on `recoverResources`, but it 
> should ignore subsequent calls with `WillRepeatedly`
> since resources are expected to recover on framework teardown.
> 
> It seems that there is a race in the test teardown logic, but this fix is 
> applicable regardless.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_endpoints_tests.cpp 
> d41185241ae8f1f4ad4f9e88b7dea6fdac991ae7 
> 
> Diff: https://reviews.apache.org/r/39300/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 38307: Libprocess: Removed namespace pollution.

[Joris Van Remoortere]

> On Sept. 11, 2015, 6:57 p.m., Joseph Wu wrote:
> > Ship it!
> > 
> > I pointed out a bunch of would-be-nice-to-refactor lines.  But I don't 
> > think it's critical to change them to ship this.

Let's make these changes in a separate patch.


- Joris


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


On Oct. 14, 2015, 8:39 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38307/
> ---
> 
> (Updated Oct. 14, 2015, 8:39 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joseph Wu, Michael Park, and 
> Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> d1c81f1d244f02bf42cab97198587ce1b8c7c407 
>   3rdparty/libprocess/src/test-master.cpp 
> 5ce91710031e6fce3e1b5791eb782941aef9f10a 
>   3rdparty/libprocess/src/test-slave.cpp 
> 03fa8e61f46dc70647f478c5adef67d8de6cf5c2 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> bb9ced8933bf2bb97ae6b3cffdb5528676e53c11 
>   3rdparty/libprocess/src/tests/decoder_tests.cpp 
> f1963fdb4c27faa9055e9f9f6edd850cebd15948 
>   3rdparty/libprocess/src/tests/encoder_tests.cpp 
> 5ad5fd415baca4423c3054a42bd9e175b72153b8 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 38f3ad7e46f5b6ef4850cdf7fdcc115715e98472 
>   3rdparty/libprocess/src/tests/io_tests.cpp 
> a7135ee0cfeef7c07ebe41815f47df24dd2b713c 
>   3rdparty/libprocess/src/tests/limiter_tests.cpp 
> ba722509fefc3cd592b6f41d02a0b12d5adf7e00 
>   3rdparty/libprocess/src/tests/metrics_tests.cpp 
> 8f15ee7e89e5346d306df71eb06a7161da13af48 
>   3rdparty/libprocess/src/tests/mutex_tests.cpp 
> 484727fd36c04f8a3380808d0f75eaad11338331 
>   3rdparty/libprocess/src/tests/owned_tests.cpp 
> 3ef76b8d5426ea10f63bf78b96f45046dd8a955a 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> ffd260a3fa2e49b3de183ba7b392b71afaaba2e5 
>   3rdparty/libprocess/src/tests/queue_tests.cpp 
> a6a0dab42ea1674256e8990255bf507aa24f24b8 
>   3rdparty/libprocess/src/tests/reap_tests.cpp 
> a844ae2b863b98c49ec8fac79e905b3c04a142fc 
>   3rdparty/libprocess/src/tests/sequence_tests.cpp 
> 88de001aad259c3035f2ff20ed61e7e119a73f54 
>   3rdparty/libprocess/src/tests/shared_tests.cpp 
> 74ea26060ac2800cb98dffbfc90494871d0a9e21 
>   3rdparty/libprocess/src/tests/ssl_client.cpp 
> 9ae59e54bab954b1a931dbc3bd2f1540deb9f5aa 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> ee30a02b112d20bced49ead15447d412f4005a8f 
>   3rdparty/libprocess/src/tests/statistics_tests.cpp 
> 657d27aed18949fa8200236662f2897b2ef4 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> 25d4a7b2fe35fb95f3f19f39bc81d6e50f5c25a3 
>   3rdparty/libprocess/src/tests/system_tests.cpp 
> 25436ea2e977d667037e2d7344a8804ec35b27b9 
>   3rdparty/libprocess/src/tests/time_tests.cpp 
> 60791ff20671859a0da91524ae8e40ff52c907d0 
>   3rdparty/libprocess/src/tests/timeseries_tests.cpp 
> ec6f4868a65b89244fb4bb8234a77c87831863c3 
> 
> Diff: https://reviews.apache.org/r/38307/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38307: Libprocess: Removed namespace pollution.

[Joris Van Remoortere]

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

(Updated Oct. 14, 2015, 8:39 a.m.)


Review request for mesos, Artem Harutyunyan, Joseph Wu, Michael Park, and Till 
Toenshoff.


Changes
---

updated.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp d1c81f1d244f02bf42cab97198587ce1b8c7c407 
  3rdparty/libprocess/src/test-master.cpp 
5ce91710031e6fce3e1b5791eb782941aef9f10a 
  3rdparty/libprocess/src/test-slave.cpp 
03fa8e61f46dc70647f478c5adef67d8de6cf5c2 
  3rdparty/libprocess/src/tests/benchmarks.cpp 
bb9ced8933bf2bb97ae6b3cffdb5528676e53c11 
  3rdparty/libprocess/src/tests/decoder_tests.cpp 
f1963fdb4c27faa9055e9f9f6edd850cebd15948 
  3rdparty/libprocess/src/tests/encoder_tests.cpp 
5ad5fd415baca4423c3054a42bd9e175b72153b8 
  3rdparty/libprocess/src/tests/http_tests.cpp 
38f3ad7e46f5b6ef4850cdf7fdcc115715e98472 
  3rdparty/libprocess/src/tests/io_tests.cpp 
a7135ee0cfeef7c07ebe41815f47df24dd2b713c 
  3rdparty/libprocess/src/tests/limiter_tests.cpp 
ba722509fefc3cd592b6f41d02a0b12d5adf7e00 
  3rdparty/libprocess/src/tests/metrics_tests.cpp 
8f15ee7e89e5346d306df71eb06a7161da13af48 
  3rdparty/libprocess/src/tests/mutex_tests.cpp 
484727fd36c04f8a3380808d0f75eaad11338331 
  3rdparty/libprocess/src/tests/owned_tests.cpp 
3ef76b8d5426ea10f63bf78b96f45046dd8a955a 
  3rdparty/libprocess/src/tests/process_tests.cpp 
ffd260a3fa2e49b3de183ba7b392b71afaaba2e5 
  3rdparty/libprocess/src/tests/queue_tests.cpp 
a6a0dab42ea1674256e8990255bf507aa24f24b8 
  3rdparty/libprocess/src/tests/reap_tests.cpp 
a844ae2b863b98c49ec8fac79e905b3c04a142fc 
  3rdparty/libprocess/src/tests/sequence_tests.cpp 
88de001aad259c3035f2ff20ed61e7e119a73f54 
  3rdparty/libprocess/src/tests/shared_tests.cpp 
74ea26060ac2800cb98dffbfc90494871d0a9e21 
  3rdparty/libprocess/src/tests/ssl_client.cpp 
9ae59e54bab954b1a931dbc3bd2f1540deb9f5aa 
  3rdparty/libprocess/src/tests/ssl_tests.cpp 
ee30a02b112d20bced49ead15447d412f4005a8f 
  3rdparty/libprocess/src/tests/statistics_tests.cpp 
657d27aed18949fa8200236662f2897b2ef4 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
25d4a7b2fe35fb95f3f19f39bc81d6e50f5c25a3 
  3rdparty/libprocess/src/tests/system_tests.cpp 
25436ea2e977d667037e2d7344a8804ec35b27b9 
  3rdparty/libprocess/src/tests/time_tests.cpp 
60791ff20671859a0da91524ae8e40ff52c907d0 
  3rdparty/libprocess/src/tests/timeseries_tests.cpp 
ec6f4868a65b89244fb4bb8234a77c87831863c3 

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


Testing
---

make check.


Thanks,

Joris Van Remoortere

Review Request 39310: Enable /master/state-summary endpoint include active flag for framework

[Guangya Liu]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Enable /master/state-summary endpoint include active flag for framework


Diffs
-

  src/master/http.cpp 3ca3725302cc831b067a2f69edbda95cd30c2520 

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


Testing
---

gyliu@devstack007:~$ curl "http://9.111.242.187:5050/master/state-summary"; 
2>/dev/null| jq .
{
  "slaves": [
{
  "used_resources": {
"mem": 256,
"disk": 0,
"cpus": 1
  },
  "unreserved_resources": {
"ports": "[31000-32000]",
"mem": 2808,
"disk": 1399183,
"cpus": 4
  },
  "resources": {
"ports": "[31000-32000]",
"mem": 2808,
"disk": 1399183,
"cpus": 4
  },
  "reserved_resources": {},
  "reregistered_time": 1444809313.03239,
  "TASK_STARTING": 0,
  "TASK_STAGING": 0,
  "TASK_RUNNING": 1,
  "TASK_LOST": 0,
  "TASK_KILLED": 0,
  "TASK_FINISHED": 0,
  "TASK_FAILED": 0,
  "TASK_ERROR": 0,
  "active": true,
  "attributes": {},
  "framework_ids": [
"ec7faf83-1838-4909-9a96-4e30a6757967-"
  ],
  "hostname": "devstack007.cn.ibm.com",
  "id": "2a7e51b2-20d2-4476-a3e3-8c3207e7427a-S0",
  "offered_resources": {
"mem": 0,
"disk": 0,
"cpus": 0
  },
  "pid": "slave(1)@9.111.242.187:5051",
  "registered_time": 1444809313.03176
}
  ],
  "hostname": "devstack007.cn.ibm.com",
  "frameworks": [
{
  "webui_url": "",
  "used_resources": {
"mem": 256,
"disk": 0,
"cpus": 1
  },
  "TASK_STARTING": 0,
  "TASK_STAGING": 0,
  "TASK_RUNNING": 1,
  "TASK_LOST": 0,
  "TASK_KILLED": 0,
  "TASK_FINISHED": 0,
  "TASK_FAILED": 0,
  "TASK_ERROR": 0,
  "active": true,  Here
  "capabilities": [],
  "hostname": "devstack007.cn.ibm.com",
  "id": "ec7faf83-1838-4909-9a96-4e30a6757967-",
  "name": "",
  "offered_resources": {
"mem": 0,
"disk": 0,
"cpus": 0
  },
  "pid": 
"scheduler-b68990a9-aed9-4182-bacb-456cbb7302d6@9.111.242.187:34782",
  "slave_ids": [
"2a7e51b2-20d2-4476-a3e3-8c3207e7427a-S0"
  ]
}
  ]
}


Thanks,

Guangya Liu