Review Request 35687: Added capabilities to state.json

[Aditi Dixit]

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

Review request for mesos, Marco Massenzio and Vinod Kone.


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


Repository: mesos


Description
---

Added capabilities to state.json


Diffs
-

  src/master/http.cpp 8b694137fe3491e4d40af0f31ee87d3e3ef8ab5a 

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


Testing
---

make check


Thanks,

Aditi Dixit

Re: Review Request 33339: Add a Java example framework to test persistent volumes.

[Adam B]

> On June 1, 2015, 4:32 p.m., Marco Massenzio wrote:
> > This is great -sorry it took so long to get to do a review.
> > 
> > Thanks for doing it, I'm quite looking forward to using it to learning more 
> > about the Persistent Framework :)
> > it would be great if we could have a bit more comments in the code to help 
> > all other newbies.
> > 
> > My review was fairly narrowly focused on Java style etc. - I'm expecting 
> > one of the commiters will be able to give you feedback about using the 
> > framework.
> 
> Jie Yu wrote:
> Sorry, this is my fault. I really don't have cycle for this at this 
> moment. Marco, mind sherperding this patch?
> 
> Jie Yu wrote:
> The logic of this patch should match that in 
> src/examples/persistent_volume_framework.cpp
> 
> Marco Massenzio wrote:
> Jie - I would be absolutely happy to do that, with one minor hitch: I'm 
> not a committer :)
> Happy to help out wherever I can, though!
> 
> haosdent huang wrote:
> Thank you very much for your review, let me update it. @marco
> 
> haosdent huang wrote:
> @marco Thank you for your review again. But some logic are follow the 
> exists Java examples and persistent_volume_framework.cpp . I am not sure 
> change it are correct or not. Anyway, I think I need to reflect this patch to 
> make it more clear. After I reflect it, I would @you and looking forward your 
> review again. Thanks a lot.
> 
> Marco Massenzio wrote:
> I understand that - and therein lies the problem :)
> As you proved, code in 'examples' is widely "used as a template" (aka, 
> copied) so it would be great if we could set an... example, by providing 
> great code there.
> 
> Translating Java almost verbatim from C++ actually results in 
> non-idiomatic constructs that either (a) look weird to Java practitioners or 
> (b) engender bad practices (that are then propagated by the "oh, but that's 
> how it's done in the other examples" - sounds familiar? :)
> 
> My suggestions (and I stress it here, they are "suggestions" - if 
> committers with more Java experience than I are happy with your code, then 
> that's just fine) are to provide some great examples, that improve on 
> existing code.
> 
> Thanks for doing this!

Thanks haosdent for coding this up, and thanks Marco for the initial review. As 
a Java/C++ polyglot, I'll volunteer to Shepherd this patch, if Jie will help 
verify that it properly tests his feature.
Unfortunately, it seems the authors of the original Java examples/tests were 
not hardened Java developers, and their poor style habits have been copied into 
subsequent examples. I won't ask haosdent to solve all of those issues, or else 
this patch would be indefinitely delayed. As long as it's no worse than the 
other examples, and adequately demonstrates/tests the feature, I'll let it 
through.


> On June 1, 2015, 4:32 p.m., Marco Massenzio wrote:
> > src/examples/java/TestPersistentVolumeFramework.java, lines 529-542
> > 
> >
> > Please consider using Apache Commons CLI instead: 
> > https://commons.apache.org/proper/commons-cli/
> 
> haosdent huang wrote:
> Yes, but it would add a new dependence to Maven. Is it acceptable? Other 
> exist java examples also simple handle the args.
> 
> Marco Massenzio wrote:
> I am not sure what the "official policy" is here - but this is a widely 
> used library (and not a major dependency either) so I think we should be fine 
> with it.
> Anyone got any opinion on this?

Anything from Apache Commons Proper should be fine. We're all Apache here.


> On June 1, 2015, 4:32 p.m., Marco Massenzio wrote:
> > src/examples/java/TestPersistentVolumeFramework.java, lines 443-444
> > 
> >
> > instead of concatenating string, either use a StringBuilder or, even 
> > better:
> > ```
> > String.format("Received framework message from executor '%s' on slave 
> > %s: '%s'", executorId, slaveId, data);
> > ```
> 
> haosdent huang wrote:
> Sure, but would not match
> 
> ```
> LOG(INFO) << "Received framework message from executor '" << executorId
>   << "' on slave " << slaveId << ": '" << data << "'";
> ```
> 
> in persistent_volume_framework.cpp
> 
> Marco Massenzio wrote:
> What do you mean? the output would be the exact same?

I agree with Marco that the output would be formatted exactly the same, and 
standard string concatenation is slow in Java, so most people use StringBuilder 
or even String.format. On the other hand, this test framework is not 
performance-critical, and none of the other Java examples/tests use 
StringBuilder/String.format. Fix it as many places as you like, but I won't 
hold up the patch for it.


> On June 1, 2015, 4:32 p.m., Marco Massenzio wrote:
> > src/examples/java/TestPersistentVolumeFramework.java, lines 3

Re: Review Request 34362: Include ExecutorInfos in master/state.json

[Adam B]

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

Ship it!


Looks great! I'll fix the preexisting typo, and commit this soon.


src/common/http.cpp (line 159)


Found a typo comment that got copied around: s/in/it/?


- Adam B


On June 7, 2015, 2:29 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34362/
> ---
> 
> (Updated June 7, 2015, 2:29 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-2743
> https://issues.apache.org/jira/browse/MESOS-2743
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Include ExecutorInfos in master/state.json
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp afce7fea334c7bfa57efc48c413bf906a2ebde32 
>   src/common/http.cpp 2ac7fba7a3aac913540f1b09768777393b79284a 
>   src/master/http.cpp 01dbdad9720a9cd30505683ae1117970cda918a0 
>   src/slave/http.cpp b5e77b09435e50db5231a2b32faf76dab91dae54 
> 
> Diff: https://reviews.apache.org/r/34362/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> I start 1500 executor in 30 slaves. In executor, it just sleep. And the test 
> result is
> 
> time curl -s -o /dev/null localhost:5050/master/state.json
> 
> real0m0.651s
> user0m0.001s
> sys 0m0.004s
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 31838: Fixed authentication failure triggered slave crash.

[Adam B]

> On March 30, 2015, 10:58 a.m., Vinod Kone wrote:
> > I think the bigger problem here is that slave shuts down all the running 
> > tasks and executors if authentication fails. I think that is dangerous. For 
> > example if the master is misconfigured with wrong credentials, this could 
> > potentially shut down the whole cluster! I think the safer thing to do is 
> > to EXIT(1) in _authenticate() instead of shutdown().

Excellent idea. Till has implemented this in a separate patch: 
https://reviews.apache.org/r/35583/diff


- Adam


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


On March 8, 2015, 5:54 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31838/
> ---
> 
> (Updated March 8, 2015, 5:54 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-2464
> https://issues.apache.org/jira/browse/MESOS-2464
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 364d911 
> 
> Diff: https://reviews.apache.org/r/31838/diff/
> 
> 
> Testing
> ---
> 
> make check && functional testing with failed authentication
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 31838: Fixed authentication failure triggered slave crash.

[Adam B]

> On March 13, 2015, 5:11 p.m., Alexander Rukletsov wrote:
> > src/slave/slave.cpp, lines 537-538
> > 
> >
> > Does it make sense to flatten this for readability? Like this:
> > ```
> > } else if (info.has_id()) {
> > ...
> > } else {
> > ...
> > }
> > ```

I like the idea of flattening the else if.


- Adam


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


On March 8, 2015, 5:54 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31838/
> ---
> 
> (Updated March 8, 2015, 5:54 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-2464
> https://issues.apache.org/jira/browse/MESOS-2464
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 364d911 
> 
> Diff: https://reviews.apache.org/r/31838/diff/
> 
> 
> Testing
> ---
> 
> make check && functional testing with failed authentication
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 35686: Change the if statement to a CHECK at the end of _runTask.

[Alexander Rukletsov]

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

Ship it!


Ship It!

- Alexander Rukletsov


On June 20, 2015, 4:42 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35686/
> ---
> 
> (Updated June 20, 2015, 4:42 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Follow-up from [r35433](https://reviews.apache.org/r/35433/#comment141187).
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp e73913b072e323033a81e64b5345cb9457137e3e 
> 
> Diff: https://reviews.apache.org/r/35686/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 31838: Fixed authentication failure triggered slave crash.

[Adam B]

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

Ship it!


LGTM, although I agree with Alex that you could clean up the if/else-if/else 
formatting. I could do it before committing, or you could.

- Adam B


On March 8, 2015, 5:54 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31838/
> ---
> 
> (Updated March 8, 2015, 5:54 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-2464
> https://issues.apache.org/jira/browse/MESOS-2464
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 364d911 
> 
> Diff: https://reviews.apache.org/r/31838/diff/
> 
> 
> Testing
> ---
> 
> make check && functional testing with failed authentication
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 35583: Updated slave to exit on authentication refusal.

[Adam B]

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

Ship it!


Ship It!

- Adam B


On June 17, 2015, 3:42 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35583/
> ---
> 
> (Updated June 17, 2015, 3:42 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2464
> https://issues.apache.org/jira/browse/MESOS-2464
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp a5ad29f 
> 
> Diff: https://reviews.apache.org/r/35583/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 35506: Excluding MarkDown files from the style hook

[Adam B]

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

Ship it!


Ship It!

- Adam B


On June 16, 2015, 4:24 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35506/
> ---
> 
> (Updated June 16, 2015, 4:24 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Joerg Schad.
> 
> 
> Bugs: MESOS-2873
> https://issues.apache.org/jira/browse/MESOS-2873
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py e87388fdde847f67209ad00f28aad3ad9c50c1ea 
> 
> Diff: https://reviews.apache.org/r/35506/diff/
> 
> 
> Testing
> ---
> 
> manual tests.
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 35433: Sent StatusUpdates if checkpointed resources don't exist on the slave.

[Alexander Rukletsov]

> On June 14, 2015, 10:46 a.m., Benjamin Hindman wrote:
> > Just so I understand, does this mean if we happen to get in the unfortunate 
> > situation where a slave has neglected to get the dynamic reservation 
> > because it was just starting up and then it gets the task launch it will 
> > shutdown the slave because the CHECK will fail? I would expect the slave to 
> > simply send a TASK_LOST. Said another way, this is not an assertion our 
> > code guarantees. If instead we were waiting for some kind of an ack from 
> > the slave that it received the dynamic reservation before it send the task 
> > launch then a CHECK would make sense.
> 
> Jie Yu wrote:
> We don't expect this to happen because we always send a 
> CheckpointResourcesMessage before sending the task to the slave and TCP 
> ensures in order delivery (out of order delivery is possible if two sockets 
> are used. it's possible because the way we create ephemeral connections, but 
> this is very unlikely to happen). Master won't send the task to the slave if 
> the slave hasn't registered.
> 
> I would rather keep the CHECK here unless we found that this is a real 
> issue (and then we can change that to send status update).
> 
> Michael Park wrote:
> So currently it is possible for this to happen, but only with a very 
> small probability. Your proposal is to keep the `CHECK` and put in the effort 
> to eliminate the possibility once we observe it as a real problem, correct? 
> The part that I don't quite understand is, what's the motivation to wait for 
> a real problem to occur when we know it's possible to run into this issue 
> (even with a small probability), the effort to change the `CHECK` to sending 
> `TASK_LOST` seems to be small?
> 
> Jie Yu wrote:
> Well, everything has a probablity to fail, the question is how large the 
> probability is. Memory could have hardware errors and a bit could be flipped 
> due to random reasons, does that mean that we have to do parity check in 
> every single location in our code base? I think my point is the probability 
> for this to fail is extremely low so that we shouldn't worry too much.
> 
> I am fine with sending a status update.
> 
> Alexander Rukletsov wrote:
> I wonder, what are the cases when the task launch request may arrive 
> before `CheckpointResourcesMessage`? If my understanding is correct, we do 
> not have delivery guarantee for `CheckpointResourcesMessage`, nor we have the 
> same queue in Master for `CheckpointResourcesMessage` and `RunTaskMessage` to 
> ensure the order. My intuition is that the probability of such an event is 
> not negligible: a network blip can occur and `CheckpointResourcesMessage` may 
> be lost or delayed, we can open another socket to the slave for 
> `RunTaskMessage`. Could you please help me understand that?
> 
> Ben Mahler wrote:
> Such a situation will manifest an `Exited` event from the socket closure. 
> At the application level, we want to ensure that if there are any `Exited` 
> events, the slave (or framework) will re-register. This is currently not 
> fully implemented: currently only the master-side `Exited` is implemented (we 
> ping the slave telling it we think it is disconnected), the slave-side 
> `Exited` is a no-op.
> 
> It may become simpler with the HTTP API since we have a single duplex 
> socket (the master does not initiate a connection with the slave (or 
> framework)). This means that the responsibility of dealing with a closed 
> socket is left to the slave (or framework) only. Off the top of my head, I'm 
> not sure if there are situations where only 1 side of the socket can be 
> broken.. so maybe it will be just as complicated :)
> 
> Let's discuss off this thread, I do have some tickets around this stuff.

Sure, would love to!


- Alexander


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


On June 19, 2015, 2:31 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35433/
> ---
> 
> (Updated June 19, 2015, 2:31 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Hindman, and Jie Yu.
> 
> 
> Bugs: MESOS-2491
> https://issues.apache.org/jira/browse/MESOS-2491
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> No bug was observed (yet), but realized I forgot about this in the dynamic 
> reservations patches.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 8df1211165169c9595e0e6e85b5ddc404345ff70 
>   src/slave/slave.cpp a5ad29f59fadba919ed82ba2892c2febe551660b 
> 
> Diff: https://reviews.apache.org/r/35433/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 

Re: Review Request 35687: Added capabilities to state.json

[Alexander Rukletsov]

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



src/master/http.cpp (line 124)


Let's make it `foreachvalue` for consistency.


- Alexander Rukletsov


On June 20, 2015, 7:14 a.m., Aditi Dixit wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35687/
> ---
> 
> (Updated June 20, 2015, 7:14 a.m.)
> 
> 
> Review request for mesos, Marco Massenzio and Vinod Kone.
> 
> 
> Bugs: MESOS-2900
> https://issues.apache.org/jira/browse/MESOS-2900
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added capabilities to state.json
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 8b694137fe3491e4d40af0f31ee87d3e3ef8ab5a 
> 
> Diff: https://reviews.apache.org/r/35687/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Aditi Dixit
> 
>

Re: Review Request 35583: Updated slave to exit on authentication refusal.

[Alexander Rukletsov]

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

Ship it!


Ship It!

- Alexander Rukletsov


On June 17, 2015, 10:42 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35583/
> ---
> 
> (Updated June 17, 2015, 10:42 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2464
> https://issues.apache.org/jira/browse/MESOS-2464
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp a5ad29f 
> 
> Diff: https://reviews.apache.org/r/35583/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 35335: Fixed JSON output error TaskInfo executor_id in slave/http.cpp

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [35335]

All tests passed.

- Mesos ReviewBot


On June 18, 2015, 11:11 p.m., Brendan Chang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35335/
> ---
> 
> (Updated June 18, 2015, 11:11 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2598
> https://issues.apache.org/jira/browse/MESOS-2598
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed JSON output error TaskInfo executor_id in slave/http.cpp
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp bc25bdd33277dbfa30410ad081ea09f0fc39c598 
>   src/tests/slave_tests.cpp 8709874411835c99b8900102fd014a9e28f2b208 
> 
> Diff: https://reviews.apache.org/r/35335/diff/
> 
> 
> Testing
> ---
> 
> Extended SlaveEndpoint test to launch a task and check for correct 
> executor_id in state.json.
> 
> It was difficult to mock queued_tasks for testing (see 
> https://issues.apache.org/jira/browse/MESOS-2896), so the extension to 
> SlaveEndpoint verifies the executor_id format for general tasks.
> 
> 
> Thanks,
> 
> Brendan Chang
> 
>

Re: Review Request 35668: Report "unevictable" memory in container statistics.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [35668]

All tests passed.

- Mesos ReviewBot


On June 19, 2015, 9:03 p.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35668/
> ---
> 
> (Updated June 19, 2015, 9:03 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Jie Yu.
> 
> 
> Bugs: mesos-2798
> https://issues.apache.org/jira/browse/mesos-2798
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Report "unevictable" memory in container statistics.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 81008ed85daea798ed19d1031de8421a7dc3fb19 
>   src/slave/containerizer/isolators/cgroups/mem.cpp 
> 9d65bf5b64ce72c1ca9a7a50e65a357e098af63e 
> 
> Diff: https://reviews.apache.org/r/35668/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Re: Review Request 35611: Added initial doxygen documentation for stout flags.

[Benjamin Hindman]

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

(Updated June 20, 2015, 3:19 p.m.)


Review request for mesos and Bernd Mathiske.


Repository: mesos


Description
---

A follow up to https://reviews.apache.org/r/34943 as requested by most 
reviewers.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
ee855da6128c2d671eb2fc7eaa2c0aab2341aebb 

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


Testing
---

make check and generated doxygen documentation.


Thanks,

Benjamin Hindman

Re: Review Request 35335: Fixed JSON output error TaskInfo executor_id in slave/http.cpp

[Benjamin Hindman]

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

Ship it!



src/tests/slave_tests.cpp (line 1049)


Minor nit, we've been using braced initializer lists either with an = or as 
a constructor with no space in between. I'll go with = here since I think 
that's what you prefer given the space.


- Benjamin Hindman


On June 18, 2015, 11:11 p.m., Brendan Chang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35335/
> ---
> 
> (Updated June 18, 2015, 11:11 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2598
> https://issues.apache.org/jira/browse/MESOS-2598
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed JSON output error TaskInfo executor_id in slave/http.cpp
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp bc25bdd33277dbfa30410ad081ea09f0fc39c598 
>   src/tests/slave_tests.cpp 8709874411835c99b8900102fd014a9e28f2b208 
> 
> Diff: https://reviews.apache.org/r/35335/diff/
> 
> 
> Testing
> ---
> 
> Extended SlaveEndpoint test to launch a task and check for correct 
> executor_id in state.json.
> 
> It was difficult to mock queued_tasks for testing (see 
> https://issues.apache.org/jira/browse/MESOS-2896), so the extension to 
> SlaveEndpoint verifies the executor_id format for general tasks.
> 
> 
> Thanks,
> 
> Brendan Chang
> 
>

Re: Review Request 35287: Rename Option::get(const T& _t) to getOrElse() and refactor original functions (stout)

[Mark Wang]

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

(Updated June 20, 2015, 4:19 p.m.)


Review request for mesos and Joris Van Remoortere.


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


Repository: mesos


Description
---

Rename Option::get(const T& _t) to getOrElse() and refactor original 
functions (stout)


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 
8d5217a699a302cc9ebb2aa10d74cced7eb2b3d9 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp 
c8d30d8c193eb14f7accfde4fe02ce0710cd1817 
  3rdparty/libprocess/3rdparty/stout/tests/option_tests.cpp 
f1ae80ab881ccfcefda69d5bfee2d969d171f1b9 

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


Testing
---

make check


Thanks,

Mark Wang

Re: Review Request 35285: Rename Option::get(const T& _t) to getOrElse() and refactor original functions (mesos)

[Mark Wang]

> On June 18, 2015, 7:44 p.m., Joris Van Remoortere wrote:
> > Hey Mark,
> > Can you make the dependency chain purely linear so the reviewbot can apply 
> > the patches correctly and build / run the tests?
> > `35287->35286->35285`

I submit a new review without any change then I try to discard the draft, I got 
"HTTP 500 INTERNAL SERVER ERROR"
can't get around with this..


- Mark


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


On June 10, 2015, 8:13 a.m., Mark Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35285/
> ---
> 
> (Updated June 10, 2015, 8:13 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2800
> https://issues.apache.org/jira/browse/MESOS-2800
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rename Option::get(const T& _t) to getOrElse() and refactor original 
> functions (mesos)
> 
> 
> Diffs
> -
> 
>   src/log/catchup.cpp f7afc38916d0a7e57cdecb0da7ccb3901e726b90 
>   src/master/main.cpp 1c33e3bf7baae45b1671f9566b4993f39d9f1294 
>   src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
>   src/slave/containerizer/containerizer.cpp 
> e995ce602261c18373ac09c823638c4a252cca86 
>   src/tests/cluster.hpp cfe7ef0c7a6dc62cddc3e5f5b5b28c8bcb2bed26 
> 
> Diff: https://reviews.apache.org/r/35285/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Mark Wang
> 
>

Re: Review Request 35286: Rename Option::get(const T& _t) to getOrElse() and refactor original functions (libprocess)

[Mark Wang]

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

(Updated June 20, 2015, 4:47 p.m.)


Review request for mesos and Joris Van Remoortere.


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


Repository: mesos


Description
---

Rename Option::get(const T& _t) to getOrElse() and refactor original 
functions (libprocess)


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp aadd7bb0ae12b93336900c76d8d5aaa4421ea198 
  3rdparty/libprocess/src/subprocess.cpp 
f41f5e2a34788e31749eb996c8ab38ea45989068 

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


Testing
---

make check


Thanks,

Mark Wang

Re: Review Request 35287: Rename Option::get(const T& _t) to getOrElse() and refactor original functions (stout)

[Mark Wang]

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

(Updated June 20, 2015, 4:48 p.m.)


Review request for mesos and Joris Van Remoortere.


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


Repository: mesos


Description
---

Rename Option::get(const T& _t) to getOrElse() and refactor original 
functions (stout)


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 
8d5217a699a302cc9ebb2aa10d74cced7eb2b3d9 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp 
c8d30d8c193eb14f7accfde4fe02ce0710cd1817 
  3rdparty/libprocess/3rdparty/stout/tests/option_tests.cpp 
f1ae80ab881ccfcefda69d5bfee2d969d171f1b9 

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


Testing
---

make check


Thanks,

Mark Wang

Re: Review Request 35285: Rename Option::get(const T& _t) to getOrElse() and refactor original functions (mesos)

[Mark Wang]

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

(Updated June 20, 2015, 4:49 p.m.)


Review request for mesos and Joris Van Remoortere.


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


Repository: mesos


Description
---

Rename Option::get(const T& _t) to getOrElse() and refactor original 
functions (mesos)


Diffs
-

  src/log/catchup.cpp f7afc38916d0a7e57cdecb0da7ccb3901e726b90 
  src/master/main.cpp 1c33e3bf7baae45b1671f9566b4993f39d9f1294 
  src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
  src/slave/containerizer/containerizer.cpp 
e995ce602261c18373ac09c823638c4a252cca86 
  src/tests/cluster.hpp cfe7ef0c7a6dc62cddc3e5f5b5b28c8bcb2bed26 

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


Testing
---

make check


Thanks,

Mark Wang

Re: Review Request 35285: Rename Option::get(const T& _t) to getOrElse() and refactor original functions (mesos)

[Mark Wang]

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

(Updated June 20, 2015, 4:48 p.m.)


Review request for mesos and Joris Van Remoortere.


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


Repository: mesos


Description
---

Rename Option::get(const T& _t) to getOrElse() and refactor original 
functions (mesos)


Diffs (updated)
-

  src/log/catchup.cpp f7afc38916d0a7e57cdecb0da7ccb3901e726b90 
  src/master/main.cpp 1c33e3bf7baae45b1671f9566b4993f39d9f1294 
  src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
  src/slave/containerizer/containerizer.cpp 
e995ce602261c18373ac09c823638c4a252cca86 
  src/tests/cluster.hpp cfe7ef0c7a6dc62cddc3e5f5b5b28c8bcb2bed26 

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


Testing
---

make check


Thanks,

Mark Wang

Re: Review Request 35285: Rename Option::get(const T& _t) to getOrElse() and refactor original functions (mesos)

[Mark Wang]

> On June 18, 2015, 7:44 p.m., Joris Van Remoortere wrote:
> > Hey Mark,
> > Can you make the dependency chain purely linear so the reviewbot can apply 
> > the patches correctly and build / run the tests?
> > `35287->35286->35285`
> 
> Mark Wang wrote:
> I submit a new review without any change then I try to discard the draft, 
> I got "HTTP 500 INTERNAL SERVER ERROR"
> can't get around with this..

issue solved. I had to use rbt tool to publish.


- Mark


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


On June 20, 2015, 4:49 p.m., Mark Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35285/
> ---
> 
> (Updated June 20, 2015, 4:49 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2800
> https://issues.apache.org/jira/browse/MESOS-2800
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rename Option::get(const T& _t) to getOrElse() and refactor original 
> functions (mesos)
> 
> 
> Diffs
> -
> 
>   src/log/catchup.cpp f7afc38916d0a7e57cdecb0da7ccb3901e726b90 
>   src/master/main.cpp 1c33e3bf7baae45b1671f9566b4993f39d9f1294 
>   src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
>   src/slave/containerizer/containerizer.cpp 
> e995ce602261c18373ac09c823638c4a252cca86 
>   src/tests/cluster.hpp cfe7ef0c7a6dc62cddc3e5f5b5b28c8bcb2bed26 
> 
> Diff: https://reviews.apache.org/r/35285/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Mark Wang
> 
>

Re: Review Request 35565: Refactored os::environment to return a std::map.

[Benjamin Hindman]

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

(Updated June 20, 2015, 6:58 p.m.)


Review request for mesos and Till Toenshoff.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
1e7aed0b48b8889eb8d042cbd9f907303d2510d3 
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
62987e0df28f28816c59d7cbad89fa2af41ade04 

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


Testing
---

make check


Thanks,

Benjamin Hindman

Review Request 35694: Added helper constructors to hashmap.

[Benjamin Hindman]

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

Review request for mesos and Till Toenshoff.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp 
4f90d3dcd880b95f22ea13c56a61c7f981eea57d 

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


Testing
---

make check


Thanks,

Benjamin Hindman

Re: Review Request 35567: Added 'executor_environment_variables' flag to slave.

[Benjamin Hindman]

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

(Updated June 20, 2015, 7 p.m.)


Review request for mesos and Till Toenshoff.


Summary (updated)
-

Added 'executor_environment_variables' flag to slave.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/slave/containerizer/containerizer.cpp 
c5ffc49899976225e319f122cb5f7556c3b09d46 
  src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
  src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 
  src/tests/containerizer.cpp 80b910515694c81cf5b1eace1d79a5178cf35426 

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


Testing
---

make check


Thanks,

Benjamin Hindman

Re: Review Request 35565: Refactored os::environment to return a std::map.

[Benjamin Hindman]

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

(Updated June 20, 2015, 7:01 p.m.)


Review request for mesos and Till Toenshoff.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
1e7aed0b48b8889eb8d042cbd9f907303d2510d3 
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
62987e0df28f28816c59d7cbad89fa2af41ade04 

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


Testing
---

make check


Thanks,

Benjamin Hindman

Re: Review Request 35694: Added helper constructors to hashmap.

[Benjamin Hindman]

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

(Updated June 20, 2015, 7:06 p.m.)


Review request for mesos and Till Toenshoff.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp 
4f90d3dcd880b95f22ea13c56a61c7f981eea57d 

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


Testing
---

make check


Thanks,

Benjamin Hindman

Review Request 35695: stout: Fixed test names.

[Michael Park]

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

Review request for mesos, Benjamin Hindman and Till Toenshoff.


Repository: mesos


Description
---

First noticed `JsonTest.parse`, then found the other cases with the following 
regex: `TEST.*(.*, [a-z].*`.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 
60c0336082caf30b2d7943d85ef3bb7a534648d8 
  3rdparty/libprocess/3rdparty/stout/tests/mac_tests.cpp 
4dbe4bab79f7536a6a7a53348f44f8ac888d3d1d 
  3rdparty/libprocess/3rdparty/stout/tests/os/sendfile_tests.cpp 
f2386d55c658a5bbdedb6b433c70f7739f6ac0d4 
  3rdparty/libprocess/3rdparty/stout/tests/os/signals_tests.cpp 
ad79a1677fb0bb92215e29f98a17c97d5309279c 
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
62987e0df28f28816c59d7cbad89fa2af41ade04 
  3rdparty/libprocess/3rdparty/stout/tests/proc_tests.cpp 
d3c9aede8e8a7f419f8eae7a7f52629d5cc1513f 
  3rdparty/libprocess/3rdparty/stout/tests/thread_tests.cpp 
7519b12d7334b16496377f60b2e830401dd0db86 
  3rdparty/libprocess/3rdparty/stout/tests/uuid_tests.cpp 
ad1d986de72a51540ebeb56db7eb399fb8e66f87 

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


Testing
---

`make check`


Thanks,

Michael Park

Review Request 35697: mesos: Fixed test names.

[Michael Park]

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

Review request for mesos, Benjamin Hindman and Till Toenshoff.


Repository: mesos


Description
---

First noticed `JsonTest.parse`, then found the other cases with the following 
regex: `TEST.*(.*, [a-z].*`.


Diffs
-

  src/tests/cram_md5_authentication_tests.cpp 
992302365682df2c800c8828a21b9d38cb318472 
  src/tests/credentials_tests.cpp ed672733ff895b1f252fa75d5653a9f3527d5b5f 
  src/tests/registrar_tests.cpp 97d0b685dd2744cf92b3a463d586e8bcf4c0af1a 

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


Testing
---

`make check`


Thanks,

Michael Park

Review Request 35696: libprocess: Fixed test names.

[Michael Park]

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

Review request for mesos, Benjamin Hindman and Till Toenshoff.


Repository: mesos


Description
---

First noticed `JsonTest.parse`, then found the other cases with the following 
regex: `TEST.*(.*, [a-z].*`.


Diffs
-

  3rdparty/libprocess/src/tests/http_tests.cpp 
86b2c7d3beae6671474cf0a5ad52a10334bc9230 
  3rdparty/libprocess/src/tests/io_tests.cpp 
84e5d746220dfa274465632280de87dc5e7f673a 
  3rdparty/libprocess/src/tests/mutex_tests.cpp 
451b234ca46dd91bff6d8463a00a40fb14070db6 
  3rdparty/libprocess/src/tests/process_tests.cpp 
660af45e7fd45bdf5d43ad9aa54477fd94f87058 
  3rdparty/libprocess/src/tests/queue_tests.cpp 
79741d37cb761593b38f21eef466d0313220d721 
  3rdparty/libprocess/src/tests/statistics_tests.cpp 
0e4960a4e2e807ee9f3f6d83cdbfa6b563352760 
  3rdparty/libprocess/src/tests/timeseries_tests.cpp 
fd906b869e8bbf5c07c67c5319dec82681e811b7 

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


Testing
---

`make check`


Thanks,

Michael Park

Re: Review Request 35694: Added helper constructors to hashmap.

[Benjamin Hindman]

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

(Updated June 20, 2015, 7:18 p.m.)


Review request for mesos and Till Toenshoff.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp 
4f90d3dcd880b95f22ea13c56a61c7f981eea57d 
  3rdparty/libprocess/3rdparty/stout/tests/hashmap_tests.cpp 
6a26d93a9a68ab18b7c9b25039a96b663a73a309 

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


Testing
---

make check


Thanks,

Benjamin Hindman

Re: Review Request 35682: Sped up Allocator::updateSlave() and Allocator::updateAllocation().

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [35679, 35680, 35682]

All tests passed.

- Mesos ReviewBot


On June 20, 2015, 1:08 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35682/
> ---
> 
> (Updated June 20, 2015, 1:08 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Jie Yu.
> 
> 
> Bugs: MESOS-2891
> https://issues.apache.org/jira/browse/MESOS-2891
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allocator::updateSlave() and Allocator::updateAllocation() are so much faster 
> now! See numbers below.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 646ee8c1c0fb824e1d17150b4e96e6281c65358f 
> 
> Diff: https://reviews.apache.org/r/35682/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> [ RUN  ] SlaveCount/HierarchicalAllocator_BENCHMARK_Test.UpdateSlave/0
> Added 1000 slaves in 768.327886ms
> Updated 1000 slaves in 488.183946ms
> [   OK ] SlaveCount/HierarchicalAllocator_BENCHMARK_Test.UpdateSlave/0 
> (1435 ms)
> [ RUN  ] SlaveCount/HierarchicalAllocator_BENCHMARK_Test.UpdateSlave/1
> Added 5000 slaves in 3.796704225secs
> Updated 5000 slaves in 2.432450845secs
> [   OK ] SlaveCount/HierarchicalAllocator_BENCHMARK_Test.UpdateSlave/1 
> (7180 ms)
> [ RUN  ] SlaveCount/HierarchicalAllocator_BENCHMARK_Test.UpdateSlave/2
> Added 1 slaves in 7.624720216secs
> Updated 1 slaves in 4.937190687secs
> [   OK ] SlaveCount/HierarchicalAllocator_BENCHMARK_Test.UpdateSlave/2 
> (14426 ms)
> [ RUN  ] SlaveCount/HierarchicalAllocator_BENCHMARK_Test.UpdateSlave/3
> Added 2 slaves in 15.412138292secs
> Updated 2 slaves in 10.148561378secs
> [   OK ] SlaveCount/HierarchicalAllocator_BENCHMARK_Test.UpdateSlave/3 
> (29457 ms)
> [ RUN  ] SlaveCount/HierarchicalAllocator_BENCHMARK_Test.UpdateSlave/4
> Added 3 slaves in 23.182005885secs
> Updated 3 slaves in 15.020974051secs
> [   OK ] SlaveCount/HierarchicalAllocator_BENCHMARK_Test.UpdateSlave/4 
> (43653 ms)
> [ RUN  ] SlaveCount/HierarchicalAllocator_BENCHMARK_Test.UpdateSlave/5
> Added 5 slaves in 38.638591555secs
> Updated 5 slaves in 24.861561733secs
> [   OK ] SlaveCount/HierarchicalAllocator_BENCHMARK_Test.UpdateSlave/5 
> (73836 ms)
> [--] 6 tests from SlaveCount/HierarchicalAllocator_BENCHMARK_Test 
> (169998 ms total)
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 35694: Added helper constructors to hashmap.

[Alexander Rukletsov]

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


Why do we use different approaches (emplace() and insert()) in c-tors? Is it 
possible to unify them for clarity? If not, mind leaving a comment explaining 
the reasoning?


3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp (line 52)


Let's avoid re-creating iterator:

```
for (auto iterator = map.begin(), auto endIterator = map.end();
 iterator != endIterator;
 ++iterator) {
```



3rdparty/libprocess/3rdparty/stout/tests/hashmap_tests.cpp (line 32)


Let's expand the test for move c-tor as well.


- Alexander Rukletsov


On June 20, 2015, 7:18 p.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35694/
> ---
> 
> (Updated June 20, 2015, 7:18 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-2832
> https://issues.apache.org/jira/browse/MESOS-2832
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp 
> 4f90d3dcd880b95f22ea13c56a61c7f981eea57d 
>   3rdparty/libprocess/3rdparty/stout/tests/hashmap_tests.cpp 
> 6a26d93a9a68ab18b7c9b25039a96b663a73a309 
> 
> Diff: https://reviews.apache.org/r/35694/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Review Request 35699: Added an invariant CHECK_EQ for available resources in HierarchicalAllocator::updateAllocation.

[Michael Park]

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/master/allocator/mesos/hierarchical.hpp 
646ee8c1c0fb824e1d17150b4e96e6281c65358f 

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


Testing
---

`make check`


Thanks,

Michael Park

Review Request 35701: Minor formatting cleanup in the Master.

[Michael Park]

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

Review request for mesos, Benjamin Hindman and Till Toenshoff.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/common/http.cpp 4c8102e3cd75e9284dac3d535545370ca37f502c 
  src/master/master.cpp 0135c155181546d3cb43e9e05bb874af846d928d 

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


Testing
---

`make check`


Thanks,

Michael Park

Review Request 35702: [WIP] Add /reserve HTTP endpoint to the master.

[Michael Park]

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

Review request for mesos, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van 
Remoortere, and Vinod Kone.


Repository: mesos


Description
---

This is a work in progress. But I'm sharing early to gather some feedback 
around:

(1) I've added `updateAvailable` to the allocator API. Is this necessary or is 
there maybe a better way?
(2) To determine the amount of available resources, I used: `available = total 
- (offered + used)`.
Is this still correct with oversubscription in the picture?
(3) I'm creating an `Offer.Operation` to perform the necessary updates in the 
allocator and such.
Feels weird to use an "offer" operation when there's not an actual offer. 
Is this fine for now?


Diffs
-

  include/mesos/master/allocator.hpp 92de1af414321281b00eaa6f129e5e3e2c849448 
  src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
  src/master/allocator/mesos/allocator.hpp 
6cfa04650d91a80211cfbc0809236f9438926c78 
  src/master/allocator/mesos/hierarchical.hpp 
646ee8c1c0fb824e1d17150b4e96e6281c65358f 
  src/master/http.cpp b893013ddd052cb58c520ac0328f4a5f0fed862e 
  src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
  src/master/master.cpp 0135c155181546d3cb43e9e05bb874af846d928d 
  src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 
  src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 
  src/tests/mesos.hpp 9157ac079808d2686592e54ea26a26e6a0825ed3 
  src/tests/reserve_tests.cpp PRE-CREATION 

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


Testing
---

Added `src/tests/reserve_tests.cpp` which still need to be fleshed out for 
error cases (missing parameters, unauthorized client, etc).


Thanks,

Michael Park

Re: Review Request 35702: [WIP] Add /reserve HTTP endpoint to the master.

[Michael Park]

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

(Updated June 20, 2015, 8:10 p.m.)


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


Changes
---

Added JIRA ticket and added AdamB as a reviewer.


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


Repository: mesos


Description
---

This is a work in progress. But I'm sharing early to gather some feedback 
around:

(1) I've added `updateAvailable` to the allocator API. Is this necessary or is 
there maybe a better way?
(2) To determine the amount of available resources, I used: `available = total 
- (offered + used)`.
Is this still correct with oversubscription in the picture?
(3) I'm creating an `Offer.Operation` to perform the necessary updates in the 
allocator and such.
Feels weird to use an "offer" operation when there's not an actual offer. 
Is this fine for now?


Diffs
-

  include/mesos/master/allocator.hpp 92de1af414321281b00eaa6f129e5e3e2c849448 
  src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
  src/master/allocator/mesos/allocator.hpp 
6cfa04650d91a80211cfbc0809236f9438926c78 
  src/master/allocator/mesos/hierarchical.hpp 
646ee8c1c0fb824e1d17150b4e96e6281c65358f 
  src/master/http.cpp b893013ddd052cb58c520ac0328f4a5f0fed862e 
  src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
  src/master/master.cpp 0135c155181546d3cb43e9e05bb874af846d928d 
  src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 
  src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 
  src/tests/mesos.hpp 9157ac079808d2686592e54ea26a26e6a0825ed3 
  src/tests/reserve_tests.cpp PRE-CREATION 

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


Testing
---

Added `src/tests/reserve_tests.cpp` which still need to be fleshed out for 
error cases (missing parameters, unauthorized client, etc).


Thanks,

Michael Park

Re: Review Request 35686: Change the if statement to a CHECK at the end of _runTask.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [35686]

All tests passed.

- Mesos ReviewBot


On June 20, 2015, 4:42 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35686/
> ---
> 
> (Updated June 20, 2015, 4:42 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Follow-up from [r35433](https://reviews.apache.org/r/35433/#comment141187).
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp e73913b072e323033a81e64b5345cb9457137e3e 
> 
> Diff: https://reviews.apache.org/r/35686/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35702: [WIP] Add /reserve HTTP endpoint to the master.

[Michael Park]

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

(Updated June 20, 2015, 8:19 p.m.)


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


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


Repository: mesos


Description (updated)
---

This is still a work in progress, but I'm sharing to gather some feedback 
around:

(1) I've added `updateAvailable` to the allocator API. Is this necessary or is 
there maybe a better way?
(2) To determine the amount of available resources, I used: `available = total 
- (offered + used)`.
Is this still correct with oversubscription in the picture?
(3) I'm creating an `Offer.Operation` to perform the necessary updates in the 
allocator and such.
Feels weird to use an "offer" operation when there's not an actual offer. 
Is this fine for now?


Diffs
-

  include/mesos/master/allocator.hpp 92de1af414321281b00eaa6f129e5e3e2c849448 
  src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
  src/master/allocator/mesos/allocator.hpp 
6cfa04650d91a80211cfbc0809236f9438926c78 
  src/master/allocator/mesos/hierarchical.hpp 
646ee8c1c0fb824e1d17150b4e96e6281c65358f 
  src/master/http.cpp b893013ddd052cb58c520ac0328f4a5f0fed862e 
  src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
  src/master/master.cpp 0135c155181546d3cb43e9e05bb874af846d928d 
  src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 
  src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 
  src/tests/mesos.hpp 9157ac079808d2686592e54ea26a26e6a0825ed3 
  src/tests/reserve_tests.cpp PRE-CREATION 

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


Testing
---

Added `src/tests/reserve_tests.cpp` which still need to be fleshed out for 
error cases (missing parameters, unauthorized client, etc).


Thanks,

Michael Park

Review Request 35703: Set refuse seconds on the correct filter in reservation test.

[Michael Park]

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

Review request for mesos, Alexander Rukletsov and Jie Yu.


Repository: mesos


Description
---

`set_refuse_seconds` should have been called on `filtersForever` rather than 
`filters`.


Diffs
-

  src/tests/reservation_tests.cpp 755a375346e0be290d4b8ffac3ecf5e7d191970d 

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


Testing
---

`make check`


Thanks,

Michael Park

Re: Review Request 35687: Added capabilities to state.json

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [35687]

All tests passed.

- Mesos ReviewBot


On June 20, 2015, 7:14 a.m., Aditi Dixit wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35687/
> ---
> 
> (Updated June 20, 2015, 7:14 a.m.)
> 
> 
> Review request for mesos, Marco Massenzio and Vinod Kone.
> 
> 
> Bugs: MESOS-2900
> https://issues.apache.org/jira/browse/MESOS-2900
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added capabilities to state.json
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 8b694137fe3491e4d40af0f31ee87d3e3ef8ab5a 
> 
> Diff: https://reviews.apache.org/r/35687/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Aditi Dixit
> 
>

Re: Review Request 35611: Added initial doxygen documentation for stout flags.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [35611]

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

Error:
 2015-06-20 21:26:09 URL:https://reviews.apache.org/r/35611/diff/raw/ 
[16549/16549] -> "35611.patch" [1]
error: patch failed: 
3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp:58
error: 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp: patch 
does not apply
Failed to apply patch

- Mesos ReviewBot


On June 20, 2015, 3:19 p.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35611/
> ---
> 
> (Updated June 20, 2015, 3:19 p.m.)
> 
> 
> Review request for mesos and Bernd Mathiske.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A follow up to https://reviews.apache.org/r/34943 as requested by most 
> reviewers.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
> ee855da6128c2d671eb2fc7eaa2c0aab2341aebb 
> 
> Diff: https://reviews.apache.org/r/35611/diff/
> 
> 
> Testing
> ---
> 
> make check and generated doxygen documentation.
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 35287: Rename Option::get(const T& _t) to getOrElse() and refactor original functions (stout)

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [35285, 35286, 35287]

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

Error:
 2015-06-20 21:41:02 URL:https://reviews.apache.org/r/35287/diff/raw/ 
[2559/2559] -> "35287.patch" [1]
error: patch failed: 
3rdparty/libprocess/3rdparty/stout/tests/option_tests.cpp:101
error: 3rdparty/libprocess/3rdparty/stout/tests/option_tests.cpp: patch does 
not apply
Failed to apply patch

- Mesos ReviewBot


On June 20, 2015, 4:48 p.m., Mark Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35287/
> ---
> 
> (Updated June 20, 2015, 4:48 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2800
> https://issues.apache.org/jira/browse/MESOS-2800
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rename Option::get(const T& _t) to getOrElse() and refactor original 
> functions (stout)
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 
> 8d5217a699a302cc9ebb2aa10d74cced7eb2b3d9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp 
> c8d30d8c193eb14f7accfde4fe02ce0710cd1817 
>   3rdparty/libprocess/3rdparty/stout/tests/option_tests.cpp 
> f1ae80ab881ccfcefda69d5bfee2d969d171f1b9 
> 
> Diff: https://reviews.apache.org/r/35287/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Mark Wang
> 
>

Re: Review Request 35567: Added 'executor_environment_variables' flag to slave.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [35561, 35562, 35563, 35564]

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

Error:
 2015-06-20 22:05:25 URL:https://reviews.apache.org/r/35564/diff/raw/ 
[7555/7555] -> "35564.patch" [1]
error: patch failed: src/slave/containerizer/containerizer.cpp:276
error: src/slave/containerizer/containerizer.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On June 20, 2015, 7 p.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35567/
> ---
> 
> (Updated June 20, 2015, 7 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-2832
> https://issues.apache.org/jira/browse/MESOS-2832
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.cpp 
> c5ffc49899976225e319f122cb5f7556c3b09d46 
>   src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
>   src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 
>   src/tests/containerizer.cpp 80b910515694c81cf5b1eace1d79a5178cf35426 
> 
> Diff: https://reviews.apache.org/r/35567/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 35565: Refactored os::environment to return a std::map.

[Cody Maloney]

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


also touches on 
https://issues.apache.org/jira/plugins/servlet/mobile#issue/MESOS-2811

- Cody Maloney


On June 20, 2015, 7:01 p.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35565/
> ---
> 
> (Updated June 20, 2015, 7:01 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-2832
> https://issues.apache.org/jira/browse/MESOS-2832
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 1e7aed0b48b8889eb8d042cbd9f907303d2510d3 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> 62987e0df28f28816c59d7cbad89fa2af41ade04 
> 
> Diff: https://reviews.apache.org/r/35565/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 35695: stout: Fixed test names.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [35695]

All tests passed.

- Mesos ReviewBot


On June 20, 2015, 7:14 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35695/
> ---
> 
> (Updated June 20, 2015, 7:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> First noticed `JsonTest.parse`, then found the other cases with the following 
> regex: `TEST.*(.*, [a-z].*`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 
> 60c0336082caf30b2d7943d85ef3bb7a534648d8 
>   3rdparty/libprocess/3rdparty/stout/tests/mac_tests.cpp 
> 4dbe4bab79f7536a6a7a53348f44f8ac888d3d1d 
>   3rdparty/libprocess/3rdparty/stout/tests/os/sendfile_tests.cpp 
> f2386d55c658a5bbdedb6b433c70f7739f6ac0d4 
>   3rdparty/libprocess/3rdparty/stout/tests/os/signals_tests.cpp 
> ad79a1677fb0bb92215e29f98a17c97d5309279c 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> 62987e0df28f28816c59d7cbad89fa2af41ade04 
>   3rdparty/libprocess/3rdparty/stout/tests/proc_tests.cpp 
> d3c9aede8e8a7f419f8eae7a7f52629d5cc1513f 
>   3rdparty/libprocess/3rdparty/stout/tests/thread_tests.cpp 
> 7519b12d7334b16496377f60b2e830401dd0db86 
>   3rdparty/libprocess/3rdparty/stout/tests/uuid_tests.cpp 
> ad1d986de72a51540ebeb56db7eb399fb8e66f87 
> 
> Diff: https://reviews.apache.org/r/35695/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35696: libprocess: Fixed test names.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [35696]

All tests passed.

- Mesos ReviewBot


On June 20, 2015, 7:14 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35696/
> ---
> 
> (Updated June 20, 2015, 7:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> First noticed `JsonTest.parse`, then found the other cases with the following 
> regex: `TEST.*(.*, [a-z].*`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 86b2c7d3beae6671474cf0a5ad52a10334bc9230 
>   3rdparty/libprocess/src/tests/io_tests.cpp 
> 84e5d746220dfa274465632280de87dc5e7f673a 
>   3rdparty/libprocess/src/tests/mutex_tests.cpp 
> 451b234ca46dd91bff6d8463a00a40fb14070db6 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 660af45e7fd45bdf5d43ad9aa54477fd94f87058 
>   3rdparty/libprocess/src/tests/queue_tests.cpp 
> 79741d37cb761593b38f21eef466d0313220d721 
>   3rdparty/libprocess/src/tests/statistics_tests.cpp 
> 0e4960a4e2e807ee9f3f6d83cdbfa6b563352760 
>   3rdparty/libprocess/src/tests/timeseries_tests.cpp 
> fd906b869e8bbf5c07c67c5319dec82681e811b7 
> 
> Diff: https://reviews.apache.org/r/35696/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35696: libprocess: Fixed test names.

[Marco Massenzio]

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

Ship it!


Thanks, Michael!

- Marco Massenzio


On June 20, 2015, 7:14 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35696/
> ---
> 
> (Updated June 20, 2015, 7:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> First noticed `JsonTest.parse`, then found the other cases with the following 
> regex: `TEST.*(.*, [a-z].*`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 86b2c7d3beae6671474cf0a5ad52a10334bc9230 
>   3rdparty/libprocess/src/tests/io_tests.cpp 
> 84e5d746220dfa274465632280de87dc5e7f673a 
>   3rdparty/libprocess/src/tests/mutex_tests.cpp 
> 451b234ca46dd91bff6d8463a00a40fb14070db6 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 660af45e7fd45bdf5d43ad9aa54477fd94f87058 
>   3rdparty/libprocess/src/tests/queue_tests.cpp 
> 79741d37cb761593b38f21eef466d0313220d721 
>   3rdparty/libprocess/src/tests/statistics_tests.cpp 
> 0e4960a4e2e807ee9f3f6d83cdbfa6b563352760 
>   3rdparty/libprocess/src/tests/timeseries_tests.cpp 
> fd906b869e8bbf5c07c67c5319dec82681e811b7 
> 
> Diff: https://reviews.apache.org/r/35696/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35695: stout: Fixed test names.

[Marco Massenzio]

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

Ship it!


Ship It!

- Marco Massenzio


On June 20, 2015, 7:14 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35695/
> ---
> 
> (Updated June 20, 2015, 7:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> First noticed `JsonTest.parse`, then found the other cases with the following 
> regex: `TEST.*(.*, [a-z].*`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 
> 60c0336082caf30b2d7943d85ef3bb7a534648d8 
>   3rdparty/libprocess/3rdparty/stout/tests/mac_tests.cpp 
> 4dbe4bab79f7536a6a7a53348f44f8ac888d3d1d 
>   3rdparty/libprocess/3rdparty/stout/tests/os/sendfile_tests.cpp 
> f2386d55c658a5bbdedb6b433c70f7739f6ac0d4 
>   3rdparty/libprocess/3rdparty/stout/tests/os/signals_tests.cpp 
> ad79a1677fb0bb92215e29f98a17c97d5309279c 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> 62987e0df28f28816c59d7cbad89fa2af41ade04 
>   3rdparty/libprocess/3rdparty/stout/tests/proc_tests.cpp 
> d3c9aede8e8a7f419f8eae7a7f52629d5cc1513f 
>   3rdparty/libprocess/3rdparty/stout/tests/thread_tests.cpp 
> 7519b12d7334b16496377f60b2e830401dd0db86 
>   3rdparty/libprocess/3rdparty/stout/tests/uuid_tests.cpp 
> ad1d986de72a51540ebeb56db7eb399fb8e66f87 
> 
> Diff: https://reviews.apache.org/r/35695/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35565: Refactored os::environment to return a std::map.

[Marco Massenzio]

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

Ship it!


Ship It!

- Marco Massenzio


On June 20, 2015, 7:01 p.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35565/
> ---
> 
> (Updated June 20, 2015, 7:01 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-2832
> https://issues.apache.org/jira/browse/MESOS-2832
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 1e7aed0b48b8889eb8d042cbd9f907303d2510d3 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> 62987e0df28f28816c59d7cbad89fa2af41ade04 
> 
> Diff: https://reviews.apache.org/r/35565/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 35697: mesos: Fixed test names.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [35697]

All tests passed.

- Mesos ReviewBot


On June 20, 2015, 7:14 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35697/
> ---
> 
> (Updated June 20, 2015, 7:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> First noticed `JsonTest.parse`, then found the other cases with the following 
> regex: `TEST.*(.*, [a-z].*`.
> 
> 
> Diffs
> -
> 
>   src/tests/cram_md5_authentication_tests.cpp 
> 992302365682df2c800c8828a21b9d38cb318472 
>   src/tests/credentials_tests.cpp ed672733ff895b1f252fa75d5653a9f3527d5b5f 
>   src/tests/registrar_tests.cpp 97d0b685dd2744cf92b3a463d586e8bcf4c0af1a 
> 
> Diff: https://reviews.apache.org/r/35697/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35699: Added an invariant CHECK_EQ for available resources in HierarchicalAllocator::updateAllocation.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [35699]

All tests passed.

- Mesos ReviewBot


On June 20, 2015, 7:52 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35699/
> ---
> 
> (Updated June 20, 2015, 7:52 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 646ee8c1c0fb824e1d17150b4e96e6281c65358f 
> 
> Diff: https://reviews.apache.org/r/35699/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35701: Minor formatting cleanup in the Master.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [35701]

All tests passed.

- Mesos ReviewBot


On June 20, 2015, 8 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35701/
> ---
> 
> (Updated June 20, 2015, 8 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp 4c8102e3cd75e9284dac3d535545370ca37f502c 
>   src/master/master.cpp 0135c155181546d3cb43e9e05bb874af846d928d 
> 
> Diff: https://reviews.apache.org/r/35701/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35702: [WIP] Add /reserve HTTP endpoint to the master.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [35702]

All tests passed.

- Mesos ReviewBot


On June 20, 2015, 8:19 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35702/
> ---
> 
> (Updated June 20, 2015, 8:19 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris 
> Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2600
> https://issues.apache.org/jira/browse/MESOS-2600
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is still a work in progress, but I'm sharing to gather some feedback 
> around:
> 
> (1) I've added `updateAvailable` to the allocator API. Is this necessary or 
> is there maybe a better way?
> (2) To determine the amount of available resources, I used: `available = 
> total - (offered + used)`.
> Is this still correct with oversubscription in the picture?
> (3) I'm creating an `Offer.Operation` to perform the necessary updates in the 
> allocator and such.
> Feels weird to use an "offer" operation when there's not an actual offer. 
> Is this fine for now?
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp 92de1af414321281b00eaa6f129e5e3e2c849448 
>   src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
>   src/master/allocator/mesos/allocator.hpp 
> 6cfa04650d91a80211cfbc0809236f9438926c78 
>   src/master/allocator/mesos/hierarchical.hpp 
> 646ee8c1c0fb824e1d17150b4e96e6281c65358f 
>   src/master/http.cpp b893013ddd052cb58c520ac0328f4a5f0fed862e 
>   src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
>   src/master/master.cpp 0135c155181546d3cb43e9e05bb874af846d928d 
>   src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 
>   src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 
>   src/tests/mesos.hpp 9157ac079808d2686592e54ea26a26e6a0825ed3 
>   src/tests/reserve_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/35702/diff/
> 
> 
> Testing
> ---
> 
> Added `src/tests/reserve_tests.cpp` which still need to be fleshed out for 
> error cases (missing parameters, unauthorized client, etc).
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35703: Set refuse seconds on the correct filter in reservation test.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [35703]

All tests passed.

- Mesos ReviewBot


On June 20, 2015, 8:51 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35703/
> ---
> 
> (Updated June 20, 2015, 8:51 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `set_refuse_seconds` should have been called on `filtersForever` rather than 
> `filters`.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp 755a375346e0be290d4b8ffac3ecf5e7d191970d 
> 
> Diff: https://reviews.apache.org/r/35703/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 33339: Add a Java example framework to test persistent volumes.

[haosdent huang]

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

(Updated June 21, 2015, 5:17 a.m.)


Review request for mesos, Adam B, Jie Yu, and Marco Massenzio.


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


Repository: mesos


Description
---

Add a Java example framework to test persistent volumes.


Diffs (updated)
-

  configure.ac 563e9c529444b3e980db6d04173f0d016a737c74 
  src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
  src/examples/java/TestPersistentVolumeFramework.java PRE-CREATION 
  src/examples/java/test-persistent-volume-framework.in PRE-CREATION 
  src/tests/examples_tests.cpp 2ff6e7a449cc5037f9a3c8d6938855c35e389cca 
  src/tests/java_persistent_volume_framework_test.sh PRE-CREATION 

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


Testing
---

make check


Thanks,

haosdent huang

Re: Review Request 33339: Add a Java example framework to test persistent volumes.

[haosdent huang]

> On June 1, 2015, 11:32 p.m., Marco Massenzio wrote:
> > This is great -sorry it took so long to get to do a review.
> > 
> > Thanks for doing it, I'm quite looking forward to using it to learning more 
> > about the Persistent Framework :)
> > it would be great if we could have a bit more comments in the code to help 
> > all other newbies.
> > 
> > My review was fairly narrowly focused on Java style etc. - I'm expecting 
> > one of the commiters will be able to give you feedback about using the 
> > framework.
> 
> Jie Yu wrote:
> Sorry, this is my fault. I really don't have cycle for this at this 
> moment. Marco, mind sherperding this patch?
> 
> Jie Yu wrote:
> The logic of this patch should match that in 
> src/examples/persistent_volume_framework.cpp
> 
> Marco Massenzio wrote:
> Jie - I would be absolutely happy to do that, with one minor hitch: I'm 
> not a committer :)
> Happy to help out wherever I can, though!
> 
> haosdent huang wrote:
> Thank you very much for your review, let me update it. @marco
> 
> haosdent huang wrote:
> @marco Thank you for your review again. But some logic are follow the 
> exists Java examples and persistent_volume_framework.cpp . I am not sure 
> change it are correct or not. Anyway, I think I need to reflect this patch to 
> make it more clear. After I reflect it, I would @you and looking forward your 
> review again. Thanks a lot.
> 
> Marco Massenzio wrote:
> I understand that - and therein lies the problem :)
> As you proved, code in 'examples' is widely "used as a template" (aka, 
> copied) so it would be great if we could set an... example, by providing 
> great code there.
> 
> Translating Java almost verbatim from C++ actually results in 
> non-idiomatic constructs that either (a) look weird to Java practitioners or 
> (b) engender bad practices (that are then propagated by the "oh, but that's 
> how it's done in the other examples" - sounds familiar? :)
> 
> My suggestions (and I stress it here, they are "suggestions" - if 
> committers with more Java experience than I are happy with your code, then 
> that's just fine) are to provide some great examples, that improve on 
> existing code.
> 
> Thanks for doing this!
> 
> Adam B wrote:
> Thanks haosdent for coding this up, and thanks Marco for the initial 
> review. As a Java/C++ polyglot, I'll volunteer to Shepherd this patch, if Jie 
> will help verify that it properly tests his feature.
> Unfortunately, it seems the authors of the original Java examples/tests 
> were not hardened Java developers, and their poor style habits have been 
> copied into subsequent examples. I won't ask haosdent to solve all of those 
> issues, or else this patch would be indefinitely delayed. As long as it's no 
> worse than the other examples, and adequately demonstrates/tests the feature, 
> I'll let it through.

@marco, @adam-mesos I reflect the code, could you review it again? Thank you 
very much.


- haosdent


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


On June 21, 2015, 5:17 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9/
> ---
> 
> (Updated June 21, 2015, 5:17 a.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, and Marco Massenzio.
> 
> 
> Bugs: MESOS-2610
> https://issues.apache.org/jira/browse/MESOS-2610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a Java example framework to test persistent volumes.
> 
> 
> Diffs
> -
> 
>   configure.ac 563e9c529444b3e980db6d04173f0d016a737c74 
>   src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
>   src/examples/java/TestPersistentVolumeFramework.java PRE-CREATION 
>   src/examples/java/test-persistent-volume-framework.in PRE-CREATION 
>   src/tests/examples_tests.cpp 2ff6e7a449cc5037f9a3c8d6938855c35e389cca 
>   src/tests/java_persistent_volume_framework_test.sh PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/9/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 33339: Add a Java example framework to test persistent volumes.

[haosdent huang]

> On June 1, 2015, 11:32 p.m., Marco Massenzio wrote:
> > src/examples/java/TestPersistentVolumeFramework.java, line 239
> > 
> >
> > unless I'm mistaken and this is a unit test (is it?) can we please 
> > rename this class in a more meaningful way?
> > 
> > TestXxxx should be reserved for test classes.
> 
> haosdent huang wrote:
> Hmm, name it to TestPersistentVolumeScheduler is follow other exist 
> examples. Like `TestScheduler` in `TestFramework.java`
> 
> Adam B wrote:
> At least a few of the other example frameworks are run as part of the 
> unit test suite, under ExamplesTest: 
> https://github.com/apache/mesos/blob/0.22.1/src/tests/examples_tests.cpp#L36
> See also: 
> https://github.com/apache/mesos/blob/0.22.1/src/examples/java/test-framework.in
> I'm not asking you to add TestPersistentVolumeFramework to ExamplesTests 
> (although it would be awesome!). I'm just explaining why some of the 
> frameworks are named TestXxx; others are just copycats.

Hmm, it could run as a unit test.


- haosdent


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


On June 21, 2015, 5:17 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9/
> ---
> 
> (Updated June 21, 2015, 5:17 a.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, and Marco Massenzio.
> 
> 
> Bugs: MESOS-2610
> https://issues.apache.org/jira/browse/MESOS-2610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a Java example framework to test persistent volumes.
> 
> 
> Diffs
> -
> 
>   configure.ac 563e9c529444b3e980db6d04173f0d016a737c74 
>   src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
>   src/examples/java/TestPersistentVolumeFramework.java PRE-CREATION 
>   src/examples/java/test-persistent-volume-framework.in PRE-CREATION 
>   src/tests/examples_tests.cpp 2ff6e7a449cc5037f9a3c8d6938855c35e389cca 
>   src/tests/java_persistent_volume_framework_test.sh PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/9/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 33339: Add a Java example framework to test persistent volumes.

[haosdent huang]

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

(Updated June 21, 2015, 5:37 a.m.)


Review request for mesos, Adam B, Jie Yu, and Marco Massenzio.


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


Repository: mesos


Description
---

Add a Java example framework to test persistent volumes.


Diffs (updated)
-

  configure.ac 563e9c529444b3e980db6d04173f0d016a737c74 
  src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
  src/examples/java/TestPersistentVolumeFramework.java PRE-CREATION 
  src/examples/java/test-persistent-volume-framework.in PRE-CREATION 
  src/tests/examples_tests.cpp 2ff6e7a449cc5037f9a3c8d6938855c35e389cca 
  src/tests/java_persistent_volume_framework_test.sh PRE-CREATION 

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


Testing
---

make check


Thanks,

haosdent huang

Re: Review Request 33339: Add a Java example framework to test persistent volumes.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [9]

All tests passed.

- Mesos ReviewBot


On June 21, 2015, 5:37 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9/
> ---
> 
> (Updated June 21, 2015, 5:37 a.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, and Marco Massenzio.
> 
> 
> Bugs: MESOS-2610
> https://issues.apache.org/jira/browse/MESOS-2610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a Java example framework to test persistent volumes.
> 
> 
> Diffs
> -
> 
>   configure.ac 563e9c529444b3e980db6d04173f0d016a737c74 
>   src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
>   src/examples/java/TestPersistentVolumeFramework.java PRE-CREATION 
>   src/examples/java/test-persistent-volume-framework.in PRE-CREATION 
>   src/tests/examples_tests.cpp 2ff6e7a449cc5037f9a3c8d6938855c35e389cca 
>   src/tests/java_persistent_volume_framework_test.sh PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/9/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>