Re: Review Request 38932: Kill health check external command process and continue check after timeout.

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On Oct. 5, 2015, 10:18 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38932/
> ---
> 
> (Updated Oct. 5, 2015, 10:18 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-3479
> https://issues.apache.org/jira/browse/MESOS-3479
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Kill health check external command process and continue check after timeout.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 1e4901335854c49e46cd7b132e79ccb11cd72ade 
>   src/health-check/main.cpp 97b25716335ec5719c1100bd73d06b7fc98036c9 
>   src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
>   src/tests/health_check_tests.cpp ff6275b19206b49eacb6761f3aeb58dd87651ade 
> 
> Diff: https://reviews.apache.org/r/38932/diff/
> 
> 
> Testing
> ---
> 
> GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="HealthCheckTest.CheckCommandTimeout" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 40379: [WIP] MESOS-3930: Set resource type as USAGE_SLACK for Oversubscription

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40375, 40379]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Nov. 21, 2015, 11:26 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40379/
> ---
> 
> (Updated Nov. 21, 2015, 11:26 a.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-3930
> https://issues.apache.org/jira/browse/MESOS-3930
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In Optimistic Offer Phase 1, it introduce `RevocableInfo::type`: USAGE_SLACK 
> for Oversubscription and ALLOCATION_SLACK for Optimistic Offer. Slave helps 
> to update `RevocableInfo::type` for Oversubscription.
> 
> 
> Diffs
> -
> 
>   src/slave/resource_estimators/fixed.cpp 305664c 
>   src/slave/slave.cpp d1126f0 
>   src/tests/oversubscription_tests.cpp 0d0bf7e 
> 
> Diff: https://reviews.apache.org/r/40379/diff/
> 
> 
> Testing
> ---
> 
> make (make check is on-going)
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 40379: [WIP] MESOS-3930: Set resource type as USAGE_SLACK for Oversubscription

[Klaus Ma]

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



src/slave/resource_estimators/fixed.cpp (line 77)


This will introduce compatibility issue, I'd like to resolve this task 
(MESOS-3930) in document.


- Klaus Ma


On Nov. 21, 2015, 7:26 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40379/
> ---
> 
> (Updated Nov. 21, 2015, 7:26 p.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-3930
> https://issues.apache.org/jira/browse/MESOS-3930
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In Optimistic Offer Phase 1, it introduce `RevocableInfo::type`: USAGE_SLACK 
> for Oversubscription and ALLOCATION_SLACK for Optimistic Offer. Slave helps 
> to update `RevocableInfo::type` for Oversubscription.
> 
> 
> Diffs
> -
> 
>   src/slave/resource_estimators/fixed.cpp 305664c 
>   src/slave/slave.cpp d1126f0 
>   src/tests/oversubscription_tests.cpp 0d0bf7e 
> 
> Diff: https://reviews.apache.org/r/40379/diff/
> 
> 
> Testing
> ---
> 
> make (make check is on-going)
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 40379: [WIP] MESOS-3930: Set resource type as USAGE_SLACK for Oversubscription

[Klaus Ma]

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

(Updated Nov. 21, 2015, 7:26 p.m.)


Review request for mesos and Guangya Liu.


Changes
---

Update the diff passed `make check`


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


Repository: mesos


Description
---

In Optimistic Offer Phase 1, it introduce `RevocableInfo::type`: USAGE_SLACK 
for Oversubscription and ALLOCATION_SLACK for Optimistic Offer. Slave helps to 
update `RevocableInfo::type` for Oversubscription.


Diffs (updated)
-

  src/slave/resource_estimators/fixed.cpp 305664c 
  src/slave/slave.cpp d1126f0 
  src/tests/oversubscription_tests.cpp 0d0bf7e 

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


Testing
---

make (make check is on-going)


Thanks,

Klaus Ma

Re: Review Request 40497: Add hex number support to numify()

[Benjamin Bannier]

> On Nov. 20, 2015, 3:23 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp, line 39
> > 
> >
> > Where you planning to output `c` here? Otherwise I suggest replacing 
> > this at least `ss.get()` (and drop the decl above).
> > 
> > I like H.Sutter's synopsis of `boost::lexical_cast` even better since 
> > it shows clearly the failure sites (modulo the throw for here), see 
> > http://www.gotw.ca/publications/mill19.htm:
> > 
> > template
> > Target lexical_cast(Source arg)
> > {
> >   std::stringstream interpreter;
> >   Target result;
> > 
> >   if(!(interpreter << arg) ||
> >  !(interpreter >> result) ||
> >  !(interpreter >> std::ws).eof())
> > throw bad_lexical_cast();
> > 
> >   return result;
> > }
> > 
> > You could do a similar impl just by injection a `<< std::hex`.
> 
> Cong Wang wrote:
> I replace that ss.get() with ss.eof(). And also I don't understand why 
> checking the return value of operator<< could work, it always return the 
> stream right? The document says we should check ss.fail().

http://en.cppreference.com/w/cpp/io/basic_ios/operator_bool


> On Nov. 20, 2015, 3:23 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp, line 26
> > 
> >
> > Would be nice to add a couple more positives here, e.g., to catch 
> > `hex_cast` impls like `boost::lexical_cast(s.substr(2)` ;)
> 
> Cong Wang wrote:
> I added some more positives, but not sure if they are 
> boost::lexical_cast(s.substr(2))...

Hmm, for `0x00` the hex prefix doesn't matter, and just extracting without it 
would give the same answer `0`. How about testing e.g., `0x10` which really 
translates to `16`, but to `10` without the prefix?


- Benjamin


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


On Nov. 20, 2015, 10:26 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40497/
> ---
> 
> (Updated Nov. 20, 2015, 10:26 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Ian Downes.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> numify() in libprocess uses boost lexical_cast() to cast numbers, but it can 
> not cast a hex number. This patch adds hex support to numify().
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 5a0ffc38d4194b9f9c53dc1cf0c90ca7bbae2afd 
>   3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp 
> ddd3dd9e015c583e04d72ac9a9b5a5ed6f1d49d5 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 14fb644b38a5cbb8cde74aab39e84305f6ab7041 
>   3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40497/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 40563: Added functionality for handling status updates from HTTP based executors

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40560, 40561, 40562, 40570, 40563]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Nov. 21, 2015, 1:45 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40563/
> ---
> 
> (Updated Nov. 21, 2015, 1:45 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, Marco Massenzio, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-3476
> https://issues.apache.org/jira/browse/MESOS-3476
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the ability for the agent to handle status updates from HTTP 
> based executors. Previously, the existing `statusUpdate` method used to 
> handle status updates sent from slave with `pid == UPID()`, sent from other 
> executors/on behalf of other executors with `pid == Some()`. This change 
> modifies the argument to be `Option`. This ensures that `pid == None()` 
> when set, the ACK is correctly routed to the corresponding HTTP based 
> executor.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp ce48a0584ab18a8d95dd02619f62df18b2276639 
>   src/slave/slave.hpp ec2dfa99e6b553e2bcd82d12db915ae8625075a1 
>   src/slave/slave.cpp d1126f00d947fdb4823b0c495335b743254ac7ee 
> 
> Diff: https://reviews.apache.org/r/40563/diff/
> 
> 
> Testing
> ---
> 
> make check + would add tests once the https://reviews.apache.org/r/39297/ 
> chain is reviewed.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 40242: Improved docs for dynamic reservation HTTP endpoints.

[Guangya Liu]

> On 十一月 19, 2015, 2:44 a.m., Guangya Liu wrote:
> > docs/reservation.md, line 242
> > 
> >
> > This will not work if end user did not enable autheration, a JIRA 
> > ticket is tracing this MESOS-3940, shall we highlight the issue here before 
> > get resolved?
> 
> Neil Conway wrote:
> I think this is okay -- the issue should be fixed shortly.

I think that we should at least clarify that the dynamic reservation works for 
both enabling and not enabling authorization in the document, it is not a MUST 
we always put : in the CURL request, thoughts?


- Guangya


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


On 十一月 20, 2015, 10:32 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40242/
> ---
> 
> (Updated 十一月 20, 2015, 10:32 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Michael Park.
> 
> 
> Bugs: MESOS-2455
> https://issues.apache.org/jira/browse/MESOS-2455
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved docs for dynamic reservation HTTP endpoints.
> 
> 
> Diffs
> -
> 
>   docs/home.md 7aa785e9ae07f2cc14eb0f1108ae4ab4c8748599 
>   docs/persistent-volume.md 0951ccb69daaa19b959e11cf3bf972a674a58305 
>   docs/reservation.md 81f21c3755b216b0932876c1ddd9de4d3fbe814a 
> 
> Diff: https://reviews.apache.org/r/40242/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 40243: Documented "role" field in Resource protobuf message.

[Guangya Liu]

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

Ship it!


Ship It!

- Guangya Liu


On 十一月 20, 2015, 10:31 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40243/
> ---
> 
> (Updated 十一月 20, 2015, 10:31 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Michael Park.
> 
> 
> Bugs: MESOS-2455
> https://issues.apache.org/jira/browse/MESOS-2455
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented "role" field in Resource protobuf message.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5ad48bd376c34dd495399b62fa0bd37ddcc5518b 
>   include/mesos/v1/mesos.proto e71ddda7f23f2272ce8eb00f358c66fce205c13b 
> 
> Diff: https://reviews.apache.org/r/40243/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 39417: Add --egress_flow_classifier_parent flag

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [40497, 40506, 39415, 39416, 39417]

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

Error:
 2015-11-21 09:49:48 URL:https://reviews.apache.org/r/39417/diff/raw/ 
[9414/9414] -> "39417.patch" [1]
error: patch failed: 
src/slave/containerizer/mesos/isolators/network/port_mapping.cpp:2388
error: src/slave/containerizer/mesos/isolators/network/port_mapping.cpp: patch 
does not apply
Failed to apply patch

- Mesos ReviewBot


On Nov. 20, 2015, 10:27 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39417/
> ---
> 
> (Updated Nov. 20, 2015, 10:27 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When --egress_unique_flow_per_container is enabled, we need to install a flow 
> classifier (fq_codel) qdisc on egress side. This flag specifies where to 
> install it in the hierarchy. By default, we install it at root. But, for 
> example, if you have already installed HTB qdisc at root, you may want this 
> to be installed in other place than root, specify an HTB class ID as its 
> parent here.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 126f4aa8e3de2a2346336991c9b9a2ea61a8cd0a 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> e50616fd609588c547c90bba6d7b3b9b3eb4c6a9 
>   src/slave/flags.hpp 6ae7c94d2e05d81c8b970e7dcaa82d8aa4de7936 
>   src/slave/flags.cpp 26f554e1a73f1f7f3d7baec7bfc1a7f456c5677c 
> 
> Diff: https://reviews.apache.org/r/39417/diff/
> 
> 
> Testing
> ---
> 
> Manual tests, with and without a pre-installed HTB qdisc and classes.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 38117: Export per container SNMP statistics

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38117]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Nov. 20, 2015, 11:25 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38117/
> ---
> 
> (Updated Nov. 20, 2015, 11:25 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Paul Brett, and Vinod Kone.
> 
> 
> Bugs: MESOS-3365
> https://issues.apache.org/jira/browse/MESOS-3365
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These stats are those we get by `netstat -s`, they are important for diagnose 
> networking issues.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5ad48bd376c34dd495399b62fa0bd37ddcc5518b 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> ae53c1b1716a7ad9c6e64f9079c972bae31e4ed2 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> e50616fd609588c547c90bba6d7b3b9b3eb4c6a9 
>   src/slave/flags.hpp 6ae7c94d2e05d81c8b970e7dcaa82d8aa4de7936 
>   src/slave/flags.cpp 26f554e1a73f1f7f3d7baec7bfc1a7f456c5677c 
> 
> Diff: https://reviews.apache.org/r/38117/diff/
> 
> 
> Testing
> ---
> 
> ./src/mesos-network-helper statistics --eth0_name=X --enable_snmp_statistics 
> --pid=
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 40378: Added link to upgrade guide to documentation page.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40378]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Nov. 20, 2015, 10:54 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40378/
> ---
> 
> (Updated Nov. 20, 2015, 10:54 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added link to upgrade guide to documentation page.
> 
> 
> Diffs
> -
> 
>   docs/home.md 7aa785e9ae07f2cc14eb0f1108ae4ab4c8748599 
>   docs/upgrades.md f2da680d6877dfc161bb33cc97358cbf46787faa 
> 
> Diff: https://reviews.apache.org/r/40378/diff/
> 
> 
> Testing
> ---
> 
> Previewed with mesos-website-container.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>