Review Request 37562: The revocable resource information also are missed for slave node in monitoring doc , fix it in this patch.

2015-08-17 Thread Yong Qiao Wang

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

Review request for mesos.


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


Repository: mesos


Description
---

The revocable resource information also are missed for slave node in monitoring 
doc ,fix it in this patch.


Diffs
-

  docs/monitoring.md 6ddf07d334ece4d4dc83d805ac0c51c9579c47c9 

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


Testing
---


Thanks,

Yong Qiao Wang



Re: Review Request 37512: Added user doc for Scheduler HTTP API.

2015-08-17 Thread Anand Mazumdar

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

Ship it!


LGTM

- Anand Mazumdar


On Aug. 18, 2015, 3:08 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37512/
> ---
> 
> (Updated Aug. 18, 2015, 3:08 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Ben Mahler.
> 
> 
> Bugs: MESOS-3281
> https://issues.apache.org/jira/browse/MESOS-3281
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mostly copy pasted from the design doc with some formatting.
> 
> 
> Diffs
> -
> 
>   docs/scheduler_http_api.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37512/diff/
> 
> 
> Testing
> ---
> 
> gist: https://gist.github.com/vinodkone/5ae803ee35274ca46a3e
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 36404: Added support for peek() to process::io

2015-08-17 Thread Artem Harutyunyan


> On July 26, 2015, 6:18 a.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/src/tests/io_tests.cpp, line 377
> > 
> >
> > we can get rid of some of these literals by:
> > s/3/sizeof(data)/
> > What do you think?

I followed what was done in the read test here.


- Artem


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


On Aug. 5, 2015, 8:26 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36404/
> ---
> 
> (Updated Aug. 5, 2015, 8:26 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Joseph Wu.
> 
> 
> Bugs: MESOS-2964
> https://issues.apache.org/jira/browse/MESOS-2964
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> JIRA: https://issues.apache.org/jira/browse/MESOS-2964
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/io.hpp 
> 975923f40f82357f31b89428f24d01df6a8ac9fc 
>   3rdparty/libprocess/src/io.cpp 4a6e18a17012994d358099ad32d4c282fea3b0b1 
>   3rdparty/libprocess/src/tests/io_tests.cpp 
> c642bab9e2845668767ad237985cb9ce1109 
> 
> Diff: https://reviews.apache.org/r/36404/diff/
> 
> 
> Testing
> ---
> 
> - Added a test case for process::io::peek
> - make check
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>



Re: Review Request 37512: Added user doc for Scheduler HTTP API.

2015-08-17 Thread Vinod Kone


> On Aug. 17, 2015, 6:37 p.m., Anand Mazumdar wrote:
> > docs/scheduler_http_api.md, line 97
> > 
> >
> > The other example don't have this line. Can we add this to the other 
> > example call except Subscribe too ?

doesn't actually look like we send "connection: close". removed it.


- Vinod


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


On Aug. 18, 2015, 3:08 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37512/
> ---
> 
> (Updated Aug. 18, 2015, 3:08 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Ben Mahler.
> 
> 
> Bugs: MESOS-3281
> https://issues.apache.org/jira/browse/MESOS-3281
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mostly copy pasted from the design doc with some formatting.
> 
> 
> Diffs
> -
> 
>   docs/scheduler_http_api.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37512/diff/
> 
> 
> Testing
> ---
> 
> gist: https://gist.github.com/vinodkone/5ae803ee35274ca46a3e
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 37512: Added user doc for Scheduler HTTP API.

2015-08-17 Thread Vinod Kone

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

(Updated Aug. 18, 2015, 3:08 a.m.)


Review request for mesos, Anand Mazumdar and Ben Mahler.


Changes
---

addressed coments. also, updated the gist.


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


Repository: mesos


Description
---

Mostly copy pasted from the design doc with some formatting.


Diffs (updated)
-

  docs/scheduler_http_api.md PRE-CREATION 

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


Testing
---

gist: https://gist.github.com/vinodkone/5ae803ee35274ca46a3e


Thanks,

Vinod Kone



Re: Review Request 37559: Fixed mutex deadlock issue in ~scheduler::Mesos().

2015-08-17 Thread Ben Mahler

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

Ship it!



src/scheduler/scheduler.cpp (lines 187 - 189)


This comment ended up confusing me further, seems to suggest that we 
ideally would be doing the await, but IMO it doesn't buy us anything?


- Ben Mahler


On Aug. 17, 2015, 11:29 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37559/
> ---
> 
> (Updated Aug. 17, 2015, 11:29 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3273
> https://issues.apache.org/jira/browse/MESOS-3273
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed the await on the mutex lock to avoid deadlock.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp d3d6682efdf4b5a6686bbe73a1e47e0559f7e309 
> 
> Diff: https://reviews.apache.org/r/37559/diff/
> 
> 
> Testing
> ---
> 
> Ran this test in repetition. It doesn't block here anymore, but there is 
> still another deadlock issue that I described in the bug.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Review Request 37559: Fixed mutex deadlock issue in ~scheduler::Mesos().

2015-08-17 Thread Vinod Kone

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Removed the await on the mutex lock to avoid deadlock.


Diffs
-

  src/scheduler/scheduler.cpp d3d6682efdf4b5a6686bbe73a1e47e0559f7e309 

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


Testing
---

Ran this test in repetition. It doesn't block here anymore, but there is still 
another deadlock issue that I described in the bug.


Thanks,

Vinod Kone



Re: Review Request 37558: Updated Proto <-> JSON conversion to use base64 for 'bytes' fields.

2015-08-17 Thread Vinod Kone

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

Ship it!


Ship It!

- Vinod Kone


On Aug. 17, 2015, 10:51 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37558/
> ---
> 
> (Updated Aug. 17, 2015, 10:51 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3284
> https://issues.apache.org/jira/browse/MESOS-3284
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See MESOS-3284 for context.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
> 82eae873b9f0d5a7232085282f35b0f22dcd891d 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
> 2b98945a08f7bbf60967cf8d1c7e5970096c05bd 
> 
> Diff: https://reviews.apache.org/r/37558/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>



Re: Review Request 37557: Updated mesos to reflect base64::decode returning a Try.

2015-08-17 Thread Vinod Kone

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

Ship it!


Ship It!

- Vinod Kone


On Aug. 17, 2015, 10:49 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37557/
> ---
> 
> (Updated Aug. 17, 2015, 10:49 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3284
> https://issues.apache.org/jira/browse/MESOS-3284
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See above.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 877c1f9129a7561fefa8f0129e0e8e24bd313138 
> 
> Diff: https://reviews.apache.org/r/37557/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>



Re: Review Request 37556: Updated base64::decode to reject forbidden characters.

2015-08-17 Thread Vinod Kone

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

Ship it!



3rdparty/libprocess/3rdparty/stout/include/stout/base64.hpp (line 94)


s/,/;/

Can you make this a TODO? I'm assuming it's non-trivial to fix this?



3rdparty/libprocess/3rdparty/stout/include/stout/base64.hpp (line 95)


s/the// ?


- Vinod Kone


On Aug. 17, 2015, 10:49 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37556/
> ---
> 
> (Updated Aug. 17, 2015, 10:49 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3284
> https://issues.apache.org/jira/browse/MESOS-3284
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `base64::decode` should return a Try to capture that invalid characters 
> cannot be decoded.
> 
> Note that base64:: could use some refactoring for clarity.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/base64.hpp 
> 110cb49fe0a1306cfa81ffe7b542f75d69b7 
>   3rdparty/libprocess/3rdparty/stout/tests/base64_tests.cpp 
> ff3d31b0ba748ae3dd346d1b70231cbaabd3189a 
> 
> Diff: https://reviews.apache.org/r/37556/diff/
> 
> 
> Testing
> ---
> 
> Updated the test, note that this still misses validation of the padding 
> suffix.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>



Re: Review Request 37336: [WIP] Added `wait()` method to process::Subprocess

2015-08-17 Thread Ben Mahler

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


In the same vein as os::shell, we should probably introduce an 'os' namespace 
in libprocess for asynchronous os utilities. In this case, process::os::shell 
which returns a Future of the output (although, ideally ).

- Ben Mahler


On Aug. 15, 2015, 2:02 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37336/
> ---
> 
> (Updated Aug. 15, 2015, 2:02 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3035
> https://issues.apache.org/jira/browse/MESOS-3035
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Jira: MESOS-3035
> 
> The original API for `process::Subprocess` still left a lot of legwork
> to do for the caller; we have now added a `wait(Duration timeout)` method
> that returns a `CommandResult` (also introduced with this patch) which
> contains useful information about the command invocation; the exit code;
> stdout and, optionally, stderr too.
> 
> The `wait()` method will wait for both the process to terminate, and
> stdout/stderr to be available to read from; it also "unpacks" them into
> ready-to-use `string`s.
> 
> This is still WIP as I'm seeing some unusual behavior and I'd like to discuss 
> with someone more expert on libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> 310cb4f8e4e2faa5545dffd196d7490c868bc5d6 
>   3rdparty/libprocess/src/subprocess.cpp 
> d6ea62ed1c914d34e0e189395831c86fff8aac22 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> ab7515325e5db0a4fd222bb982f51243d7b7e39d 
> 
> Diff: https://reviews.apache.org/r/37336/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>



Review Request 37558: Updated Proto <-> JSON conversion to use base64 for 'bytes' fields.

2015-08-17 Thread Ben Mahler

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

See MESOS-3284 for context.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
82eae873b9f0d5a7232085282f35b0f22dcd891d 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
2b98945a08f7bbf60967cf8d1c7e5970096c05bd 

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


Testing
---

make check


Thanks,

Ben Mahler



Review Request 37556: Updated base64::decode to reject forbidden characters.

2015-08-17 Thread Ben Mahler

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

`base64::decode` should return a Try to capture that invalid characters cannot 
be decoded.

Note that base64:: could use some refactoring for clarity.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/base64.hpp 
110cb49fe0a1306cfa81ffe7b542f75d69b7 
  3rdparty/libprocess/3rdparty/stout/tests/base64_tests.cpp 
ff3d31b0ba748ae3dd346d1b70231cbaabd3189a 

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


Testing
---

Updated the test, note that this still misses validation of the padding suffix.


Thanks,

Ben Mahler



Review Request 37557: Updated mesos to reflect base64::decode returning a Try.

2015-08-17 Thread Ben Mahler

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

See above.


Diffs
-

  src/master/http.cpp 877c1f9129a7561fefa8f0129e0e8e24bd313138 

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


Testing
---

make check


Thanks,

Ben Mahler



Re: Review Request 37541: Add TraceEvent API

2015-08-17 Thread Cong Wang

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

(Updated Aug. 17, 2015, 10:34 p.m.)


Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu.


Changes
---

Detect debugfs or tracefs mount at run-time.


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


Repository: mesos


Description
---

Based on the PerfEvent API's, add API for Linux kernel trace events, especially 
the schedule trace events.


Diffs (updated)
-

  src/linux/perf.hpp b77b61d7048b12cea4586bcf802cbc2ff634331b 
  src/linux/perf.cpp cb1a13d0b1754a50f0121bfda522056ff8c3e3c8 
  src/tests/containerizer/perf_tests.cpp 
6b3d70f3e7ea8f59f94e6961491d4e9a730e3334 

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


Testing
---

make check


Thanks,

Cong Wang



Re: Review Request 37426: MESOS-3251 : Fixing "host" field of request header.

2015-08-17 Thread Timothy Chen

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

Ship it!


Ship It!

- Timothy Chen


On Aug. 17, 2015, 9:56 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37426/
> ---
> 
> (Updated Aug. 17, 2015, 9:56 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Timothy Chen.
> 
> 
> Bugs: MESOS-3251
> https://issues.apache.org/jira/browse/MESOS-3251
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently libprocess http API sets the "Host" header field from the peer 
> socket
> address (IP:port). The problem is that socket address might not be right HTTP
> server and might be just a proxy.
>   Changed the logic to look for http server URL's domain first to populate the
> "Host" field.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/http.cpp 660950fc9b66e8bca24d0888dda1f20b44d8ff49 
> 
> Diff: https://reviews.apache.org/r/37426/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>



Re: Review Request 37505: Fix broken health check in docker executor.

2015-08-17 Thread Timothy Chen

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


Hi haosdent, thanks for working on this but I think running the healthcheck 
outside of the container doesn't make much sense to me.
I think we should try to run it inside of the container (docker exec), but 
since docker exec is introduced until few later versions we also need to check 
to make sure the right version is running on the slave.

- Timothy Chen


On Aug. 16, 2015, 9:12 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37505/
> ---
> 
> (Updated Aug. 16, 2015, 9:12 a.m.)
> 
> 
> Review request for mesos, Adam B and Timothy Chen.
> 
> 
> Bugs: MESOS-3136
> https://issues.apache.org/jira/browse/MESOS-3136
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix broken health check in docker executor.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 256d53d59d5cda63bbeb8c987ce0019e24b9fb77 
>   src/launcher/executor.cpp 9fa7dcfc39a6706f545b3328e468d9cd25d603ae 
>   src/tests/health_check_tests.cpp 157a56aa06677d8b7a2cef53b29ed05cb4b5d8ea 
> 
> Diff: https://reviews.apache.org/r/37505/diff/
> 
> 
> Testing
> ---
> 
> # Add a new test, HealthCheckTest.ROOT_DOCKER_DockerHealthyTask
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="HealthCheckTest.ROOT_DOCKER_DockerHealthyTask" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 37426: MESOS-3251 : Fixing "host" field of request header.

2015-08-17 Thread Jojy Varghese

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

(Updated Aug. 17, 2015, 9:56 p.m.)


Review request for mesos, Anand Mazumdar and Timothy Chen.


Changes
---

Added checks for standard ports.


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


Repository: mesos


Description
---

Currently libprocess http API sets the "Host" header field from the peer socket
address (IP:port). The problem is that socket address might not be right HTTP
server and might be just a proxy.
  Changed the logic to look for http server URL's domain first to populate the
"Host" field.


Diffs (updated)
-

  3rdparty/libprocess/src/http.cpp 660950fc9b66e8bca24d0888dda1f20b44d8ff49 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 37549: Add jenkins script to run docker tests.

2015-08-17 Thread Timothy Chen


> On Aug. 17, 2015, 9:33 p.m., Vinod Kone wrote:
> > 2 high level comments.
> > 
> > Why did you write a whole new script instead of making the current 
> > jenkins_build.sh work? This is 90% duplicated code which makes me sad.
> > 
> > Also, we shouldn't really depend on non-ASF hosted code (mesosphere's 
> > wrapdocker) here. Can that script be committed to this repo?

Hi Vinod,

I just talked to Artem and I'm actually going to follow what he has done around 
mesos-build-images and build a specific image for docker unit tests so that we 
can simply just reuse the same build_jenkins_tests script but with a new docker 
image.

And this new image is basically just includes the wrapdocker (which comes from 
Docker's dind image, allows us to run docker in docker) and set 
GTEST_FILTER=*DOCKER*.

Hopefully this makes you happy :)


- Timothy


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


On Aug. 17, 2015, 8:16 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37549/
> ---
> 
> (Updated Aug. 17, 2015, 8:16 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add jenkins script to run docker tests.
> 
> 
> Diffs
> -
> 
>   support/docker_tests_jenkins_build.sh PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37549/diff/
> 
> 
> Testing
> ---
> 
> ./support/docker_jenkins_build_test.sh in ubuntu 14.04
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 37549: Add jenkins script to run docker tests.

2015-08-17 Thread Vinod Kone

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


2 high level comments.

Why did you write a whole new script instead of making the current 
jenkins_build.sh work? This is 90% duplicated code which makes me sad.

Also, we shouldn't really depend on non-ASF hosted code (mesosphere's 
wrapdocker) here. Can that script be committed to this repo?

- Vinod Kone


On Aug. 17, 2015, 8:16 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37549/
> ---
> 
> (Updated Aug. 17, 2015, 8:16 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add jenkins script to run docker tests.
> 
> 
> Diffs
> -
> 
>   support/docker_tests_jenkins_build.sh PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37549/diff/
> 
> 
> Testing
> ---
> 
> ./support/docker_jenkins_build_test.sh in ubuntu 14.04
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 37532: Add QUIESCE call interface to the scheduler

2015-08-17 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [37532]

All tests passed.

- Mesos ReviewBot


On Aug. 17, 2015, 1:47 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37532/
> ---
> 
> (Updated Aug. 17, 2015, 1:47 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is just part of MESOS-3037, this patch only add the interface
> of QUIESCE call.
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler.hpp ee198b6955882f4f31466ca05429ca16fbf2f5cd 
>   include/mesos/scheduler/scheduler.proto 
> 89daf8a6b74057ee156b3ad691397e76fcb835b8 
>   include/mesos/v1/scheduler/scheduler.proto 
> bd5e82a614b1163b29f9b20e562208efa1ba4b55 
>   src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be 
>   src/master/master.cpp c5e6c6f3304060d4c92d52851951f10bc432500e 
>   src/sched/sched.cpp 012af0508eeceeccd168b29f36fa258d20b28c21 
>   src/tests/scheduler_tests.cpp 77c26353afc33f5099be2d1e597ffc630e559968 
> 
> Diff: https://reviews.apache.org/r/37532/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 37555: Included /usr/bin/sh in the test root filesystem.

2015-08-17 Thread Vinod Kone

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

Ship it!


Ship It!

- Vinod Kone


On Aug. 17, 2015, 8:56 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37555/
> ---
> 
> (Updated Aug. 17, 2015, 8:56 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3050
> https://issues.apache.org/jira/browse/MESOS-3050
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Included /usr/bin/sh in the test root filesystem.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/rootfs.hpp 55dd4964cfb1ca0e5f7b7616ccc6d5ad2be135d7 
> 
> Diff: https://reviews.apache.org/r/37555/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 37547: Used execlp instead of execl to exec processes in Mesos.

2015-08-17 Thread Vinod Kone

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

Ship it!


Ship It!

- Vinod Kone


On Aug. 17, 2015, 7:49 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37547/
> ---
> 
> (Updated Aug. 17, 2015, 7:49 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3050
> https://issues.apache.org/jira/browse/MESOS-3050
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used execlp instead of execl to exec processes in Mesos.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 9fa7dcfc39a6706f545b3328e468d9cd25d603ae 
>   src/slave/containerizer/mesos/launch.cpp 
> 96ca47dff69d7706b570a96244549164918d2c2f 
>   src/tests/containerizer/isolator_tests.cpp 
> fdb706482d2edb89daf4f5a7cd14ee2381b57957 
> 
> Diff: https://reviews.apache.org/r/37547/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 37545: Used execlp instead of execl to exec processes in stout.

2015-08-17 Thread Vinod Kone

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

Ship it!


Ship It!

- Vinod Kone


On Aug. 17, 2015, 7:48 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37545/
> ---
> 
> (Updated Aug. 17, 2015, 7:48 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3050
> https://issues.apache.org/jira/browse/MESOS-3050
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used execlp instead of execl to exec processes in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp 
> 5592e10746f0e23ef6f6d5d5e890bd91091051d6 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> c848268dc8fb595f1f4d9e81ef0179b722bdba4d 
> 
> Diff: https://reviews.apache.org/r/37545/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 37546: Used execlp instead of execl to exec processes in libprocess.

2015-08-17 Thread Vinod Kone

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

Ship it!


Ship It!

- Vinod Kone


On Aug. 17, 2015, 7:49 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37546/
> ---
> 
> (Updated Aug. 17, 2015, 7:49 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3050
> https://issues.apache.org/jira/browse/MESOS-3050
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used execlp instead of execl to exec processes in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> 310cb4f8e4e2faa5545dffd196d7490c868bc5d6 
> 
> Diff: https://reviews.apache.org/r/37546/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 37544: Fixed a bug in port mapping tests due to os::shell refactor.

2015-08-17 Thread Vinod Kone

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

Ship it!


Ship It!

- Vinod Kone


On Aug. 17, 2015, 7:48 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37544/
> ---
> 
> (Updated Aug. 17, 2015, 7:48 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a bug in port mapping tests due to os::shell refactor.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/port_mapping_tests.cpp 
> d3526b886c3a6d354b698013c7c59cd1fbda3cbb 
> 
> Diff: https://reviews.apache.org/r/37544/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-08-17 Thread Timothy Chen

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

Ship it!


Ship It!


src/linux/cgroups.cpp (line 1686)


More like a code suggestion, looks like reap is always called after kill, 
and just waiting on all the statuses you gathered.

I wonder why not just move collect inside of  kill(processes) in the end 
and return that instead of another callback?


- Timothy Chen


On Aug. 13, 2015, 1:52 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36620/
> ---
> 
> (Updated Aug. 13, 2015, 1:52 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Timothy Chen.
> 
> 
> Bugs: MESOS-3086
> https://issues.apache.org/jira/browse/MESOS-3086
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP Added Non-Freezeer Task Killer.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp 6ef42ed1bc719f334d1ac6e90919a1bc1840d31f 
> 
> Diff: https://reviews.apache.org/r/36620/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> + manual tests
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



Review Request 37541: Add TraceEvent API

2015-08-17 Thread Cong Wang

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

Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu.


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


Repository: mesos


Description
---

Based on the PerfEvent API's, add API for Linux kernel trace events, especially 
the schedule trace events.


Diffs
-

  src/linux/perf.hpp b77b61d7048b12cea4586bcf802cbc2ff634331b 
  src/linux/perf.cpp cb1a13d0b1754a50f0121bfda522056ff8c3e3c8 
  src/tests/containerizer/perf_tests.cpp 
6b3d70f3e7ea8f59f94e6961491d4e9a730e3334 

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


Testing
---

make check


Thanks,

Cong Wang



Review Request 37540: Add PerfEvent API

2015-08-17 Thread Cong Wang

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

Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu.


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


Repository: mesos


Description
---

Abstract Linux kernel perf event API and provide API to collect schedule events.


Diffs
-

  src/linux/perf.hpp b77b61d7048b12cea4586bcf802cbc2ff634331b 
  src/linux/perf.cpp cb1a13d0b1754a50f0121bfda522056ff8c3e3c8 
  src/tests/containerizer/perf_tests.cpp 
6b3d70f3e7ea8f59f94e6961491d4e9a730e3334 

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


Testing
---

make check


Thanks,

Cong Wang



Re: Review Request 37555: Included /usr/bin/sh in the test root filesystem.

2015-08-17 Thread Jie Yu

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

(Updated Aug. 17, 2015, 8:56 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Removed the tailing comma.


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


Repository: mesos


Description
---

Included /usr/bin/sh in the test root filesystem.


Diffs (updated)
-

  src/tests/containerizer/rootfs.hpp 55dd4964cfb1ca0e5f7b7616ccc6d5ad2be135d7 

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


Testing
---

sudo make check


Thanks,

Jie Yu



Re: Review Request 37555: Included /usr/bin/sh in the test root filesystem.

2015-08-17 Thread Jie Yu


> On Aug. 17, 2015, 8:52 p.m., Marco Massenzio wrote:
> > src/tests/containerizer/rootfs.hpp, line 108
> > 
> >
> > why the trailing comma?

My bad. Removed.


> On Aug. 17, 2015, 8:52 p.m., Marco Massenzio wrote:
> > src/tests/containerizer/rootfs.hpp, line 119
> > 
> >
> > this seems very 'ad-hoc'?
> > not sure what `rootfs->add()` does, but could we just add `/usr/bin` to 
> > `directories` above, maybe?
> > 
> > Not a big deal, though.

I initially did add /usr/bin to 'directories', but found out that copying the 
whole '/usr/bin' slows down the tests a lot ('/usr/bin' is huge on my dev 
host). So I did this change.


- Jie


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


On Aug. 17, 2015, 8:47 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37555/
> ---
> 
> (Updated Aug. 17, 2015, 8:47 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3050
> https://issues.apache.org/jira/browse/MESOS-3050
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Included /usr/bin/sh in the test root filesystem.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/rootfs.hpp 55dd4964cfb1ca0e5f7b7616ccc6d5ad2be135d7 
> 
> Diff: https://reviews.apache.org/r/37555/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 37555: Included /usr/bin/sh in the test root filesystem.

2015-08-17 Thread Marco Massenzio

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

Ship it!


a couple of nits, but LGTM.


src/tests/containerizer/rootfs.hpp (line 108)


why the trailing comma?



src/tests/containerizer/rootfs.hpp (line 119)


this seems very 'ad-hoc'?
not sure what `rootfs->add()` does, but could we just add `/usr/bin` to 
`directories` above, maybe?

Not a big deal, though.


- Marco Massenzio


On Aug. 17, 2015, 8:47 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37555/
> ---
> 
> (Updated Aug. 17, 2015, 8:47 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3050
> https://issues.apache.org/jira/browse/MESOS-3050
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Included /usr/bin/sh in the test root filesystem.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/rootfs.hpp 55dd4964cfb1ca0e5f7b7616ccc6d5ad2be135d7 
> 
> Diff: https://reviews.apache.org/r/37555/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 37426: MESOS-3251 : Fixing "host" field of request header.

2015-08-17 Thread Timothy Chen

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



3rdparty/libprocess/src/http.cpp (line 798)


Let's add the port to make sure we're adhering to the HTTP 1.1 standard


- Timothy Chen


On Aug. 13, 2015, 12:46 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37426/
> ---
> 
> (Updated Aug. 13, 2015, 12:46 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Timothy Chen.
> 
> 
> Bugs: MESOS-3251
> https://issues.apache.org/jira/browse/MESOS-3251
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently libprocess http API sets the "Host" header field from the peer 
> socket
> address (IP:port). The problem is that socket address might not be right HTTP
> server and might be just a proxy.
>   Changed the logic to look for http server URL's domain first to populate the
> "Host" field.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/http.cpp 4dcbd74b894b483c4d166c23fac55ac8dba75166 
> 
> Diff: https://reviews.apache.org/r/37426/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>



Review Request 37555: Included /usr/bin/sh in the test root filesystem.

2015-08-17 Thread Jie Yu

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

Review request for mesos, Ben Mahler and Vinod Kone.


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


Repository: mesos


Description
---

Included /usr/bin/sh in the test root filesystem.


Diffs
-

  src/tests/containerizer/rootfs.hpp 55dd4964cfb1ca0e5f7b7616ccc6d5ad2be135d7 

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


Testing
---

sudo make check


Thanks,

Jie Yu



Re: Review Request 37512: Added user doc for Scheduler HTTP API.

2015-08-17 Thread Guangya Liu

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



docs/scheduler_http_api.md (line 12)


s/Mesos master/Mesos Master

Also I think that we should make master unified in this document. Currently 
there are "master", "Master", "Mesos master", "Mesos Master", we may want to 
select one as the final name of master and apply to the whole document.


- Guangya Liu


On 八月 17, 2015, 6:26 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37512/
> ---
> 
> (Updated 八月 17, 2015, 6:26 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Ben Mahler.
> 
> 
> Bugs: MESOS-3281
> https://issues.apache.org/jira/browse/MESOS-3281
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mostly copy pasted from the design doc with some formatting.
> 
> 
> Diffs
> -
> 
>   docs/scheduler_http_api.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37512/diff/
> 
> 
> Testing
> ---
> 
> gist: https://gist.github.com/vinodkone/5ae803ee35274ca46a3e
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Review Request 37549: Add jenkins script to run docker tests.

2015-08-17 Thread Timothy Chen

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

Review request for mesos.


Repository: mesos


Description
---

Add jenkins script to run docker tests.


Diffs
-

  support/docker_tests_jenkins_build.sh PRE-CREATION 

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


Testing
---

./support/docker_jenkins_build_test.sh in ubuntu 14.04


Thanks,

Timothy Chen



Re: Review Request 37531: MESOS-3070

2015-08-17 Thread Guangya Liu

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



src/master/master.cpp (line 3218)


Does this logic still needed? I see that the taskTag logic should already 
covered this?



src/messages/messages.proto (line 65)


I want some comments here for what is taskTag


- Guangya Liu


On Aug. 17, 2015, 1:28 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37531/
> ---
> 
> (Updated Aug. 17, 2015, 1:28 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3070
> https://issues.apache.org/jira/browse/MESOS-3070
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3070 (Master CHECK failure if a framework uses duplicated task id.)
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 
>   include/mesos/type_utils.hpp dafe1df0cb5d0b83ca0579068916fe7fda848f02 
>   src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
>   src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 
>   src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c 
> 
> Diff: https://reviews.apache.org/r/37531/diff/
> 
> 
> Testing
> ---
> 
> Draft code diff, will update UT cases later.
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Review Request 37547: Used execlp instead of execl to exec processes in Mesos.

2015-08-17 Thread Jie Yu

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

Review request for mesos, Ben Mahler and Vinod Kone.


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


Repository: mesos


Description
---

Used execlp instead of execl to exec processes in Mesos.


Diffs
-

  src/launcher/executor.cpp 9fa7dcfc39a6706f545b3328e468d9cd25d603ae 
  src/slave/containerizer/mesos/launch.cpp 
96ca47dff69d7706b570a96244549164918d2c2f 
  src/tests/containerizer/isolator_tests.cpp 
fdb706482d2edb89daf4f5a7cd14ee2381b57957 

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


Testing
---

sudo make check


Thanks,

Jie Yu



Review Request 37546: Used execlp instead of execl to exec processes in libprocess.

2015-08-17 Thread Jie Yu

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

Review request for mesos, Ben Mahler and Vinod Kone.


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


Repository: mesos


Description
---

Used execlp instead of execl to exec processes in libprocess.


Diffs
-

  3rdparty/libprocess/include/process/subprocess.hpp 
310cb4f8e4e2faa5545dffd196d7490c868bc5d6 

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


Testing
---

sudo make check


Thanks,

Jie Yu



Review Request 37545: Used execlp instead of execl to exec processes in stout.

2015-08-17 Thread Jie Yu

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

Review request for mesos, Ben Mahler and Vinod Kone.


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


Repository: mesos


Description
---

Used execlp instead of execl to exec processes in stout.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp 
5592e10746f0e23ef6f6d5d5e890bd91091051d6 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
c848268dc8fb595f1f4d9e81ef0179b722bdba4d 

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


Testing
---

sudo make check


Thanks,

Jie Yu



Review Request 37544: Fixed a bug in port mapping tests due to os::shell refactor.

2015-08-17 Thread Jie Yu

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

Review request for mesos, Ben Mahler and Vinod Kone.


Repository: mesos


Description
---

Fixed a bug in port mapping tests due to os::shell refactor.


Diffs
-

  src/tests/containerizer/port_mapping_tests.cpp 
d3526b886c3a6d354b698013c7c59cd1fbda3cbb 

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


Testing
---

sudo make check


Thanks,

Jie Yu



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

2015-08-17 Thread Marco Massenzio

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



src/examples/java/TestPersistentVolumeFramework.java (lines 535 - 537)


here (and everywhere else) can you please explain what the arguments are 
and, even, what an example use would be?

this would be great for folks planning to use this framework as a 
'template' for their own.



src/examples/java/TestPersistentVolumeFramework.java (line 560)


why `8`? what does this 'magic number' represent?
is this something that should be a CONSTANT or a user/configuration 
selectable?

please also add a comment as to what is going on here.



src/examples/java/TestPersistentVolumeFramework.java (line 569)


s/by/for



src/examples/java/TestPersistentVolumeFramework.java (line 574)


this is really *not* a getter - and I agree that `createCreateOperation` 
sounds awful, but I guess that we don't much have a choice...

Also, you may want to explain in the @param clause that the generic 
Resource here, really should be a Volume (create, supposedly, in the 
`getShardPersistentVolume` maybe?)

same below for `getLaunchOperation`

IMO you can conflate the two into one factory method:
```
public static Offer.Operation createOperation(
Operation.Type type,
Resource volume,
TaskInfo task) {
  Operation.Builder builder = 
  Operation.newBuilder()
   .setType(type);
  switch (type) {
case Operation.Type.LAUNCH:
builder.setLaunch();
break;
...
  }
  return builder.build();
}
```
or something along those lines.
(and does it really need to be a `static` method?)



src/examples/java/TestPersistentVolumeFramework.java (line 603)


please lose the `Test` moniker - everyone will assume this is a unit test 
and will be scratching their head.

Also, please move to its own class file.



src/examples/java/TestPersistentVolumeFramework.java (line 622)


As I mentioned earlier (and I must stress, I feel pretty strongly about 
this :) we should use a Logger here (and everywhere) instead of System.out AND 
use something like log4j or whatever you feel comfortable with - and have a 
log4j.properties that folks can fiddle with and configure logging as they see 
fit.

This is just Java good practice and will set a good example.



src/examples/java/TestPersistentVolumeFramework.java (lines 647 - 651)


this is going to be VERY verbose - replace with a log.debug(...)



src/examples/java/TestPersistentVolumeFramework.java (line 657)


use `offers.size()` to dimension your new List (unless you expect it to 
grow?)



src/examples/java/TestPersistentVolumeFramework.java (line 661)


please refactor all this code out into its own helper method.

same below for WAITING



src/examples/java/TestPersistentVolumeFramework.java (line 662)


you see? now anyone reading this code will be scratching their head trying 
to recall what contains which :)



src/examples/java/TestPersistentVolumeFramework.java (line 669)


as everywhere else: `if (!errMsg.isEmpty())`
but you can see that this is not a good pattern



src/examples/java/TestPersistentVolumeFramework.java (line 678)


can you add a comment as to why you are doing this here? are you creating a 
new file on the slave?
why?
(I mean, add this to the comment, don't explain it just to me here, but to 
all future readers of this code)



src/examples/java/TestPersistentVolumeFramework.java (lines 701 - 703)


I'm almost sure (but can't verify right now) that you can create this list 
with something like:

```
List tmpOperations = 
Lists.newArrayList(getCreateOperation(volume),
getLaunchOperation(task));
```

or something similar.
At the very least, take advantage of Java 7:
```
List tmpOperations = new ArrayList<>(2);
```



src/examples/java/TestPersistentVolumeFramework.java (line 750)


unnecessary break
also, I think you should just bail here?
(I

Re: Review Request 37512: Added user doc for Scheduler HTTP API.

2015-08-17 Thread Anand Mazumdar

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


This looks very thorough. Just some minor comments/nits around typos.


docs/scheduler_http_api.md (line 12)


s/via one/via



docs/scheduler_http_api.md (line 14)


Since we refer to this endpoint via name "/scheduler" in the rest of the 
document. Why not add this line at the end after the fully qualified URL :

We would be referring to this endpoint by its suffix "/scheduler" in the 
rest of the document.



docs/scheduler_http_api.md (line 16)


How about this :

This endpoint accepts HTTP POST requests with data encoded as JSON 
(Content-Type: application/json) or binary protobuf (Content-Type: 
application/x-protobuf).
The first request that a scheduler sends to “/scheduler” endpoint is called 
SUBSCRIBE and results in a streaming response (Transfer-Encoding: chunked). 
**Schedulers are expected to keep the subscription connection open as long as 
possible (barring errors in network, software, hardware etc.) and incrementally 
process the response** (NOTE: HTTP client libraries that can only parse the 
response after the connection is closed cannot be used). For the encoding used, 
please refer to “Events” section below.



docs/scheduler_http_api.md (line 19)


s/In line with the HTTP standard, a "202 Accepted"/An "Accepted"

s/response just/response



docs/scheduler_http_api.md (line 24)


s/exact protobuf/protobuf



docs/scheduler_http_api.md (line 28)


s/very first/first



docs/scheduler_http_api.md (line 30)


How about this ?

To subscribe with the master, the scheduler sends a HTTP POST request with 
encoded `SUBSCRIBE` message containing the required field FrameworkInfo. Note 
that if "subscribe.framework_info.id" is not set, Master considers the 
scheduler as a new one and subscribes it by assigning a FrameworkID. The HTTP 
response is a stream with RecordIO encoding, with the first event being 
`SUBSCRIBED` event (see details in **Events** section).



docs/scheduler_http_api.md (line 72)


s/with the same FrameworkID/containing the same



docs/scheduler_http_api.md (line 76)


s/a HTTP 4xx response/, a HTTP 4xx response



docs/scheduler_http_api.md (line 78)


s/result in/result in a

s/"403 Forbidden" response instead of "202 Accepted" response/"403 
Forbidden" instead of "202 Accepted" response

s/“400 Bad Request”/"400 Bad Request" response

s/bad/missing



docs/scheduler_http_api.md (line 81)


How about:

When Mesos receives this request, it will shut down all executors (and 
consequently kill tasks) and remove persistent volumes (if requested). The 
framework is then removed and all open connections from this scheduler to the 
Master are closed.



docs/scheduler_http_api.md (line 97)


The other example don't have this line. Can we add this to the other 
example call except Subscribe too ?



docs/scheduler_http_api.md (line 156)


How about :

Sent by the scheduler to remove any/all filters that it has previously set 
via `ACCEPT` or `DECLINE` calls.



docs/scheduler_http_api.md (line 224)


Kill "Note:uuid is needed to send an acknowledgement" as it is redundant.



docs/scheduler_http_api.md (line 249)


s/status of a list of/status of



docs/scheduler_http_api.md (line 333)


s/Very first event/The first event



docs/scheduler_http_api.md (line 373)


Sent by the master whenever there is a status update that is generated by 
the executor, slave or master.



docs/scheduler_http_api.md (line 414)


Note that there is no guaranteed order between the `FAILURE`, `UPDATE` and 
`RESCIND` events.



docs/scheduler_http_api.md (line 444)


s/working/alive

s/This will also help/This also helps



docs/scheduler_http_api.md (line 472)


Re: Review Request 37494: Fixed JSON wrapper to properly encode bytes.

2015-08-17 Thread Ben Mahler

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



3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp (lines 497 - 498)


Does this TODO need updating based on how picojson does it?


- Ben Mahler


On Aug. 14, 2015, 11:24 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37494/
> ---
> 
> (Updated Aug. 14, 2015, 11:24 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Ben Mahler.
> 
> 
> Bugs: MESOS-3267
> https://issues.apache.org/jira/browse/MESOS-3267
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See the bug for details.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp 
> 8f848ad9646d828b9efa2fb0f3225c82ac81b23e 
>   3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 
> dd692094e164e33ba0f13e2b994c14cbfd827cbf 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
> 018ff51d10f4ba076609704d6e3b2c704c82b016 
> 
> Diff: https://reviews.apache.org/r/37494/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 37208: Fix the spell error in help message of slave component.

2015-08-17 Thread Guangya Liu

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

Ship it!


Ship It!

- Guangya Liu


On 八月 17, 2015, 6:50 a.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37208/
> ---
> 
> (Updated 八月 17, 2015, 6:50 a.m.)
> 
> 
> Review request for mesos and Marco Massenzio.
> 
> 
> Bugs: MESOS-3228
> https://issues.apache.org/jira/browse/MESOS-3228
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix the spell error in help message of slave component.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 82b6cf47af26f0533ff603a67240777e9a9b986e 
> 
> Diff: https://reviews.apache.org/r/37208/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>



Re: Review Request 37460: Prevent perf test failures from killing the test harness.

2015-08-17 Thread Ben Mahler

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



src/tests/containerizer/perf_tests.cpp (lines 65 - 67)


Hm.. we still have assertions in this test, so could we just change this 
CHECK to an ASSERT and leave the test as is?

It doesn't seem worth introducing conditional logic here to try to proceed 
when we still haven't eliminated ASSERTs that can fail the test. ASSERTs are ok 
since our tests should be passing :)



src/tests/containerizer/perf_tests.cpp (lines 116 - 117)


This will crash when parse doesn't contain ""


- Ben Mahler


On Aug. 14, 2015, 3:01 a.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37460/
> ---
> 
> (Updated Aug. 14, 2015, 3:01 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-2834
> https://issues.apache.org/jira/browse/MESOS-2834
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prevent perf test failures from killing the test harness.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/perf_tests.cpp 
> 6b3d70f3e7ea8f59f94e6961491d4e9a730e3334 
> 
> Diff: https://reviews.apache.org/r/37460/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>



Re: Review Request 37512: Added user doc for Scheduler HTTP API.

2015-08-17 Thread Vinod Kone

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

(Updated Aug. 17, 2015, 6:26 p.m.)


Review request for mesos, Anand Mazumdar and Ben Mahler.


Changes
---

attached "bug" id.


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


Repository: mesos


Description
---

Mostly copy pasted from the design doc with some formatting.


Diffs
-

  docs/scheduler_http_api.md PRE-CREATION 

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


Testing
---

gist: https://gist.github.com/vinodkone/5ae803ee35274ca46a3e


Thanks,

Vinod Kone



Re: Review Request 37424: Timeout the perf future if the process does not complete.

2015-08-17 Thread Ben Mahler

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



src/linux/perf.cpp (line 199)


We may want to update this to become killtree in a separate patch? Or are 
we guaranteed that perf will clean up child processes? If so, that warrants a 
comment :)



src/linux/perf.cpp (lines 304 - 312)


How about we just use the discard semantics that are already set up inside 
initialize()? If a discard is requested, we will terminate.

This leaves the composition in the caller, which we usually prefer (rather 
than pushing in timeout logic to all libraries, it's easier to provide discard 
semantics and have the caller discard when it's no longer interested).

So the idea here is now your callers use a .after() which does a discard().


- Ben Mahler


On Aug. 13, 2015, 12:29 a.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37424/
> ---
> 
> (Updated Aug. 13, 2015, 12:29 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Timeout the perf future if the process does not complete.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp cb1a13d0b1754a50f0121bfda522056ff8c3e3c8 
> 
> Diff: https://reviews.apache.org/r/37424/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>



Re: Review Request 37208: Fix the spell error in help message of slave component.

2015-08-17 Thread Niklas Nielsen

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

Ship it!


Ship It!

- Niklas Nielsen


On Aug. 16, 2015, 11:50 p.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37208/
> ---
> 
> (Updated Aug. 16, 2015, 11:50 p.m.)
> 
> 
> Review request for mesos and Marco Massenzio.
> 
> 
> Bugs: MESOS-3228
> https://issues.apache.org/jira/browse/MESOS-3228
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix the spell error in help message of slave component.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 82b6cf47af26f0533ff603a67240777e9a9b986e 
> 
> Diff: https://reviews.apache.org/r/37208/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>



Re: Review Request 37208: Fix the spell error in help message of slave component.

2015-08-17 Thread Marco Massenzio

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


Sorry it's taking so long - but I can't commit.
I've asked one of my colleagues at Mesosphere to commit this.

In general, no matter how small the patch, it's always advisable to have a 
"shepherd" (ie, someone with 'committer' status) to help commit the patch.

- Marco Massenzio


On Aug. 17, 2015, 6:50 a.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37208/
> ---
> 
> (Updated Aug. 17, 2015, 6:50 a.m.)
> 
> 
> Review request for mesos and Marco Massenzio.
> 
> 
> Bugs: MESOS-3228
> https://issues.apache.org/jira/browse/MESOS-3228
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix the spell error in help message of slave component.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 82b6cf47af26f0533ff603a67240777e9a9b986e 
> 
> Diff: https://reviews.apache.org/r/37208/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>



Re: Review Request 37423: Split out common functions for running "perf" into a common perf class with wrapper functions to allow for reuse between sample, valid and version operations.

2015-08-17 Thread Ben Mahler

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

Ship it!


Thanks Paul, just some cleanups and we can get this committed.


src/linux/perf.cpp (lines 158 - 161)


s/Execute/Executes/ s/parameters/arguments/

Feel free to add a TODO for me:

```
TODO(bmahler): Add a process::os::shell to generalize this.
```



src/linux/perf.cpp (line 159)


s/returning/returns/



src/linux/perf.cpp (lines 185 - 190)


Can you remove duration entirely from `Perf` now that it's not used?



src/linux/perf.cpp (lines 318 - 319)


Can you update this message since we're not necessarily collecting perf 
statistics now?



src/linux/perf.cpp (lines 353 - 354)


Can you remove this comment? 'PerfSampler' seems to refer to an older 
version of this patch.



src/linux/perf.cpp (lines 359 - 363)


Could you add some newlines for readability?

```
  Time start = Clock::now();

  Perf* sampler = new Perf(argv, duration);
  Future future = sampler->future();
  spawn(sampler, true);

  return future.then([=] (const Future& output) ->
```



src/linux/perf.cpp (line 363)


Can you capture 'start' and 'duration' explicitly rather than all values?



src/linux/perf.cpp (line 365)


If you look at the style guide, this should be up on the previous line FWICT



src/linux/perf.cpp (lines 366 - 381)


This is indented too much, can you indent based on the style guide for 
lambdas?



src/linux/perf.cpp (line 367)


= wrapping should be indented by 2 (this uses 4)



src/linux/perf.cpp (lines 369 - 371)


How about tacking on the failure reason:

```
"Failed to parse perf sample: " + ...
```



src/linux/perf.cpp (lines 373 - 380)


No need for the non-const copy, you can just use .get() to alter the value 
directly. In the future, you could use `->` which will be a lot cleaner 
syntactically: https://issues.apache.org/jira/browse/MESOS-2757



src/linux/perf.cpp (lines 406 - 407)


This should fit on one line?



src/linux/perf.cpp (lines 433 - 434)


Having to pass the duration into argv and sample seems a bit messy, but I 
don't have a suggestion for an easy way to clean it up.

Should fit on one line?

```
>>> len('  return internal::sample(internal::argv(events, cgroups, 
duration), duration);')
79
```


- Ben Mahler


On Aug. 13, 2015, 12:19 a.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37423/
> ---
> 
> (Updated Aug. 13, 2015, 12:19 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3185
> https://issues.apache.org/jira/browse/MESOS-3185
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Split out common functions for running "perf" into a common perf class with 
> wrapper functions to allow for reuse between sample, valid and version 
> operations.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp cb1a13d0b1754a50f0121bfda522056ff8c3e3c8 
> 
> Diff: https://reviews.apache.org/r/37423/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>



Re: Review Request 37502: Add CMAKE_CXX_FLAGS to GMOCK_CONFIG_CMD in CMake.

2015-08-17 Thread haosdent huang


> On Aug. 17, 2015, 5:45 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/CMakeLists.txt, line 174
> > 
> >
> > I believe you need some more quotation marks.  (So that the C++flags 
> > get properly assigned)
> > 
> > i.e. 
> > ```
> > set(GMOCK_CONFIG_CMD "${GMOCK_ROOT}/configure
> >  ^
> >   --prefix=${GMOCK_ROOT}-lib/lib
> >   CXXFLAGS=\"${GMOCK_CONFIG_CXXFLAGS}\" "
> > ^ ^ ^
> > ```
> 
> haosdent huang wrote:
> Actually not need, I found " have some strange behaviour here.

Here also have a similiar example show not need add quotation marks. 
http://stackoverflow.com/questions/5971921/building-a-library-using-autotools-from-cmake


- haosdent


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


On Aug. 15, 2015, 12:07 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37502/
> ---
> 
> (Updated Aug. 15, 2015, 12:07 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-3270
> https://issues.apache.org/jira/browse/MESOS-3270
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add CMAKE_CXX_FLAGS to GMOCK_CONFIG_CMD in CMake.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 997cc0d0e316e316136d4746e50e9e292a82b36b 
> 
> Diff: https://reviews.apache.org/r/37502/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 37502: Add CMAKE_CXX_FLAGS to GMOCK_CONFIG_CMD in CMake.

2015-08-17 Thread haosdent huang


> On Aug. 17, 2015, 5:45 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/CMakeLists.txt, line 174
> > 
> >
> > I believe you need some more quotation marks.  (So that the C++flags 
> > get properly assigned)
> > 
> > i.e. 
> > ```
> > set(GMOCK_CONFIG_CMD "${GMOCK_ROOT}/configure
> >  ^
> >   --prefix=${GMOCK_ROOT}-lib/lib
> >   CXXFLAGS=\"${GMOCK_CONFIG_CXXFLAGS}\" "
> > ^ ^ ^
> > ```

Actually not need, I found " have some strange behaviour here.


- haosdent


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


On Aug. 15, 2015, 12:07 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37502/
> ---
> 
> (Updated Aug. 15, 2015, 12:07 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-3270
> https://issues.apache.org/jira/browse/MESOS-3270
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add CMAKE_CXX_FLAGS to GMOCK_CONFIG_CMD in CMake.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 997cc0d0e316e316136d4746e50e9e292a82b36b 
> 
> Diff: https://reviews.apache.org/r/37502/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 37502: Add CMAKE_CXX_FLAGS to GMOCK_CONFIG_CMD in CMake.

2015-08-17 Thread Joseph Wu

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



3rdparty/libprocess/3rdparty/CMakeLists.txt (line 170)


I believe you need some more quotation marks.  (So that the C++flags get 
properly assigned)

i.e. 
```
set(GMOCK_CONFIG_CMD "${GMOCK_ROOT}/configure
 ^
  --prefix=${GMOCK_ROOT}-lib/lib
  CXXFLAGS=\"${GMOCK_CONFIG_CXXFLAGS}\" "
^ ^ ^
```


- Joseph Wu


On Aug. 15, 2015, 5:07 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37502/
> ---
> 
> (Updated Aug. 15, 2015, 5:07 a.m.)
> 
> 
> Review request for mesos, Alex Clemmer, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-3270
> https://issues.apache.org/jira/browse/MESOS-3270
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add CMAKE_CXX_FLAGS to GMOCK_CONFIG_CMD in CMake.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 997cc0d0e316e316136d4746e50e9e292a82b36b 
> 
> Diff: https://reviews.apache.org/r/37502/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 37518: Add the revocable metrics information in monitoring doc

2015-08-17 Thread Vinod Kone

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

Ship it!


Ship It!

- Vinod Kone


On Aug. 17, 2015, 7:41 a.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37518/
> ---
> 
> (Updated Aug. 17, 2015, 7:41 a.m.)
> 
> 
> Review request for mesos and Zhiwei Chen.
> 
> 
> Bugs: MESOS-3278
> https://issues.apache.org/jira/browse/MESOS-3278
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add the revocable metrics information in monitoring doc
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md d80f9362b33fec307eaaf93398631e6b2d02678f 
> 
> Diff: https://reviews.apache.org/r/37518/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>



Re: Review Request 37493: Detect gflags when present and link when building Python module

2015-08-17 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [37493]

All tests passed.

- Mesos ReviewBot


On Aug. 17, 2015, 6:49 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37493/
> ---
> 
> (Updated Aug. 17, 2015, 6:49 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Marco Massenzio, Till 
> Toenshoff, and Timothy Chen.
> 
> 
> Bugs: MESOS-1010
> https://issues.apache.org/jira/browse/MESOS-1010
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Detect gflags when present and link when building Python module
> 
> 
> Diffs
> -
> 
>   configure.ac a478ebd331ff2eed25ced2d86d9e6bda3376e2ab 
> 
> Diff: https://reviews.apache.org/r/37493/diff/
> 
> 
> Testing
> ---
> 
> Did the following:
> 
> ../configure && make && make check
> 
> For each of these cases:
> 1. using bundled glog, with gflags not installed
> 2. using bundled glog, with gflags installed
> 3. using system glog (built without gflags) via --with-glog=..., with gflags 
> not installed
> 4. using system glog (built without gflags) via --with-glog=..., with gflags 
> installed
> 5. using system glog (built with gflags) via --with-glog=..., with gflags 
> installed
> 
> Test suite passes in all cases.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 37511: Fixed master to reject non-subscribe calls made before subscription.

2015-08-17 Thread Anand Mazumdar

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

Ship it!


Ship It!

- Anand Mazumdar


On Aug. 16, 2015, 11:41 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37511/
> ---
> 
> (Updated Aug. 16, 2015, 11:41 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As the design doc says the master should reject non-subscribe calls if 
> framework hasn't registered yet.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp a73ee17bcef72791b06240a4673f466de582c41b 
> 
> Diff: https://reviews.apache.org/r/37511/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> No new tests yet because it's not clear how to simulate scheduler 
> disconnection on the master.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Review Request 37532: Add QUIESCE call interface to the scheduler

2015-08-17 Thread Guangya Liu

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This is just part of MESOS-3037, this patch only add the interface
of QUIESCE call.


Diffs
-

  include/mesos/scheduler.hpp ee198b6955882f4f31466ca05429ca16fbf2f5cd 
  include/mesos/scheduler/scheduler.proto 
89daf8a6b74057ee156b3ad691397e76fcb835b8 
  include/mesos/v1/scheduler/scheduler.proto 
bd5e82a614b1163b29f9b20e562208efa1ba4b55 
  src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be 
  src/master/master.cpp c5e6c6f3304060d4c92d52851951f10bc432500e 
  src/sched/sched.cpp 012af0508eeceeccd168b29f36fa258d20b28c21 
  src/tests/scheduler_tests.cpp 77c26353afc33f5099be2d1e597ffc630e559968 

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


Testing
---


Thanks,

Guangya Liu



Review Request 37531: MESOS-3070

2015-08-17 Thread Klaus Ma

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

Review request for mesos.


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


Repository: mesos


Description
---

MESOS-3070 (Master CHECK failure if a framework uses duplicated task id.)


Diffs
-

  include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 
  include/mesos/type_utils.hpp dafe1df0cb5d0b83ca0579068916fe7fda848f02 
  src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be 
  src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
  src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 
  src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c 

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


Testing
---

Draft code diff, will update UT cases later.


Thanks,

Klaus Ma



Re: Review Request 37513: added Scrapinghub to "powered by mesos"

2015-08-17 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [37513]

All tests passed.

- Mesos ReviewBot


On Aug. 17, 2015, 3 a.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37513/
> ---
> 
> (Updated Aug. 17, 2015, 3 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3276
> https://issues.apache.org/jira/browse/MESOS-3276
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> added Scrapinghub to "powered by mesos"
> 
> 
> Diffs
> -
> 
>   docs/powered-by-mesos.md 13a8fb1192001f9ebedfb38bb291af02b58e879c 
> 
> Diff: https://reviews.apache.org/r/37513/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>



Re: Review Request 37511: Fixed master to reject non-subscribe calls made before subscription.

2015-08-17 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [37511]

All tests passed.

- Mesos ReviewBot


On Aug. 16, 2015, 11:41 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37511/
> ---
> 
> (Updated Aug. 16, 2015, 11:41 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As the design doc says the master should reject non-subscribe calls if 
> framework hasn't registered yet.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp a73ee17bcef72791b06240a4673f466de582c41b 
> 
> Diff: https://reviews.apache.org/r/37511/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> No new tests yet because it's not clear how to simulate scheduler 
> disconnection on the master.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 37512: Added user doc for Scheduler HTTP API.

2015-08-17 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [37512]

All tests passed.

- Mesos ReviewBot


On Aug. 17, 2015, 12:07 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37512/
> ---
> 
> (Updated Aug. 17, 2015, 12:07 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mostly copy pasted from the design doc with some formatting.
> 
> 
> Diffs
> -
> 
>   docs/scheduler_http_api.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37512/diff/
> 
> 
> Testing
> ---
> 
> gist: https://gist.github.com/vinodkone/5ae803ee35274ca46a3e
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 37518: Add the revocable metrics information in monitoring doc

2015-08-17 Thread Yong Qiao Wang

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

(Updated Aug. 17, 2015, 7:41 a.m.)


Review request for mesos and Zhiwei Chen.


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


Repository: mesos


Description
---

Add the revocable metrics information in monitoring doc


Diffs
-

  docs/monitoring.md d80f9362b33fec307eaaf93398631e6b2d02678f 

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


Testing
---


Thanks,

Yong Qiao Wang



Review Request 37518: Add the revocable metrics information in monitoring doc

2015-08-17 Thread Yong Qiao Wang

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

Review request for mesos.


Repository: mesos


Description
---

Add the revocable metrics information in monitoring doc


Diffs
-

  docs/monitoring.md d80f9362b33fec307eaaf93398631e6b2d02678f 

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


Testing
---


Thanks,

Yong Qiao Wang