Re: Review Request 44514: Implemented prepare() method of "network/cni" isolator.

[Qian Zhang]

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

(Updated March 23, 2016, 1:10 p.m.)


Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.


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


Repository: mesos


Description
---

Implemented prepare() method of "network/cni" isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
3878a7e85fe24175b3bd5e3a6268cf32a07f2d8b 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
9552312f9ba1e4df6564cfb737cc41e041cf4407 

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


Testing
---

make check


Thanks,

Qian Zhang

Re: Review Request 33174: Fix for docker not configuring CFS quotas correctly

[Steve Niemitz]

> On March 16, 2016, 11:08 p.m., Jie Yu wrote:
> > This patch breaks all the ROOT DOCKER tests in our internal CI. I've 
> > reverted it for now. Can you do a sudo make check with docker?
> 
> Steve Niemitz wrote:
> ok, I see what the problem is here.  The issue is with the 
> mesos-docker-executor code path (when launching a task w/ a TaskInfo, 
> launchExecutorProcess).  Because the code path only waits for the 
> mesos-docker-executor process to launch, the inspect in update() fails since 
> the docker container hasn't launched yet.
> 
> I'm not sure what the best solution is here, I'll play around with some 
> options.

Also, interestingly enough, if you remove the check in update() the checks if 
the resources have changed, even without my patch it breaks because the 
container isn't there yet, so this is even now just kind of accidentally 
working.


- Steve


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


On March 16, 2016, 8:10 p.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33174/
> ---
> 
> (Updated March 16, 2016, 8:10 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2617
> https://issues.apache.org/jira/browse/MESOS-2617
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix for docker containerizer not configuring CFS quotas correctly.
> 
> It would be nice to refactor all this isolation code in a way that can be 
> shared between all containerizers, as this is basically just copied from the 
> CgroupsCpushareIsolator, but that's a much bigger undertaking.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 79cd955e9c241becff52cc4bbef81dcc16802ee7 
>   src/slave/containerizer/docker.cpp fb9188a19a5cd8211d4f36f9647ebb70de560109 
> 
> Diff: https://reviews.apache.org/r/33174/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>

Re: Review Request 45022: Windows: Add Windows-friendly implementation of `rm.hpp`.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [45022, 45015, 45009, 44084, 44978, 44082, 44081, 44808]

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

Error:
2016-03-23 02:51:35 URL:https://reviews.apache.org/r/44081/diff/raw/ 
[4367/4367] -> "44081.patch" [1]
error: patch failed: 
3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp:464
error: 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp: patch 
does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/12101/console

- Mesos ReviewBot


On March 18, 2016, 7:39 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45022/
> ---
> 
> (Updated March 18, 2016, 7:39 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-4415
> https://issues.apache.org/jira/browse/MESOS-4415
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We currently depend on arcane "POSIX-like" Windows APIs like `::remove`.
> This API is incompatible with NT paths -- which we will eventually have
> to transition to for the Windows integration -- and has no documented
> behavior interacting with subsystems like the driver subsystem, which is
> required for symlinks.
> 
> This commit will move us to proper core Windows API equivalents for
> `::remove`.
> 
> Review: https://reviews.apache.org/r/45022
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 7bd4bfbc2ec5922879dcefddc12137336b11be52 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rm.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/rm.hpp 
> 52568b303c03fd57b81f6cc67782444ce734dd41 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rm.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
> 4c30189bb8261ccfc699da0f31b8b1fd3e9b3c83 
> 
> Diff: https://reviews.apache.org/r/45022/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 44136: Libprocess: [1/2] Conditioned out Windows-incompatible includes.

[Daniel Pravat]

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

(Updated March 23, 2016, 2:46 a.m.)


Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


Summary (updated)
-

Libprocess: [1/2] Conditioned out Windows-incompatible includes.


Repository: mesos


Description (updated)
---

Libprocess: [1/2] Conditioned out Windows-incompatible includes.


Diffs (updated)
-

  3rdparty/libprocess/src/http.cpp 3ca0cfd37ac58f6d2bf5341dc88e8abed05fe994 
  3rdparty/libprocess/src/pid.cpp 9387f59a3834af368bf37f8cc2e85102f0bb34f6 

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


Testing
---

OSX: make
Windows: build/run


Thanks,

Daniel Pravat

Re: Review Request 45168: Removed old comment from 'mesos.proto'.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45168]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 22, 2016, 5:48 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45168/
> ---
> 
> (Updated March 22, 2016, 5:48 p.m.)
> 
> 
> Review request for mesos, Adam B, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed old comment from 'mesos.proto'.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b965f5a6b1aff28feb2ab088c30ea927b9247403 
> 
> Diff: https://reviews.apache.org/r/45168/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44450: Rescind all outstanding offers to satisfy weights update.

[Yongqiao Wang]

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

(Updated March 23, 2016, 1:59 a.m.)


Review request for mesos, Adam B and Alexander Rukletsov.


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


Repository: mesos


Description
---

Rescind all outstanding offers to satisfy weights update.


Diffs
-

  src/master/master.hpp 124d43931a5c8a00ee0aaa604feb1761795209f2 
  src/master/weights_handler.cpp c9a1b0d9adbeb1e165999cdbb4f295b24f10b18f 

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


Testing
---

Make && Make check.

Manual test steps:

- Start Mesos master:
$ ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/tmp/mesos-master

- Start Mesos slave:
$ ./bin/mesos-slave.sh --master=127.0.0.1:5050

- Register a framwork with `curl`
$ curl -v http://127.0.0.1:5050/api/v1/scheduler -H "Content-type: 
application/json" -X POST -d @subscribe.json
$  cat subscribe.json
{
   "type": "SUBSCRIBE",

   "subscribe" : {
  "framework_info": {
  "user" :  "root",
  "name" :  "comsumer c1 HTTP Framework",
  "role" :  "mesos",
  "principal":"wyq"
  },

  "force" : true
  }
}

And this framework will receive an offer:
{"subscribed":{"framework_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-"}},"type":"SUBSCRIBED"}20
{"type":"HEARTBEAT"}680
{"offers":{"offers":[{"agent_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-S0"},"framework_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-"},"hostname":"192.168.1.5","id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-O0"},"resources":[{"name":"cpus","role":"*","scalar":{"value":8.0},"type":"SCALAR"},{"name":"mem","role":"*","scalar":{"value":15360.0},"type":"SCALAR"},{"name":"disk","role":"*","scalar":{"value":470832.0},"type":"SCALAR"},{"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"role":"*","type":"RANGES"}],"url":{"address":{"hostname":"192.168.1.5","ip":"192.168.1.5","port":5051},"path":"\/slave(1)","scheme":"http"}}]},"type":"OFFERS"}20
{"type":"HEARTBEAT"}20

- Update the weight of role `mesos`
$ curl --data "[{\"weight\":1.8,\"role\":\"mesos\"}]" -X PUT 
http://127.0.0.1:5050/weights

Receive an rescind offer, and a new offer received:
{"rescind":{"offer_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-O0"}},"type":"RESCIND"}680
{"offers":{"offers":[{"agent_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-S0"},"framework_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-"},"hostname":"192.168.1.5","id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-O1"},"resources":[{"name":"cpus","role":"*","scalar":{"value":8.0},"type":"SCALAR"},{"name":"mem","role":"*","scalar":{"value":15360.0},"type":"SCALAR"},{"name":"disk","role":"*","scalar":{"value":470832.0},"type":"SCALAR"},{"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"role":"*","type":"RANGES"}],"url":{"address":{"hostname":"192.168.1.5","ip":"192.168.1.5","port":5051},"path":"\/slave(1)","scheme":"http"}}]},"type":"OFFERS"}20
{"type":"HEARTBEAT"}20

- Update the weight of role `mesos1`
$ curl --data "[{\"weight\":2.8,\"role\":\"mesos1\"}]" -X PUT 
http://127.0.0.1:5050/weights

- The outstanding offer will not be rescinded.


(TODO) I will add couple of tests for this patch in another JIRA.


Thanks,

Yongqiao Wang

Re: Review Request 43879: Added allocator metrics for number of allocations made.

[Ben Mahler]

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


Fix it, then Ship it!




Thanks! Just some minor adjustments I'll make before committing.


docs/monitoring.md (line 873)


How about:

```
Number of times the allocation algorithm has run

```



src/master/allocator/mesos/metrics.hpp (line 59)


How about:

// Number of times the allocation algorithm has run.



src/tests/hierarchical_allocator_tests.cpp (lines 2684 - 2686)


Could we put this next to the resource metrics test?



src/tests/hierarchical_allocator_tests.cpp (lines 2684 - 2685)


How about:

```
// This test checks that the number of times the allocation
// algorithm has run is correctly reflected in the metric.
```



src/tests/hierarchical_allocator_tests.cpp (line 2686)


s/Metrics/Metric/ in this patch since there's only one



src/tests/hierarchical_allocator_tests.cpp (line 2695)


We'll typically use size_t for this kind of variable



src/tests/hierarchical_allocator_tests.cpp (line 2699)


It's a bit unfortunate to use the [] operator here since it will insert the 
value if it was missing, could we switch to .contains? This would be consistent 
with the comments I made on the previous patches.

Also, why not use your allocations variable? s/0/allocations/


- Ben Mahler


On March 21, 2016, 11:08 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> ---
> 
> (Updated March 21, 2016, 11:08 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator metrics for number of allocations made.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110 
>   src/master/allocator/mesos/hierarchical.cpp 
> 70291075c00a9a557529c2562dedcfc6c6c3ec32 
>   src/master/allocator/mesos/metrics.hpp 
> d04e9a11c77b6fb2d522608e3085f81f8a6657ca 
>   src/master/allocator/mesos/metrics.cpp 
> 46dd7f7b3d8d49c59b42e789fe5dcd49f1693ace 
>   src/tests/hierarchical_allocator_tests.cpp 
> 459e02576f6d05abbbcc83ae5cabac5c66e93f05 
> 
> Diff: https://reviews.apache.org/r/43879/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> I confirmed that this does not lead to general performance regressions in the 
> allocator; this is partially expected since the added code only inserts 
> metrics in the allocator while the actual work is perform asynchronously. 
> These tests where performed with 
> `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an optimized build 
> under OS X using clang(trunk) as compiler.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 43561: Improve Ranges parsing to handle single values.

[Klaus Ma]

> On March 21, 2016, 10:02 p.m., Joris Van Remoortere wrote:
> > src/common/values.cpp, line 612
> > 
> >
> > Not yours: We shouldn't capture temporaries by reference.
> > Either:
> > 1) capture by value: `const vector tokens = ...`
> > 2) iterator over the result:
> > ```
> > foreach (const string& token, strings::tokenize(...)) {
> > ```
> > 
> > I mention it because you copy the pattern below.
> 
> Joris Van Remoortere wrote:
> There are a couple more of these that you missed. No need to update the 
> review again, i'll just re-open this and you can fix them after bmahler can 
> take a look.

Updated patches for related code in this files :).


- Klaus


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


On March 22, 2016, 9:22 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43561/
> ---
> 
> (Updated March 22, 2016, 9:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4627
> https://issues.apache.org/jira/browse/MESOS-4627
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improve Ranges parsing to handle single values.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 9f3c0b9930bd3dd9d84d97119419825862f531c3 
>   src/tests/resources_tests.cpp 6b004d64bb25112b19fc5d98b5bca874c5329e8c 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/43561/diff/
> 
> 
> Testing
> ---
> 
> make
> make check GTEST_FILTER=~"*"
> ./src/mesos-test
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 43880: Added allocated metrics for total and allocated scalar resources.

[Ben Mahler]

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


Fix it, then Ship it!




Thanks for the patience Benjamin! I'll make some adjustments based on the 
comments below, and get this committed shortly.


docs/monitoring.md (lines 869 - 917)


Let's say a user makes a custom resource named "quota", then it mixes with 
the "quota" directory we created earlier. The suggestion from the last review 
was to add a "resources/" prefix, sound good?

Once we follow up on ensuring that all resources are represented (not just 
cpus, mem, disk), it would also be great to change this to just be 
parameterized on  like we did with quota.



docs/monitoring.md (line 887)


s/of/or/

Did you do a self review? ;)



docs/monitoring.md (line 908)


s/of/or/



src/master/allocator/mesos/hierarchical.hpp (lines 290 - 294)


Let's perhaps include the word resources when naming these, and omit the 
comments (since we comment the metric insted of the function in the existing 
code).

Let's do a s/resourceName/resource/ to be consistent with your last patch.

How about `s/_total/_resources_total/` and 
`s/_allocated/_resources_allocated/` here and on the gauge variable names to be 
a bit more clear? It's also consistent with the master's approach to naming 
these gauges (note that this also fits well with naming the metric with a 
"resources/" prefix).



src/master/allocator/mesos/hierarchical.cpp (line 1691)


s/resourceName/resource/

s/std:://



src/master/allocator/mesos/hierarchical.cpp (line 1700)


s/.get()./->/



src/master/allocator/mesos/hierarchical.cpp (line 1708)


s/resourceName/resource/

s/std:://



src/master/allocator/mesos/hierarchical.cpp (line 1713)


s/.get()./->/



src/master/allocator/mesos/hierarchical.cpp (line 1715)


two newlines here



src/master/allocator/mesos/metrics.hpp (lines 58 - 62)


s/kind//

How about `resources_total` and `resources_offered_or_allocated`?



src/master/allocator/mesos/metrics.cpp (lines 50 - 53)


Let's also pull in dominic's TODO about dynamically adding gauges:

```
// Create resource gauges.
//
// TODO(bbannier) Add support for more than just scalar resources.
// TODO(bbannier) Simplify this once MESOS-3214 is fixed.
// TODO(dhamon): Set these up dynamically when adding a slave based on the
// resources the slave exposes.
```



src/master/allocator/mesos/metrics.cpp (lines 54 - 55)


s/resourceKinds/resources/ (since you called the singular loop variable 
'resource' already.. :))



src/master/allocator/mesos/metrics.cpp (line 55)


newline here?



src/master/allocator/mesos/metrics.cpp (lines 56 - 61)


We don't need a map right now, we can just use a vector to be consistent 
with the master's metrics. We can switch to a map when we start dynamically 
adding gauges.



src/master/allocator/mesos/metrics.cpp (lines 56 - 74)


How about the following structure, which is similar to the master's metrics 
and seems a bit cleaner?

```
  foreach (const string& resource, resources) {
Gauge total(
"allocator/mesos/resources/" + resource + "/total",
defer(allocator,
  ::_resources_total,
  resource));

Gauge offered_or_allocated(
"allocator/mesos/resources/" + resource + "/offered_or_allocated",
defer(allocator,
  
::_resources_offered_or_allocated,
  resource));

resources_total.push_back(total);
resources_offered_or_allocated.push_back(offered_or_allocated);

process::metrics::add(total);
process::metrics::add(offered_or_allocated);
  }
```



src/master/allocator/mesos/metrics.cpp (line 106)


Whoops, I missed these earlier, no need for std:: qualifiers here and below.



src/tests/hierarchical_allocator_tests.cpp (lines 2437 - 2438)

Review Request 45183: Implemented mounting host system config files to container.

[Gilbert Song]

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

Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
Chen.


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


Repository: mesos


Description
---

Implemented mounting host system config files to container.


Diffs
-

  src/slave/containerizer/mesos/isolators/docker/runtime.hpp 
90179119ef297855091dad3fe969aa79810bf209 
  src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
f97a9a92895387a9d504810a2ae971cfb5d3dbb4 

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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 45185: Introduced an agent flag 'system_config_files'.

[Gilbert Song]

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

Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
Chen.


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


Repository: mesos


Description
---

Introduced an agent flag 'system_config_files'.


Diffs
-

  docs/configuration.md 73ee8fa7a77a6ceaf44e1f4c54ab4027a7109258 
  src/slave/flags.hpp 3067c128f1fded4f130f4d45f191584c2f30ad9c 
  src/slave/flags.cpp ce028825ad99f54a231b4b18dde277b63aa0525c 

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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 45184: Added test for mounting host system config files.

[Gilbert Song]

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

Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
Chen.


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


Repository: mesos


Description
---

Added test for mounting host system config files.


Diffs
-

  src/tests/containerizer/runtime_isolator_tests.cpp 
9f3b0b08da7cebba722062a9932fae1b5f825efb 

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


Testing
---

make check

sudo ./bin/mesos-test.sh


Thanks,

Gilbert Song

Re: Review Request 44674: Supported image name specified with private registry prefix.

[Guangya Liu]

> On 三月 22, 2016, 2:23 p.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp, line 
> > 395
> > 
> >
> > Just a question: Where did set the schema was set to `https` for this 
> > case?
> 
> Gilbert Song wrote:
> Hey Guangya, we set `https` as default in /uri/fetchers/docker.cpp:
> 
> DockerFetcherPluginProcess::getManifestUri()
> DockerFetcherPluginProcess::getBlobUri()

Thanks Gilbert, got it.


- Guangya


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


On 三月 17, 2016, 4:57 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44674/
> ---
> 
> (Updated 三月 17, 2016, 4:57 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4939
> https://issues.apache.org/jira/browse/MESOS-4939
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported image name specified with private registry prefix.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6d637ed14f35feb554c8fcc63a7a7e046aaca574 
> 
> Diff: https://reviews.apache.org/r/44674/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-test.sh --gtest_filter="ProvisionerDockerRegistryPullerTest"
> 
> Tested with private registry localhost:80/ubuntu
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44946: Add utility functions to manipulate XFS project quotas.

[James Peach]

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

(Updated March 22, 2016, 11:24 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased and fixed review comments.


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


Repository: mesos


Description
---

Add utility functions to manipulate XFS project quotas.


Diffs (updated)
-

  src/Makefile.am 6552e48eab2708a28dd69adba3ec759cb5aeca4c 
  src/slave/containerizer/mesos/isolators/disk/xfs/utils.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/disk/xfs/utils.cpp PRE-CREATION 

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


Testing
---

Make check. Manual verification. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44948: Add XFS disk resource isolator.

[James Peach]

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

(Updated March 22, 2016, 11:24 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


Bugs: MESOs-4828
https://issues.apache.org/jira/browse/MESOs-4828


Repository: mesos


Description
---

Track sandbox directory usage by dynamically assigning XFS project
quotas. We track a range of XFS project IDs, assigning a project ID
and a project quota to each sandbox as it is created. When the task
reaches the quota, writes will fail with EDQUOT, and the task will have
an opportunity to handle that.

Quotas are not applied to volume resources since the isolator interface
has no insight into the volume lifecycle. Thus it is not currently
possible to accurately assign and reclaim project IDs.

If LOW is the lower bound of the project ID range and HIGH is the upper
bound, you can show the currently allocated project quotas using the
xfs_quota command:

  $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"

To show the project ID assigned to the file PATH, use the xfs_io command:

  $ xfs_io -r -c stat PATH


Diffs (updated)
-

  src/Makefile.am 6552e48eab2708a28dd69adba3ec759cb5aeca4c 
  src/slave/containerizer/mesos/containerizer.cpp 
ee7a265975323ca891114a286357c8e42901560c 
  src/slave/containerizer/mesos/isolators/disk/xfs.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/disk/xfs.cpp PRE-CREATION 
  src/slave/flags.hpp 3067c128f1fded4f130f4d45f191584c2f30ad9c 
  src/slave/flags.cpp ce028825ad99f54a231b4b18dde277b63aa0525c 

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


Testing
---

Make check. Manual testing. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44949: Add XFS disk isolator tests.

[James Peach]

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

(Updated March 22, 2016, 11:24 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Add basic XFS disk isolator tests by cloning the POSIX disk isolator
tests and making minor changes for the differences in semantics.


Diffs (updated)
-

  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 

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


Testing
---

Make check. Manual testing.


Thanks,

James Peach

Re: Review Request 44950: Add XFS disk isolator documentation.

[James Peach]

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

(Updated March 22, 2016, 11:24 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


Bugs: MESOs-4828
https://issues.apache.org/jira/browse/MESOs-4828


Repository: mesos


Description
---

Add XFS disk isolator documentation.


Diffs (updated)
-

  docs/configuration.md 73ee8fa7a77a6ceaf44e1f4c54ab4027a7109258 
  docs/mesos-containerizer.md 15fb5bdbe74e059614b8948108f32cd04b623305 

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


Testing
---

Make check. Source inspection.


Thanks,

James Peach

Re: Review Request 44946: Add utility functions to manipulate XFS project quotas.

[James Peach]

> On March 22, 2016, 4:46 p.m., Gilbert Song wrote:
> > BTW, should we also add `utils.cpp` to `CMakeList.txt`?

Cmake support is going to need a lot more than that :-/


- James


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


On March 22, 2016, 1:20 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44946/
> ---
> 
> (Updated March 22, 2016, 1:20 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4828
> https://issues.apache.org/jira/browse/MESOS-4828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add utility functions to manipulate XFS project quotas.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/slave/containerizer/mesos/isolators/disk/xfs/utils.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/disk/xfs/utils.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44946/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual verification. Tests in subsequent patches.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 44947: Add tests for XFS project quota utilities.

[James Peach]

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

(Updated March 22, 2016, 11:21 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Add tests for XFS project quota utilities.


Diffs (updated)
-

  src/Makefile.am 6552e48eab2708a28dd69adba3ec759cb5aeca4c 
  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 
  src/tests/environment.cpp ee59a09c44242a2d31ff539106edbcb316b120aa 

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


Testing
---

Make check. Manual testing. These tests.


Thanks,

James Peach

Re: Review Request 44945: Add autoconf tests for XFS project quotas.

[James Peach]

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

(Updated March 22, 2016, 11:21 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Add autoconf tests for XFS project quotas.


Diffs (updated)
-

  configure.ac 9ec4bc1cff3b0b46dd2e7ece2c1f2d19ffb8 

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


Testing
---

Make check. Manual verification.


Thanks,

James Peach

Re: Review Request 44135: Libprocess: Use Windows-compatible memory fence in logging.

[Daniel Pravat]

> On March 22, 2016, 9:52 p.m., Neil Conway wrote:
> > 3rdparty/libprocess/include/process/logging.hpp, line 57
> > 
> >
> > I wonder whether we can use something from C++11 to avoid the need for 
> > non-portable code here. From a quick skim of the reference page,  
> > `std::atomic_thread_fence(std::memory_order_seq_cst)` _might_ work, but I 
> > haven't dug in enough to say for sure.

It may be posible, in fact std::atomic_thread_fence uses MemoryBarrier on x64. 
However this requires glog patching.


- Daniel


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


On March 18, 2016, 6:49 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44135/
> ---
> 
> (Updated March 18, 2016, 6:49 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Alex 
> Clemmer, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Libprocess: Use Windows-compatible memory fence in logging.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/logging.hpp 
> ee680ae8ecd837f24ec7731d55680ad8440808db 
> 
> Diff: https://reviews.apache.org/r/44135/diff/
> 
> 
> Testing
> ---
> 
> OSX: make
> Windows: build/run
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 45123: Updated docs for deletion of persistent volumes.

[Neil Conway]

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

(Updated March 22, 2016, 11:02 p.m.)


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


Changes
---

Rebase, update storage-management discussion in multi-disk doc.


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


Repository: mesos


Description
---

Updated docs for deletion of persistent volumes.


Diffs (updated)
-

  docs/multiple-disk.md 5cdce26721212597052515a9cf2fd9277d24e42c 
  docs/persistent-volume.md 4b9c59daf6fdcee4a102e19d6eb4df9b5eddfa54 
  docs/reservation.md 55924adb94028702e15db7e191915157552981d0 
  docs/upgrades.md f8bd8a2fcf1b38eaed1bd77e8a9dd3e3370008e5 

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


Testing
---


Thanks,

Neil Conway

Re: Review Request 45123: Updated docs for deletion of persistent volumes.

[Neil Conway]

> On March 22, 2016, 7:58 a.m., Guangya Liu wrote:
> > What about 
> > https://github.com/apache/mesos/blob/master/docs/multiple-disk.md#storage-management
> >  ? I think that we should also update here as well.

Good catch! Thanks, fixed.


- Neil


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


On March 22, 2016, 6:24 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45123/
> ---
> 
> (Updated March 22, 2016, 6:24 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2408
> https://issues.apache.org/jira/browse/MESOS-2408
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated docs for deletion of persistent volumes.
> 
> 
> Diffs
> -
> 
>   docs/persistent-volume.md 4b9c59daf6fdcee4a102e19d6eb4df9b5eddfa54 
>   docs/reservation.md 55924adb94028702e15db7e191915157552981d0 
>   docs/upgrades.md 83b839f7fb996385baaa6ef1081dc1116cd6e338 
> 
> Diff: https://reviews.apache.org/r/45123/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 44135: Libprocess: Use Windows-compatible memory fence in logging.

[Neil Conway]

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




3rdparty/libprocess/include/process/logging.hpp (line 57)


I wonder whether we can use something from C++11 to avoid the need for 
non-portable code here. From a quick skim of the reference page,  
`std::atomic_thread_fence(std::memory_order_seq_cst)` _might_ work, but I 
haven't dug in enough to say for sure.


- Neil Conway


On March 18, 2016, 6:49 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44135/
> ---
> 
> (Updated March 18, 2016, 6:49 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Alex 
> Clemmer, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Libprocess: Use Windows-compatible memory fence in logging.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/logging.hpp 
> ee680ae8ecd837f24ec7731d55680ad8440808db 
> 
> Diff: https://reviews.apache.org/r/44135/diff/
> 
> 
> Testing
> ---
> 
> OSX: make
> Windows: build/run
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 44674: Supported image name specified with private registry prefix.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44672, 44673, 44674]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 17, 2016, 4:57 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44674/
> ---
> 
> (Updated March 17, 2016, 4:57 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4939
> https://issues.apache.org/jira/browse/MESOS-4939
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported image name specified with private registry prefix.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6d637ed14f35feb554c8fcc63a7a7e046aaca574 
> 
> Diff: https://reviews.apache.org/r/44674/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-test.sh --gtest_filter="ProvisionerDockerRegistryPullerTest"
> 
> Tested with private registry localhost:80/ubuntu
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44514: Implemented prepare() method of "network/cni" isolator.

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/network/cni/cni.hpp (line 117)


Can you use 'Owned' here?



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 228)


s/i/ifIndex/



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 229)


s/names/networkNames/



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (lines 249 - 250)


I would make it more explicit here:
```
NetworkInfo networkInfo;
networkInfo.networkName = networkName;
networkInfo.ifName = "eth" + stringify(ifIndex);

networkInfos.put(name, networkInfo);
```


- Jie Yu


On March 22, 2016, 9:50 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44514/
> ---
> 
> (Updated March 22, 2016, 9:50 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented prepare() method of "network/cni" isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44514/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 45022: Windows: Add Windows-friendly implementation of `rm.hpp`.

[Michael Park]

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




3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rm.hpp (lines 49 - 
53)


```
  const BOOL result = os::stat::isdir(path) ? 
::RemoveDirectory(path.c_str())
: ::DeleteFile(path.c_str());
```



3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp (line 200)


Why do we need this? I would much prefer to remove this.



3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp (lines 239 - 
243)


I understand the comment, but why re-build the expected set from scratch?

That is, I see the pairing of `mkdir` + `insert`, `touch` + `insert`, etc. 
Why not pair up `rm` + `erase`?



3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp (line 247)


`s/ASSERT_ERROR/EXPECT_ERROR/`


- Michael Park


On March 18, 2016, 7:39 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45022/
> ---
> 
> (Updated March 18, 2016, 7:39 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-4415
> https://issues.apache.org/jira/browse/MESOS-4415
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We currently depend on arcane "POSIX-like" Windows APIs like `::remove`.
> This API is incompatible with NT paths -- which we will eventually have
> to transition to for the Windows integration -- and has no documented
> behavior interacting with subsystems like the driver subsystem, which is
> required for symlinks.
> 
> This commit will move us to proper core Windows API equivalents for
> `::remove`.
> 
> Review: https://reviews.apache.org/r/45022
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 7bd4bfbc2ec5922879dcefddc12137336b11be52 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rm.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/rm.hpp 
> 52568b303c03fd57b81f6cc67782444ce734dd41 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rm.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
> 4c30189bb8261ccfc699da0f31b8b1fd3e9b3c83 
> 
> Diff: https://reviews.apache.org/r/45022/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 45163: Reflecting rename of Tachyon to Alluxio.

[Neil Conway]

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


Ship it!




Ship It!

- Neil Conway


On March 22, 2016, 6:03 p.m., Jiri Simsa wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45163/
> ---
> 
> (Updated March 22, 2016, 6:03 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-5002
> https://issues.apache.org/jira/browse/MESOS-5002
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Tachyon has been renamed to Alluxio. The documentation on frameworks should 
> be updated to reflect that.
> 
> 
> Diffs
> -
> 
>   docs/frameworks.md 7c5517cc4f18fc5e0c2348e6be42c186f4a280b8 
> 
> Diff: https://reviews.apache.org/r/45163/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jiri Simsa
> 
>

Re: Review Request 44622: Introduced a protobuf message "NetworkInfo".

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/network/cni/cni.hpp (line 30)


Let's not use this. Just use fully qualified name in the header.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 41)


Let's do
```
namespace spec = mesos::internal::slave::cni::spec;
```


- Jie Yu


On March 22, 2016, 7:33 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44622/
> ---
> 
> (Updated March 22, 2016, 7:33 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced a protobuf message "NetworkInfo".
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.proto PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44622/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44255]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 14, 2016, 5:50 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44255/
> ---
> 
> (Updated March 14, 2016, 5:50 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Guangya Liu, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-4492
> https://issues.apache.org/jira/browse/MESOS-4492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
>   src/master/http.cpp 950206baf7f3a1cccdc49d810126473966d8d021 
>   src/master/master.cpp 8d6d3c6468c6b85fe09c33cf9747cc3d1f515ab9 
>   src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
>   src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
>   src/tests/metrics_tests.cpp 419d275e0b32817388120222bd433ee6f4835efd 
> 
> Diff: https://reviews.apache.org/r/44255/diff/
> 
> 
> Testing
> ---
> 
> ChangLog:
> 
> v3:
>   - Move the couting out of common code path to http endpoint and master 
> accept call separately to reflect its logic.
> 
> v2:
>   - Documenting those metrics
>   - Add test code for MetricsTest as suggested by Guangya
>   - post-review.py does not update original 
> RR(https://reviews.apache.org/r/44058/), but only create a new one even if I 
> rebased.
> 
> 
> Tests:
> 1. make check GTEST_FILTER="MetricsTest.Master" on Centos-7 
> (3.10.0-123.el7.x86_640)
> 
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from MetricsTest
> [ RUN  ] MetricsTest.Master
> [   OK ] MetricsTest.Master (211 ms)
> [--] 1 test from MetricsTest (211 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (244 ms total)
> [  PASSED  ] 1 test
> 
> 2. Verify its functionality with 'reserve' http endpoint as an test case
> 
> # curl http://ipdc02-kvm-guest2:5050/metrics/snapshot | python -mjson.tool | 
> grep reserve
> "master/messages_reserve_resource": 0.0,
> "master/messages_unreserve_resource": 0.0,
> 
> 
> # curl -i -d slaveId=6250553a-2f39-4a92-9073-4618d130f433-S1  -d resources='[ 
> { "name": "cpus", "type": "SCALAR","scalar": { "value": 1 
> },"reservation":{"principal": "XiaoHaHa"}}  ]' -X POST  
> ipdc02-kvm-guest2:5050/master/reserve
> HTTP/1.1 200 OK
> Date: Fri, 26 Feb 2016 19:59:01 GMT
> Content-Length: 0
> 
> # curl http://ipdc02-kvm-guest2:5050/metrics/snapshot  | python -mjson.tool | 
> grep reserve
> "master/messages_reserve_resource": 1.0,
> "master/messages_unreserve_resource": 0.0,
> 
> 
> Thanks,
> 
> fan du
> 
>

Re: Review Request 43884: Added allocator metrics for used quotas.

[Ben Mahler]

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


Fix it, then Ship it!




Looks good! I'll make some minor adjustments based on the comments prior to 
getting this committed.


docs/monitoring.md (lines 876 - 890)


Hm.. looks like the "resources/" prefix was left out? Any reason for that? 
The "resources/" prefix helps us if we add any quota metrics that are not tied 
to the resources.



docs/monitoring.md (line 880)


"offered or allocated" here as well?



src/master/allocator/mesos/hierarchical.cpp (line 1697)


Just 2 spaces need here instead of 4



src/master/allocator/mesos/metrics.hpp (lines 58 - 64)


s/resource kind/resource/ just because we haven't generally been using the 
work "kind" in this manner to my knowledge

Only 2 spaces needed on the continuation lines here.



src/master/allocator/mesos/metrics.cpp (line 21)


This should go above process and stout here?



src/master/allocator/mesos/metrics.cpp (line 37)


How about a .self() here insted of taking a reference?



src/master/allocator/mesos/metrics.cpp (line 92)


Any reason not to just pass the 'gauge' here rather than doing the lookup?

```
  allocated.put(resource.name(), gauge);
  process::metrics::add(gauge);
```

Ditto below.



src/master/allocator/mesos/metrics.cpp (line 102)


2 spaces unless wrapping an open parenthesis



src/master/allocator/mesos/metrics.cpp (line 103)


you can omit the 'allocator' here to use a deferred context where 
libprocess manages the execution, see here: 
https://github.com/apache/mesos/tree/0.28.0/3rdparty/libprocess#defer


- Ben Mahler


On March 18, 2016, 4:08 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43884/
> ---
> 
> (Updated March 18, 2016, 4:08 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4723
> https://issues.apache.org/jira/browse/MESOS-4723
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator metrics for used quotas.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110 
>   src/master/allocator/mesos/hierarchical.hpp 
> 0c622b8569bc79ae4e365a57e463ca5349356713 
>   src/master/allocator/mesos/hierarchical.cpp 
> 70291075c00a9a557529c2562dedcfc6c6c3ec32 
>   src/master/allocator/mesos/metrics.hpp 
> d04e9a11c77b6fb2d522608e3085f81f8a6657ca 
>   src/master/allocator/mesos/metrics.cpp 
> 46dd7f7b3d8d49c59b42e789fe5dcd49f1693ace 
>   src/tests/hierarchical_allocator_tests.cpp 
> 459e02576f6d05abbbcc83ae5cabac5c66e93f05 
> 
> Diff: https://reviews.apache.org/r/43884/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> I confirmed that this does not lead to general performance regressions in the 
> allocator; this is partially expected since the added code only inserts 
> metrics in the allocator while the actual work is perform asynchronously. 
> These tests where performed with 
> `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an optimized build 
> under OS X using clang(trunk) as compiler.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 44934: Updated mesos-execute to add support for Appc.

[Jojy Varghese]

> On March 22, 2016, 5:10 p.m., Tom Runyon wrote:
> > It would be helpful to update docs/container-image.md to include example 
> > appc deployments using mesos-execute.

Created MESOS-5006.


- Jojy


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


On March 20, 2016, 4:19 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44934/
> ---
> 
> (Updated March 20, 2016, 4:19 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4978
> https://issues.apache.org/jira/browse/MESOS-4978
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated mesos-execute to add support for Appc.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/44934/diff/
> 
> 
> Testing
> ---
> 
> Tested with various Appc images.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44549: Introduced a protobuf message "NetworkConfig".

[Jie Yu]

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




src/Makefile.am (line 788)


the source is guarded by LINUX but the header is not. I removed the guard 
for LINUX for the source for now (since spec can be used on non-linux based 
systems).


- Jie Yu


On March 17, 2016, 8:56 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44549/
> ---
> 
> (Updated March 17, 2016, 8:56 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced a protobuf message "NetworkConfig".
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0517dd1ae7125ac1ae85cc83b6daaca66a10b8b4 
>   src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.proto PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44549/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 44549: Introduced a protobuf message "NetworkConfig".

[Jie Yu]

> On March 17, 2016, 3:37 a.m., Jie Yu wrote:
> > src/CMakeLists.txt, line 279
> > 
> >
> > I don't think we put headers here.
> 
> Qian Zhang wrote:
> I had the same concern before, because it seems most source files here 
> are .cpp. But I also see the followings, so I guess it should be OK to put 
> headers here as well?
> `slave/containerizer/composing.cpp`
> `slave/containerizer/composing.hpp` 
> `slave/containerizer/docker.cpp`
> `slave/containerizer/docker.hpp`

I checked with Alex Clemmer who wrote the CMakefile at the first place. He 
suggested us to not include headers for consistency.

"The short answer is that we should technically do this so that IDEs like 
Visual Studio and Eclipse correctly report the header files in the source, but 
we don't, and it's not important for the functionality (_e.g._, we will 
correctly rebuild if you alter a header file, as long as that folder is 
included by CMake). So in other words: as implemented, including .hpp would 
require some doing, and we should sit down and do all the files if we choose 
this."


- Jie


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


On March 17, 2016, 8:56 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44549/
> ---
> 
> (Updated March 17, 2016, 8:56 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced a protobuf message "NetworkConfig".
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0517dd1ae7125ac1ae85cc83b6daaca66a10b8b4 
>   src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.proto PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44549/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 44851: Renamed an allocator metric.

[Ben Mahler]

> On March 22, 2016, 5:50 p.m., Ben Mahler wrote:
> > The changelog update would be great as well here, I'll take care of that 
> > before committing. Thanks!

I'll also update the existing test:

```
$ grep -R allocator/event_queue_dispatches src/tests
src/tests/master_tests.cpp:  EXPECT_EQ(1u, 
snapshot.values.count("allocator/event_queue_dispatches"));
```


- Ben


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


On March 22, 2016, 5:32 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44851/
> ---
> 
> (Updated March 22, 2016, 5:32 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-5001
> https://issues.apache.org/jira/browse/MESOS-5001
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since allocators can be replaced with a custom module instead use a
> name clearly communicating that the metrics reported here are related
> to the default (hierarchical) allocator.
> 
> For backwards compatibility we continue to support the old metrics
> name.
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md 83b839f7fb996385baaa6ef1081dc1116cd6e338 
>   src/master/allocator/mesos/metrics.hpp 
> d04e9a11c77b6fb2d522608e3085f81f8a6657ca 
>   src/master/allocator/mesos/metrics.cpp 
> 46dd7f7b3d8d49c59b42e789fe5dcd49f1693ace 
> 
> Diff: https://reviews.apache.org/r/44851/diff/
> 
> 
> Testing
> ---
> 
> check make check (OS X, clang-trunk) here and with later patches using other 
> allocator metrics.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 44852: Documented existing allocator metrics.

[Ben Mahler]

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


Ship it!




Ship It!

- Ben Mahler


On March 18, 2016, 4:08 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44852/
> ---
> 
> (Updated March 18, 2016, 4:08 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented existing allocator metrics.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110 
> 
> Diff: https://reviews.apache.org/r/44852/diff/
> 
> 
> Testing
> ---
> 
> Checked rendering with packaged docker image.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 44851: Renamed an allocator metric.

[Ben Mahler]

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


Fix it, then Ship it!




The changelog update would be great as well here, I'll take care of that before 
committing. Thanks!


docs/upgrades.md (line 157)


Looks like this is missing the "-x" from your href above?


- Ben Mahler


On March 22, 2016, 5:32 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44851/
> ---
> 
> (Updated March 22, 2016, 5:32 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-5001
> https://issues.apache.org/jira/browse/MESOS-5001
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since allocators can be replaced with a custom module instead use a
> name clearly communicating that the metrics reported here are related
> to the default (hierarchical) allocator.
> 
> For backwards compatibility we continue to support the old metrics
> name.
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md 83b839f7fb996385baaa6ef1081dc1116cd6e338 
>   src/master/allocator/mesos/metrics.hpp 
> d04e9a11c77b6fb2d522608e3085f81f8a6657ca 
>   src/master/allocator/mesos/metrics.cpp 
> 46dd7f7b3d8d49c59b42e789fe5dcd49f1693ace 
> 
> Diff: https://reviews.apache.org/r/44851/diff/
> 
> 
> Testing
> ---
> 
> check make check (OS X, clang-trunk) here and with later patches using other 
> allocator metrics.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 45168: Removed old comment from 'mesos.proto'.

[Greg Mann]

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

Review request for mesos, Adam B, Joris Van Remoortere, and Vinod Kone.


Repository: mesos


Description
---

Removed old comment from 'mesos.proto'.


Diffs
-

  include/mesos/mesos.proto b965f5a6b1aff28feb2ab088c30ea927b9247403 

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


Testing
---


Thanks,

Greg Mann

Re: Review Request 44851: Renamed an allocator metric.

[Benjamin Bannier]

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

(Updated March 22, 2016, 6:32 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
---

Fixed accidentally mutilation of unrelated paragraphs.


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


Repository: mesos


Description
---

Since allocators can be replaced with a custom module instead use a
name clearly communicating that the metrics reported here are related
to the default (hierarchical) allocator.

For backwards compatibility we continue to support the old metrics
name.


Diffs (updated)
-

  docs/upgrades.md 83b839f7fb996385baaa6ef1081dc1116cd6e338 
  src/master/allocator/mesos/metrics.hpp 
d04e9a11c77b6fb2d522608e3085f81f8a6657ca 
  src/master/allocator/mesos/metrics.cpp 
46dd7f7b3d8d49c59b42e789fe5dcd49f1693ace 

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


Testing
---

check make check (OS X, clang-trunk) here and with later patches using other 
allocator metrics.


Thanks,

Benjamin Bannier

Review Request 45166: Fixed flaky `MasterTest.SlavesEndpointTwoSlaves`.

[Anand Mazumdar]

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

Review request for mesos, Alexander Rojas and Neil Conway.


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


Repository: mesos


Description
---

We were not correctly waiting for the master to register the
slave before making a call to the `slaves` endpoint. This change
adds a pause/settle to ensure that. Note that `FUTURE_PROTOBUF`
just gurrantees that the method has dispatched but does not ensure
that it has completed.


Diffs
-

  src/tests/master_tests.cpp d34ba0bdd71efd261850d8c205c16cecb701ac7c 

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


Testing
---

Ran it in a loop. Previously used to fail quite often.


Thanks,

Anand Mazumdar

Re: Review Request 44748: Stout: Added implementation of `read` that works on Windows.

[Alex Clemmer]

> On March 21, 2016, 9:02 a.m., Joris Van Remoortere wrote:
> > Can you consider the feedback frmo the `write` review and rebase this?

Alright, for posterity the `write` review is #44747, and I've adopted those 
suggestions here, too.


- Alex


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


On March 22, 2016, 5:21 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44748/
> ---
> 
> (Updated March 22, 2016, 5:21 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-3641
> https://issues.apache.org/jira/browse/MESOS-3641
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: Added implementation of `read` that works on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 7bd4bfbc2ec5922879dcefddc12137336b11be52 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/read.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/read.hpp 
> d494cbf8a2a24c88b0569634cfcbf29de0784797 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/read.hpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44748/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 44748: Stout: Added implementation of `read` that works on Windows.

[Alex Clemmer]

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

(Updated March 22, 2016, 5:21 p.m.)


Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


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


Repository: mesos


Description
---

Stout: Added implementation of `read` that works on Windows.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
7bd4bfbc2ec5922879dcefddc12137336b11be52 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/read.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/read.hpp 
d494cbf8a2a24c88b0569634cfcbf29de0784797 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/read.hpp 
PRE-CREATION 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 44747: Stout: Added implementation of `write` that works on Windows.

[Alex Clemmer]

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

(Updated March 22, 2016, 5:20 p.m.)


Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


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


Repository: mesos


Description
---

Stout: Added implementation of `write` that works on Windows.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
7bd4bfbc2ec5922879dcefddc12137336b11be52 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/write.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp 
a2a5d0b7af673fee89f66e2722846af7b7fd059a 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/write.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/write.hpp 
1715bf5e6c590e7d14f59d517b30a281346365be 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 44661: Deprecated the `docker_stop_timeout` flag.

[Alexander Rukletsov]

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

(Updated March 22, 2016, 5:15 p.m.)


Review request for mesos, Ben Mahler and Timothy Chen.


Changes
---

BenM's comments; changelog update.


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


Repository: mesos


Description
---

Instead, a combination of `executor_shutdown_grace_period`
agent flag and task kill policies should be used.


Diffs (updated)
-

  CHANGELOG eab924e8066875d7679f5d09af2da434aa554699 
  docs/configuration.md d10fa2e7fc7c477de2f0e30e10da6d817ecbf404 
  src/docker/executor.hpp abbc419533ab40312e917931a2fc2ce78b38da41 
  src/slave/containerizer/docker.cpp fb9188a19a5cd8211d4f36f9647ebb70de560109 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 

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


Testing
---

The complete chain was tested. See https://reviews.apache.org/r/44662/.


Thanks,

Alexander Rukletsov

Re: Review Request 44660: Used `KillPolicy` and shutdown grace period in docker executor.

[Alexander Rukletsov]

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

(Updated March 22, 2016, 5:13 p.m.)


Review request for mesos, Ben Mahler, Gilbert Song, and Timothy Chen.


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


Repository: mesos


Description
---

The docker executor determines how much time it allots the
underlying container to clean up (via passing the timeout to
the docker daemon) based on both optional task's `KillPolicy`
and optional `shutdown_grace_period` field in `ExecutorInfo`.


Diffs (updated)
-

  include/mesos/mesos.proto b965f5a6b1aff28feb2ab088c30ea927b9247403 
  include/mesos/v1/mesos.proto d2ab6ed187ec0d0e5dbac92607b612bb55b1a682 
  src/docker/executor.cpp afc769d0887e3842106e4c350e94c95c8ffc085e 

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


Testing
---

The complete chain was tested. See https://reviews.apache.org/r/44662/.


Thanks,

Alexander Rukletsov

Re: Review Request 44657: Used `KillPolicy` and shutdown grace period in command executor.

[Alexander Rukletsov]

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

(Updated March 22, 2016, 5:12 p.m.)


Review request for mesos, Ben Mahler and Gilbert Song.


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


Repository: mesos


Description
---

The command executor determines how much time it allots the
underlying task to clean up (effectively how long to wait for
the task to comply to SIGTERM before sending SIGKILL) based
on both optional task's `KillPolicy` and optional
`shutdown_grace_period` field in `ExecutorInfo`.


Diffs (updated)
-

  src/launcher/executor.cpp 2df62f09637b55c63ae6fe5d5a70b8debc02fbe2 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 

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


Testing
---

The complete chain was tested. See https://reviews.apache.org/r/44662/.


Thanks,

Alexander Rukletsov

Re: Review Request 44657: Used `KillPolicy` and shutdown grace period in command executor.

[Alexander Rukletsov]

> On March 19, 2016, 1:52 a.m., Ben Mahler wrote:
> > src/launcher/executor.cpp, line 929
> > 
> >
> > It doesn't crash, it just exits :)

Oh, right : )


- Alexander


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


On March 18, 2016, 5:19 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44657/
> ---
> 
> (Updated March 18, 2016, 5:19 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Gilbert Song.
> 
> 
> Bugs: MESOS-4909
> https://issues.apache.org/jira/browse/MESOS-4909
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The command executor determines how much time it allots the
> underlying task to clean up (effectively how long to wait for
> the task to comply to SIGTERM before sending SIGKILL) based
> on both optional task's `KillPolicy` and optional
> `shutdown_grace_period` field in `ExecutorInfo`.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 4149f084b0b234fb995df04e9d1ca704feec082e 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
> 
> Diff: https://reviews.apache.org/r/44657/diff/
> 
> 
> Testing
> ---
> 
> The complete chain was tested. See https://reviews.apache.org/r/44662/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 44934: Updated mesos-execute to add support for Appc.

[Tom Runyon]

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



It would be helpful to update docs/container-image.md to include example appc 
deployments using mesos-execute.

- Tom Runyon


On March 20, 2016, 4:19 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44934/
> ---
> 
> (Updated March 20, 2016, 4:19 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4978
> https://issues.apache.org/jira/browse/MESOS-4978
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated mesos-execute to add support for Appc.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/44934/diff/
> 
> 
> Testing
> ---
> 
> Tested with various Appc images.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44707: Added validation for task's kill policy.

[Alexander Rukletsov]

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

(Updated March 22, 2016, 5:10 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Added a test.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/validation.hpp 29dbdf1da540db4966ff9e86037badde06e69a4b 
  src/master/validation.cpp 820a9faee6fde09a98317854b181fe897167e2ca 
  src/tests/master_validation_tests.cpp 
8d0070a1b8e8dcc7fe6360f8c6c6182ba9edef7d 

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


Testing
---

On Mac OS 10.10.4:
`make check`
`GTEST_FILTER="TaskValidationTest.KillPolicyGracePeriodIsNonNegative" 
./bin/mesos-tests.sh --gtest_repeat=100 --gtest_break_on_failure`


Thanks,

Alexander Rukletsov

Re: Review Request 44656: Introduced `KillPolicy` protobuf.

[Alexander Rukletsov]

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

(Updated March 22, 2016, 5:09 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

BenM's comments; changelog update.


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


Repository: mesos


Description
---

Describes a kill policy for a task. Currently does not express
different policies (e.g. hitting HTTP endpoints), only controls
how long to wait between graceful and forcible task kill.


Diffs (updated)
-

  CHANGELOG eab924e8066875d7679f5d09af2da434aa554699 
  include/mesos/mesos.proto b965f5a6b1aff28feb2ab088c30ea927b9247403 
  include/mesos/v1/mesos.proto d2ab6ed187ec0d0e5dbac92607b612bb55b1a682 

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


Testing
---

The complete chain was tested. See https://reviews.apache.org/r/44662/.


Thanks,

Alexander Rukletsov

Re: Review Request 44994: Added a test for executor shutdown grace period.

[Alexander Rukletsov]

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

(Updated March 22, 2016, 5:07 p.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762 
  src/tests/slave_tests.cpp ea1d776077bf638885db8421194aa4427c772169 

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


Testing
---

On Mac OS 10.10.4:
`make check`
`GTEST_FILTER="*ExecutorShutdownGracePeriod*" ./bin/mesos-tests.sh 
--gtest_repeat=100 --gtest_break_on_failure`


Thanks,

Alexander Rukletsov

Re: Review Request 45039: Updated the comment for launching tasks and accepting offers.

[Alexander Rukletsov]

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

(Updated March 22, 2016, 5:07 p.m.)


Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

If the task does not pass validation,
its resources are considered declined.


Diffs (updated)
-

  docs/app-framework-development-guide.md 
1d8bebde67f69fd414509b8861571137d3569b46 
  include/mesos/scheduler.hpp 14c7ff964aa7b94f439d16e605380661d2279d54 
  src/java/src/org/apache/mesos/SchedulerDriver.java 
bf866f5ebece2505eaa27bf39a1382cd1a2a069a 
  src/python/interface/src/mesos/interface/__init__.py 
232890daa6d222ae1c86906bbc484c8e635c4eb7 

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


Testing
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Re: Review Request 44655: Made `shutdown_grace_period` configurable in `ExecutorInfo`.

[Alexander Rukletsov]

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

(Updated March 22, 2016, 5:05 p.m.)


Review request for mesos, Ben Mahler and Gilbert Song.


Changes
---

Updated changelog.


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


Repository: mesos


Description
---

If `ExecutorInfo.shutdown_grace_period` is set, the executor
driver uses it, otherwise it falls back to the environment
variable `MESOS_EXECUTOR_SHUTDOWN_GRACE_PERIOD`.


Diffs (updated)
-

  CHANGELOG eab924e8066875d7679f5d09af2da434aa554699 
  docs/configuration.md d10fa2e7fc7c477de2f0e30e10da6d817ecbf404 
  include/mesos/executor/executor.proto 
ae211194a44e0bf2fadc79e833881e45ea3eb2c2 
  include/mesos/mesos.proto b965f5a6b1aff28feb2ab088c30ea927b9247403 
  include/mesos/v1/executor/executor.proto 
36a2b3f9bc3aaa524f655b9e686a6d33512e6aaa 
  include/mesos/v1/mesos.proto d2ab6ed187ec0d0e5dbac92607b612bb55b1a682 
  src/slave/containerizer/containerizer.cpp 
f6fc7863d0c215611f170dc0c89aa229407b5137 
  src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 

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


Testing
---

The complete chain was tested. See https://reviews.apache.org/r/44662/.


Thanks,

Alexander Rukletsov

Re: Review Request 44946: Add utility functions to manipulate XFS project quotas.

[Gilbert Song]

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



BTW, should we also add `utils.cpp` to `CMakeList.txt`?


src/slave/containerizer/mesos/isolators/disk/xfs/utils.cpp (line 94)


Just some small formatting issues:

We try to add Apostrophe for the variables in error/failure messages, e.g.,
```
return ErrnoError("Unable to access '" + path + "'");
```

Could we update the others below :) ?



src/slave/containerizer/mesos/isolators/disk/xfs/utils.cpp (line 99)


ditto.



src/slave/containerizer/mesos/isolators/disk/xfs/utils.cpp (line 104)


one newline above?



src/slave/containerizer/mesos/isolators/disk/xfs/utils.cpp (line 139)


ditto.



src/slave/containerizer/mesos/isolators/disk/xfs/utils.cpp (line 177)


ditto.



src/slave/containerizer/mesos/isolators/disk/xfs/utils.cpp (line 228)


ditto here. Could we do:

```return Error("Failed to open '" + path + "': " + fd.error());
```



src/slave/containerizer/mesos/isolators/disk/xfs/utils.cpp (line 235)


ditto.



src/slave/containerizer/mesos/isolators/disk/xfs/utils.cpp (line 256)


ditto.



src/slave/containerizer/mesos/isolators/disk/xfs/utils.cpp (line 264)


ditto.



src/slave/containerizer/mesos/isolators/disk/xfs/utils.cpp (line 275)


ditto.



src/slave/containerizer/mesos/isolators/disk/xfs/utils.cpp (line 291)


ditto.



src/slave/containerizer/mesos/isolators/disk/xfs/utils.cpp (line 299)


ditto.



src/slave/containerizer/mesos/isolators/disk/xfs/utils.cpp (line 310)


ditto.


- Gilbert Song


On March 21, 2016, 6:20 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44946/
> ---
> 
> (Updated March 21, 2016, 6:20 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4828
> https://issues.apache.org/jira/browse/MESOS-4828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add utility functions to manipulate XFS project quotas.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/slave/containerizer/mesos/isolators/disk/xfs/utils.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/disk/xfs/utils.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44946/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual verification. Tests in subsequent patches.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 44674: Supported image name specified with private registry prefix.

[Gilbert Song]

> On March 22, 2016, 7:23 a.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp, line 
> > 395
> > 
> >
> > Just a question: Where did set the schema was set to `https` for this 
> > case?

Hey Guangya, we set `https` as default in /uri/fetchers/docker.cpp:

DockerFetcherPluginProcess::getManifestUri()
DockerFetcherPluginProcess::getBlobUri()


- Gilbert


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


On March 17, 2016, 9:57 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44674/
> ---
> 
> (Updated March 17, 2016, 9:57 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4939
> https://issues.apache.org/jira/browse/MESOS-4939
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported image name specified with private registry prefix.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6d637ed14f35feb554c8fcc63a7a7e046aaca574 
> 
> Diff: https://reviews.apache.org/r/44674/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-test.sh --gtest_filter="ProvisionerDockerRegistryPullerTest"
> 
> Tested with private registry localhost:80/ubuntu
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44672: Added normalize method to registry puller.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On March 21, 2016, 7:06 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44672/
> ---
> 
> (Updated March 21, 2016, 7:06 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, Shuai Lin, 
> and Timothy Chen.
> 
> 
> Bugs: MESOS-4877
> https://issues.apache.org/jira/browse/MESOS-4877
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added normalize method to registry puller.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6d637ed14f35feb554c8fcc63a7a7e046aaca574 
> 
> Diff: https://reviews.apache.org/r/44672/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-test.sh 
> --gtest_filter="*ProvisionerDockerRegistryPullerTest*"
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44673: Added test for registry puller normalize.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On March 21, 2016, 7:06 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44673/
> ---
> 
> (Updated March 21, 2016, 7:06 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, Shuai Lin, 
> and Timothy Chen.
> 
> 
> Bugs: MESOS-4877
> https://issues.apache.org/jira/browse/MESOS-4877
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for registry puller normalize.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> e4b30b9e51cd1191bfa9d9cf68a274814fdc4873 
> 
> Diff: https://reviews.apache.org/r/44673/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-test.sh 
> --gtest_filter="*ProvisionerDockerRegistryPullerTest*"
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 45134: Skip FetcherTest zip tests when `unzip` is uninstalled.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On March 22, 2016, 1:10 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45134/
> ---
> 
> (Updated March 22, 2016, 1:10 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Bugs: MESOS-4993
> https://issues.apache.org/jira/browse/MESOS-4993
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Skip FetcherTest zip tests when `unzip` is uninstalled.
> 
> 
> Diffs
> -
> 
>   src/tests/environment.cpp ee1bbe6b4e3dda1e27b63d71a08ef0d1d254741a 
>   src/tests/fetcher_tests.cpp 375e690ead4f954a66a6efc2f2daa7fb1f1a024d 
> 
> Diff: https://reviews.apache.org/r/45134/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 45082: Implemented cleanup() method of "network/cni" isolator.

[Avinash sridharan]

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




src/slave/containerizer/mesos/isolators/network/cni/cni.hpp (line 151)


I guess if we are going to use `attach` for connecting to networks, lets 
use `detach` over here.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 554)


Calling dispatch on itself I think can be avoided.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (lines 644 - 645)


These two CHECKS don't make sense. What if the plugin got deleted or there 
was a bug in the plugin because of which it wasn't able to delete the 
interfaces or release the IP addresses. The Agent should not die because of an 
error in a 3rd party plugin.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (lines 647 - 649)


This is a bit odd, __disconnect always returns an error ? The plugin can 
return error codes and error logs which we should be propagating upstream 
through failure semantics.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 653)


Why are we returning a Future over here? We should be 
returning a Future there is no list since you are deletnig the entire 
container directory in one shot. This seems a bit odd.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 686)


This is dangerous we should be returning a const reference passed down in 
the call as a return argument. Just do a return list() over here. 

PS: Please see my comment on returning Future above.


- Avinash sridharan


On March 20, 2016, 4:30 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45082/
> ---
> 
> (Updated March 20, 2016, 4:30 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented cleanup() method of "network/cni" isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45082/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

[Greg Mann]

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


Fix it, then Ship it!





src/master/master.cpp (line 3028)


Blank line after this?


- Greg Mann


On March 14, 2016, 5:50 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44255/
> ---
> 
> (Updated March 14, 2016, 5:50 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Guangya Liu, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-4492
> https://issues.apache.org/jira/browse/MESOS-4492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
>   src/master/http.cpp 950206baf7f3a1cccdc49d810126473966d8d021 
>   src/master/master.cpp 8d6d3c6468c6b85fe09c33cf9747cc3d1f515ab9 
>   src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
>   src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
>   src/tests/metrics_tests.cpp 419d275e0b32817388120222bd433ee6f4835efd 
> 
> Diff: https://reviews.apache.org/r/44255/diff/
> 
> 
> Testing
> ---
> 
> ChangLog:
> 
> v3:
>   - Move the couting out of common code path to http endpoint and master 
> accept call separately to reflect its logic.
> 
> v2:
>   - Documenting those metrics
>   - Add test code for MetricsTest as suggested by Guangya
>   - post-review.py does not update original 
> RR(https://reviews.apache.org/r/44058/), but only create a new one even if I 
> rebased.
> 
> 
> Tests:
> 1. make check GTEST_FILTER="MetricsTest.Master" on Centos-7 
> (3.10.0-123.el7.x86_640)
> 
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from MetricsTest
> [ RUN  ] MetricsTest.Master
> [   OK ] MetricsTest.Master (211 ms)
> [--] 1 test from MetricsTest (211 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (244 ms total)
> [  PASSED  ] 1 test
> 
> 2. Verify its functionality with 'reserve' http endpoint as an test case
> 
> # curl http://ipdc02-kvm-guest2:5050/metrics/snapshot | python -mjson.tool | 
> grep reserve
> "master/messages_reserve_resource": 0.0,
> "master/messages_unreserve_resource": 0.0,
> 
> 
> # curl -i -d slaveId=6250553a-2f39-4a92-9073-4618d130f433-S1  -d resources='[ 
> { "name": "cpus", "type": "SCALAR","scalar": { "value": 1 
> },"reservation":{"principal": "XiaoHaHa"}}  ]' -X POST  
> ipdc02-kvm-guest2:5050/master/reserve
> HTTP/1.1 200 OK
> Date: Fri, 26 Feb 2016 19:59:01 GMT
> Content-Length: 0
> 
> # curl http://ipdc02-kvm-guest2:5050/metrics/snapshot  | python -mjson.tool | 
> grep reserve
> "master/messages_reserve_resource": 1.0,
> "master/messages_unreserve_resource": 0.0,
> 
> 
> Thanks,
> 
> fan du
> 
>

Re: Review Request 45123: Updated docs for deletion of persistent volumes.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42470, 42471, 42472, 42473, 42474, 42503, 42683, 42504, 
42593, 42505, 42506, 42684, 45117, 45118, 45119, 45120, 45121, 45122, 45123]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 22, 2016, 6:24 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45123/
> ---
> 
> (Updated March 22, 2016, 6:24 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2408
> https://issues.apache.org/jira/browse/MESOS-2408
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated docs for deletion of persistent volumes.
> 
> 
> Diffs
> -
> 
>   docs/persistent-volume.md 4b9c59daf6fdcee4a102e19d6eb4df9b5eddfa54 
>   docs/reservation.md 55924adb94028702e15db7e191915157552981d0 
>   docs/upgrades.md 83b839f7fb996385baaa6ef1081dc1116cd6e338 
> 
> Diff: https://reviews.apache.org/r/45123/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 44706: Implemented isolate() method of "network/cni" isolator.

[Avinash sridharan]

> On March 21, 2016, 3:57 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 339
> > 
> >
> > This is just a thought. Maybe its better to use `await` over here, and 
> > use a lambda or `.onAny` on the await to parse the list of futures returned 
> > on await and clean up any checkpointed data for networks that we were not 
> > able to join due to plugin errors ?
> 
> Qian Zhang wrote:
> I think any clean up works should be handled in the `cleanup` method, 
> that's should be the design idea of isolator. Here we are doing all or 
> nothing, so if there is a call to a CNI plugin fails for any reasons, we will 
> return a `Failure` which should be eventually handled in 
> `Slave::executorLaunched`, and it will call `MesosContainerizer::destroy` 
> which will eventually call each isolator's `cleanup` method to do the cleanup.

Sounds reasonable.


> On March 21, 2016, 3:57 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 391
> > 
> >
> > Shouldn't we be cleaning up the check pointed data for this ifname if 
> > there is an error while launching the plugin for this ifname ?
> 
> Qian Zhang wrote:
> I think we should do the cleanup in the `cleanup` method.

Agreed.


> On March 21, 2016, 3:57 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 445
> > 
> >
> > Why the `CHECK_READY` here ? If the future is not READY it could be in 
> > error, which is fine we should just return an ERROR ?
> 
> Qian Zhang wrote:
> Because when `NetworkCniIsolatorProcess::__connect` is called, we expect 
> `output` must be ready otherwise `NetworkCniIsolatorProcess::__connect` 
> should not be called. I think it is a common way to use `then`, please take a 
> look at: 
> https://github.com/apache/mesos/blob/0.28.0/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp#L3173:L3186
>  as a reference.

The fact that a plugin might misbehave should not cause the agent to crash. I 
think this defensive check doesn't serve much purpose apart from causing a 
potential panic in the Agent.


> On March 21, 2016, 3:57 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 471
> > 
> >
> > Again why the CHECK ? The result might not have an IPv4 or an IPv6 
> > allocation due to an IPAM error in which case we should return a failure.
> 
> Qian Zhang wrote:
> I think the output of IPAM plugin has no IPv4 address and no IPv6 address 
> is an unexpected result, so that's why I use `CHECK` here, but I agree with 
> you returning a `Failure` is more appropriate, will update the patch 
> accordingly later.

Why is it an unexpected result? What happens if the IPAM is oversubscribed and 
runs out of addresses? That should definitely not cause the Agent to crash. We 
should not be able to launch containers, but the Agent should not be crashing. 
IPAM running out of addresses is not a terminal behavior, it will recover when 
containers are destroyed and IP addresses are freed.


> On March 21, 2016, 3:57 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines 328-336
> > 
> >
> > why do we need this dispatch ? The dispatch is to itself, so seems a 
> > bit odd. Can we invoke the `subprocess` for the plugin in _connect directly 
> > ?
> > 
> > Basically not sure why we need connect & _connect.
> 
> Qian Zhang wrote:
> The idea is similar with how `MesosContainerizer` call each isolator: 
> https://github.com/apache/mesos/blob/0.28.0/src/slave/containerizer/mesos/containerizer.cpp#L1171:L1181
> 
> I'd like to handle each call to a CNI plugin in a separate libprocess 
> dispatch event, so that's why I call `connect` via `dispatch`.

In the example you gave the `isolator` does a dispatch on an isolator process. 
So the `MesosContainerizer` effectively does a dispatch on a separate 
libprocess thread, which is the intended behavior. Here it seems a bit odd that 
dispatch is scheduling an event on itself. Functionally nothing wrong, but this 
is not idiomatic and we should avoid this.


- Avinash


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


On March 20, 2016, 4:27 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. 

Re: Review Request 44674: Supported image name specified with private registry prefix.

[Guangya Liu]

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




src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp (line 395)


Just a question: Where did set the schema was set to `https` for this case?


- Guangya Liu


On 三月 17, 2016, 4:57 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44674/
> ---
> 
> (Updated 三月 17, 2016, 4:57 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4939
> https://issues.apache.org/jira/browse/MESOS-4939
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported image name specified with private registry prefix.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6d637ed14f35feb554c8fcc63a7a7e046aaca574 
> 
> Diff: https://reviews.apache.org/r/44674/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-test.sh --gtest_filter="ProvisionerDockerRegistryPullerTest"
> 
> Tested with private registry localhost:80/ubuntu
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44706: Implemented isolate() method of "network/cni" isolator.

[Qian Zhang]

> On March 21, 2016, 3:08 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.hpp, lines 32-56
> > 
> >
> > Can we introduce paths.hpp|cpp under cni/ directory  for the canonical 
> > locations.
> > 
> > ```
> > constexpr char ROOT_DIR[] = "...";
> > 
> > string cni::paths::getNamespaceHandle(
> > const string& rootDir,
> > const ContainerID& containerId);
> > 
> > string cni::paths::getNetworkPath(
> > const string& rootDir,
> > const ContainerID& containerId,
> > const string& name);
> > 
> > string cni::paths::getIPv4Path(
> > const string& rootDir,
> > const ContainerID& containerId,
> > const string& name,
> > const string& ifname);
> > 
> > string cni::paths::getIPv6Path(
> > const string& rootDir,
> > const ContainerID& containerId,
> > const string& name,
> > const string& ifname);
> > ```

Thanks for the comments, I have two questions:
1. Do you mean we define rootDir as a const string like this: `constexpr char 
ROOT_DIR[] = "/var/run/mesos/isolators/network/cni/"`? If so, I'd like to know 
why we still need `rootDir` as the first parameter of those methods (e.g., 
`getNamespaceHandle`), I think we should be able to directly use that const 
string inside of those methods rather than passing it as a parameter, right?
2. Take `getNamespaceHandle` as an example, so it will be like:
string getNamespaceHandle(const ContainerID& containerId)
{
  return path::join(ROOT_DIR, containerId);
}
I think each of those `getXXX` method will be just a call to `path::join()`, so 
what is its advantage against directly calling `path::join()` in cni.cpp?


> On March 21, 2016, 3:08 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines 206-212
> > 
> >
> > I suggest we save a rootDir in the isolator process. We can easily 
> > switch to use a flag later. Also, we need to call 'realpath' here to make 
> > sure it's a realpath.
> > 
> > We also need to make sure ROOT_DIR is a self bind mounted directory 
> > (slave+shared) so that namespace bind mount does not leak into containers.

Do you mean we call `realpath()` to get the real path of the const string 
`ROOT_DIR` first and then call `mkdir` with the real path as its parameter to 
create the directory?

And can you please elaborate why the namespace bind mount can be leaked into 
containers if we do not make `ROOT_DIR` as a self bind mounted directory? I 
just want to know the rationale behind it :-)


- Qian


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


On March 21, 2016, 12:27 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44706/
> ---
> 
> (Updated March 21, 2016, 12:27 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented isolate() method of "network/cni" isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44706/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 44450: Rescind all outstanding offers to satisfy weights update.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [41681, 43863, 44450]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 22, 2016, 5:50 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44450/
> ---
> 
> (Updated March 22, 2016, 5:50 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-4881
> https://issues.apache.org/jira/browse/MESOS-4881
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rescind all outstanding offers to satisfy weights update.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 124d43931a5c8a00ee0aaa604feb1761795209f2 
>   src/master/weights_handler.cpp c9a1b0d9adbeb1e165999cdbb4f295b24f10b18f 
> 
> Diff: https://reviews.apache.org/r/44450/diff/
> 
> 
> Testing
> ---
> 
> Make && Make check.
> 
> Manual test steps:
> 
> - Start Mesos master:
> $ ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/tmp/mesos-master
> 
> - Start Mesos slave:
> $ ./bin/mesos-slave.sh --master=127.0.0.1:5050
> 
> - Register a framwork with `curl`
> $ curl -v http://127.0.0.1:5050/api/v1/scheduler -H "Content-type: 
> application/json" -X POST -d @subscribe.json
> $  cat subscribe.json
> {
>"type": "SUBSCRIBE",
> 
>"subscribe" : {
>   "framework_info": {
>   "user" :  "root",
>   "name" :  "comsumer c1 HTTP Framework",
> "role" :  "mesos",
> "principal":"wyq"
>   },
> 
>   "force" : true
>   }
> }
> 
> And this framework will receive an offer:
> {"subscribed":{"framework_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-"}},"type":"SUBSCRIBED"}20
> {"type":"HEARTBEAT"}680
> {"offers":{"offers":[{"agent_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-S0"},"framework_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-"},"hostname":"192.168.1.5","id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-O0"},"resources":[{"name":"cpus","role":"*","scalar":{"value":8.0},"type":"SCALAR"},{"name":"mem","role":"*","scalar":{"value":15360.0},"type":"SCALAR"},{"name":"disk","role":"*","scalar":{"value":470832.0},"type":"SCALAR"},{"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"role":"*","type":"RANGES"}],"url":{"address":{"hostname":"192.168.1.5","ip":"192.168.1.5","port":5051},"path":"\/slave(1)","scheme":"http"}}]},"type":"OFFERS"}20
> {"type":"HEARTBEAT"}20
> 
> - Update the weight of role `mesos`
> $ curl --data "[{\"weight\":1.8,\"role\":\"mesos\"}]" -X PUT 
> http://127.0.0.1:5050/weights
> 
> Receive an rescind offer, and a new offer received:
> {"rescind":{"offer_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-O0"}},"type":"RESCIND"}680
> {"offers":{"offers":[{"agent_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-S0"},"framework_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-"},"hostname":"192.168.1.5","id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-O1"},"resources":[{"name":"cpus","role":"*","scalar":{"value":8.0},"type":"SCALAR"},{"name":"mem","role":"*","scalar":{"value":15360.0},"type":"SCALAR"},{"name":"disk","role":"*","scalar":{"value":470832.0},"type":"SCALAR"},{"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"role":"*","type":"RANGES"}],"url":{"address":{"hostname":"192.168.1.5","ip":"192.168.1.5","port":5051},"path":"\/slave(1)","scheme":"http"}}]},"type":"OFFERS"}20
> {"type":"HEARTBEAT"}20
> 
> - Update the weight of role `mesos1`
> $ curl --data "[{\"weight\":2.8,\"role\":\"mesos1\"}]" -X PUT 
> http://127.0.0.1:5050/weights
> 
> - The outstanding offer will not be rescinded.
> 
> 
> (TODO) I will add couple of tests for this patch in another JIRA.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 44851: Renamed an allocator metric.

[Benjamin Bannier]

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

(Updated March 22, 2016, 2:42 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


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


Repository: mesos


Description
---

Since allocators can be replaced with a custom module instead use a
name clearly communicating that the metrics reported here are related
to the default (hierarchical) allocator.

For backwards compatibility we continue to support the old metrics
name.


Diffs (updated)
-

  docs/upgrades.md 83b839f7fb996385baaa6ef1081dc1116cd6e338 
  src/master/allocator/mesos/metrics.hpp 
d04e9a11c77b6fb2d522608e3085f81f8a6657ca 
  src/master/allocator/mesos/metrics.cpp 
46dd7f7b3d8d49c59b42e789fe5dcd49f1693ace 

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


Testing
---

check make check (OS X, clang-trunk) here and with later patches using other 
allocator metrics.


Thanks,

Benjamin Bannier

Re: Review Request 43561: Improve Ranges parsing to handle single values.

[Klaus Ma]

> On March 21, 2016, 10:02 p.m., Joris Van Remoortere wrote:
> > src/common/values.cpp, line 613
> > 
> >
> > @benm I wish we had support for iterating over these splicers eg:
> > `foreachtoken(temp, ",\n", [](const string& token) { ... });`
> > 
> > These helpers become rather heavy (at 3 nested) considering their 
> > utility.

@joris, I think `foreachtoken` is also nested loop. When doing the patches, 
there're three options to me: FSM, regex and nested loop; FSM & regex seems 
hard to maintain, so I used nested loop.


- Klaus


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


On March 22, 2016, 9:22 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43561/
> ---
> 
> (Updated March 22, 2016, 9:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4627
> https://issues.apache.org/jira/browse/MESOS-4627
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improve Ranges parsing to handle single values.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 9f3c0b9930bd3dd9d84d97119419825862f531c3 
>   src/tests/resources_tests.cpp 6b004d64bb25112b19fc5d98b5bca874c5329e8c 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/43561/diff/
> 
> 
> Testing
> ---
> 
> make
> make check GTEST_FILTER=~"*"
> ./src/mesos-test
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 43561: Improve Ranges parsing to handle single values.

[Klaus Ma]

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

(Updated March 22, 2016, 9:22 p.m.)


Review request for mesos, Alexander Rukletsov, Ben Mahler, and Joris Van 
Remoortere.


Changes
---

Address comments to avoid reference on tmp var.


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


Repository: mesos


Description
---

Improve Ranges parsing to handle single values.


Diffs (updated)
-

  src/common/values.cpp 9f3c0b9930bd3dd9d84d97119419825862f531c3 
  src/tests/resources_tests.cpp 6b004d64bb25112b19fc5d98b5bca874c5329e8c 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 

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


Testing
---

make
make check GTEST_FILTER=~"*"
./src/mesos-test


Thanks,

Klaus Ma

Re: Review Request 45134: Skip FetcherTest zip tests when `unzip` is uninstalled.

[Tomasz Janiszewski]

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

(Updated March 22, 2016, 1:10 p.m.)


Review request for mesos, Jie Yu and Neil Conway.


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


Repository: mesos


Description
---

Skip FetcherTest zip tests when `unzip` is uninstalled.


Diffs (updated)
-

  src/tests/environment.cpp ee1bbe6b4e3dda1e27b63d71a08ef0d1d254741a 
  src/tests/fetcher_tests.cpp 375e690ead4f954a66a6efc2f2daa7fb1f1a024d 

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


Testing
---


Thanks,

Tomasz Janiszewski

Re: Review Request 45134: Skip FetcherTest zip tests when `unzip` is uninstalled.

[haosdent huang]

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


Fix it, then Ship it!




Ship It!


src/tests/environment.cpp (line 253)


Suggest put after `RootFilter` as well.



src/tests/environment.cpp (line 697)


Could you put this after
```
  filters.push_back(Owned(new RootFilter()));
```
I think it should ordered alphabetically.


- haosdent huang


On March 22, 2016, 12:27 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45134/
> ---
> 
> (Updated March 22, 2016, 12:27 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Bugs: MESOS-4993
> https://issues.apache.org/jira/browse/MESOS-4993
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Skip FetcherTest zip tests when `unzip` is uninstalled.
> 
> 
> Diffs
> -
> 
>   src/tests/environment.cpp ee1bbe6b4e3dda1e27b63d71a08ef0d1d254741a 
>   src/tests/fetcher_tests.cpp 375e690ead4f954a66a6efc2f2daa7fb1f1a024d 
> 
> Diff: https://reviews.apache.org/r/45134/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 45014: Add /containers endpoint to return ResourceUsage.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45014]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 18, 2016, 5:27 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45014/
> ---
> 
> (Updated March 18, 2016, 5:27 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add /containers endpoint to return ResourceUsage.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 4eb1fafdfa72094511b0b2684a3c2705bd8c7c5e 
>   src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/slave_tests.cpp ea1d776077bf638885db8421194aa4427c772169 
> 
> Diff: https://reviews.apache.org/r/45014/diff/
> 
> 
> Testing
> ---
> 
> make check
> `curl agent_ip:port/containers` returns same json content as `curl 
> agent_ip:port/monitor/statistics.json`
> 
> This is a draft patch of adding /containers endpoint to agent 
> [MESOS-4891](https://issues.apache.org/jira/browse/MESOS-4891)
> 
> ContainerStatus will be added to response based on this patch.
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 43561: Improve Ranges parsing to handle single values.

[Klaus Ma]

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

(Updated March 22, 2016, 8:25 p.m.)


Review request for mesos, Alexander Rukletsov, Ben Mahler, and Joris Van 
Remoortere.


Changes
---

Address Joris' comments.


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


Repository: mesos


Description
---

Improve Ranges parsing to handle single values.


Diffs (updated)
-

  src/common/values.cpp 9f3c0b9930bd3dd9d84d97119419825862f531c3 
  src/tests/resources_tests.cpp 6b004d64bb25112b19fc5d98b5bca874c5329e8c 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 

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


Testing
---

make
make check GTEST_FILTER=~"*"
./src/mesos-test


Thanks,

Klaus Ma

Re: Review Request 45134: Skip FetcherTest zip tests when `unzip` is uninstalled.

[Tomasz Janiszewski]

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

(Updated March 22, 2016, 12:20 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Skip FetcherTest zip tests when `unzip` is uninstalled.


Diffs (updated)
-

  src/tests/environment.cpp ee1bbe6b4e3dda1e27b63d71a08ef0d1d254741a 
  src/tests/fetcher_tests.cpp 375e690ead4f954a66a6efc2f2daa7fb1f1a024d 

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


Testing
---


Thanks,

Tomasz Janiszewski

Review Request 45158: Cleaned up formatting in CHANGELOG.

[Alexander Rukletsov]

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

Review request for mesos and Joris Van Remoortere.


Repository: mesos


Description
---

See summary.


Diffs
-

  CHANGELOG eab924e8066875d7679f5d09af2da434aa554699 

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


Testing
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Re: Review Request 44376: Upgrade zookeeper to 3.4.8 to support Power LE platform.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [44376]

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

Error:
2016-03-22 11:20:54 URL:https://reviews.apache.org/r/44376/diff/raw/ 
[9419/9419] -> "44376.patch" [1]
error: missing binary patch data for '3rdparty/zookeeper-3.4.8.tar.gz'
error: binary patch does not apply to '3rdparty/zookeeper-3.4.5.tar.gz'
error: 3rdparty/zookeeper-3.4.5.tar.gz: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/12091/console

- Mesos ReviewBot


On March 22, 2016, 5:07 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44376/
> ---
> 
> (Updated March 22, 2016, 5:07 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Alex Clemmer, Kapil Arya, Neil Conway, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-4612
> https://issues.apache.org/jira/browse/MESOS-4612
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade zookeeper to 3.4.8 to support Power LE platform.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 49aa55741d76aa88c8fbb526f18908312bb0c717 
>   3rdparty/cmake/Versions.cmake 24490d399f20b31b6336e92d1bd5d9a7230f31f7 
>   3rdparty/versions.am c2dae2fb521b12344b93bf771dd5497ba8d446c3 
>   3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 
>   3rdparty/zookeeper-3.4.5.tar.gz 1a547fe17a6fad86012f855d3c4cc38fed4899fc 
>   3rdparty/zookeeper-3.4.8.patch PRE-CREATION 
>   src/examples/java/test-log.in 4c8547aaa115779ae7cec58edde01ab85feeb1b1 
>   src/python/native_common/ext_modules.py.in 
> c335bd83024bc07b6243dd59d775e7f29adc7520 
>   src/tests/zookeeper_test_server.cpp 
> 0dc041fef8973d35114b9f76a6a4002853884670 
> 
> Diff: https://reviews.apache.org/r/44376/diff/
> 
> 
> Testing
> ---
> 
> sudo make dist check
> 
> sudo ./src/mesos-tests --benchmark
> 
> sudo ./support/run-upgrade.py --prev=../mesos-0.27.0/build --next=./build
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 44656: Introduced `KillPolicy` protobuf.

[Alexander Rukletsov]

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

(Updated March 22, 2016, 10:30 a.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description (updated)
---

Describes a kill policy for a task. Currently does not express
different policies (e.g. hitting HTTP endpoints), only controls
how long to wait between graceful and forcible task kill.


Diffs
-

  include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
  include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 

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


Testing
---

The complete chain was tested. See https://reviews.apache.org/r/44662/.


Thanks,

Alexander Rukletsov

Re: Review Request 45039: Updated the comment for launching tasks and accepting offers.

[Alexander Rukletsov]

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

(Updated March 22, 2016, 10:28 a.m.)


Review request for mesos and Ben Mahler.


Summary (updated)
-

Updated the comment for launching tasks and accepting offers.


Repository: mesos


Description (updated)
---

If the task does not pass validation,
its resources are considered declined.


Diffs
-

  docs/app-framework-development-guide.md 
1d8bebde67f69fd414509b8861571137d3569b46 
  include/mesos/scheduler.hpp 14c7ff964aa7b94f439d16e605380661d2279d54 
  src/java/src/org/apache/mesos/SchedulerDriver.java 
bf866f5ebece2505eaa27bf39a1382cd1a2a069a 
  src/python/interface/src/mesos/interface/__init__.py 
232890daa6d222ae1c86906bbc484c8e635c4eb7 

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


Testing
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Re: Review Request 44993: Updated `TestContainerizer` to support default actions.

[Alexander Rukletsov]

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

(Updated March 22, 2016, 10:28 a.m.)


Review request for mesos and Ben Mahler.


Repository: mesos


Description (updated)
---

We use `ON_CALL` and `WillByDefault` here to specify the default
actions. This allows the tests to leverage the `DoDefault` action.
However, `ON_CALL` results in a "Uninteresting mock function call"
warning unless each test puts expectations in place. As a result,
we also use `EXPECT_CALL` and `WillRepeatedly` to get the best of
both worlds: the ability to use `DoDefault` and no warnings when
expectations are not explicit.


Diffs
-

  src/tests/containerizer.cpp c6772ce5908edaab6c3189a65e8446217d1c7c27 

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


Testing
---

On Mac OS 10.10.4:
`make check`
`GTEST_FILTER="SlaveTest.*" ./bin/mesos-tests.sh --gtest_repeat=100 
--gtest_break_on_failure`


Thanks,

Alexander Rukletsov

Re: Review Request 45040: Added a test for task's kill policy.

[Alexander Rukletsov]

> On March 19, 2016, 1:35 a.m., Ben Mahler wrote:
> > Thanks for the test! Although I wouldn't want to ship this unless you show 
> > some manual testing in the 'testing done' section, since this test doesn't 
> > exercise the functionality :)

I think since we pulled up half of the test into a separate test, we don't need 
this one any more. I'll discard the review request.


> On March 19, 2016, 1:35 a.m., Ben Mahler wrote:
> > src/tests/slave_tests.cpp, lines 3419-3424
> > 
> >
> > You won't need to bother with this once we pull out the validation 
> > test, right?

Correct.


> On March 19, 2016, 1:35 a.m., Ben Mahler wrote:
> > src/tests/slave_tests.cpp, lines 3442-3444
> > 
> >
> > You shouldn't need to do this once the validation test is pulled out, 
> > right?

Right.


> On March 19, 2016, 1:35 a.m., Ben Mahler wrote:
> > src/tests/slave_tests.cpp, line 3482
> > 
> >
> > Why the resume here?

I think we should resume the clock before exiting the test. I've heard plans to 
even add a check in our test harness that verified the clock is not paused.


- Alexander


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


On March 19, 2016, 10:40 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45040/
> ---
> 
> (Updated March 19, 2016, 10:40 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4909
> https://issues.apache.org/jira/browse/MESOS-4909
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp ea1d776077bf638885db8421194aa4427c772169 
> 
> Diff: https://reviews.apache.org/r/45040/diff/
> 
> 
> Testing
> ---
> 
> On Mac OS 10.10.4:
> `make check`
> `GTEST_FILTER="*TaskKillPolicy*" ./bin/mesos-tests.sh --gtest_repeat=100 
> --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 44378: Upgrade libev to 4.22 to support PowerPC LE platform.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [44378]

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

Error:
2016-03-22 10:20:35 URL:https://reviews.apache.org/r/44378/diff/raw/ 
[5793/5793] -> "44378.patch" [1]
error: missing binary patch data for 
'3rdparty/libprocess/3rdparty/libev-4.22.tar.gz'
error: binary patch does not apply to 
'3rdparty/libprocess/3rdparty/libev-4.22.tar.gz'
error: 3rdparty/libprocess/3rdparty/libev-4.22.tar.gz: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/12089/console

- Mesos ReviewBot


On March 22, 2016, 1:38 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44378/
> ---
> 
> (Updated March 22, 2016, 1:38 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Alex Clemmer, Kapil Arya, Neil Conway, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-4803
> https://issues.apache.org/jira/browse/MESOS-4803
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade libev to 4.22 to support PowerPC LE platform.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake 24490d399f20b31b6336e92d1bd5d9a7230f31f7 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 6b07aefc58a1daa235b35e83832e47d1878e2e94 
>   3rdparty/libprocess/3rdparty/libev-4.15.patch 
> bbd83e6928e6caba3bc5a9739823d60923cfaa2a 
>   3rdparty/libprocess/3rdparty/libev-4.15.tar.gz 
> 4c282b573aa9331fd16197ef286faf323b6515eb 
>   3rdparty/libprocess/3rdparty/libev-4.22.patch PRE-CREATION 
>   3rdparty/libprocess/3rdparty/libev-4.22.tar.gz PRE-CREATION 
>   3rdparty/libprocess/3rdparty/versions.am 
> 98195b8eb60b2673d610d8ab7ea31103f137debf 
>   LICENSE 66a99b8a84e614cc89a22df02e3c47d01e26cd39 
>   src/python/native_common/ext_modules.py.in 
> c335bd83024bc07b6243dd59d775e7f29adc7520 
> 
> Diff: https://reviews.apache.org/r/44378/diff/
> 
> 
> Testing
> ---
> 
> sudo make dist check
> 
> sudo ./src/mesos-tests --benchmark
> 
> sudo ./support/run-upgrade.py --prev=../mesos-0.27.0/build --next=./build
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 44512: Support to get weights info by /weights.

[Yongqiao Wang]

> On March 21, 2016, 9:03 a.m., Adam B wrote:
> > Good start, but you're missing a few things. Are these coming in subsequent 
> > patches?
> > - Authentication for GET requests
> > - Tests for GETs
> > - Documentation updates

I will add a separated patch for GET requet test.


> On March 21, 2016, 9:03 a.m., Adam B wrote:
> > src/master/http.cpp, line 1178
> > 
> >
> > Will you be adding authentication to GETs in a separate patch?

Currently, the GET request has supported the authentication:

```
$ curl -v -X GET http://localhost:5050/weights | python -mjson.tool
* Hostname was NOT found in DNS cache
  % Total% Received % Xferd  Average Speed   TimeTime Time  Current
 Dload  Upload   Total   SpentLeft  Speed
  0 00 00 0  0  0 --:--:-- --:--:-- --:--:-- 0* 
  Trying ::1...
* connect to ::1 port 5050 failed: Connection refused
*   Trying 127.0.0.1...
* Connected to localhost (127.0.0.1) port 5050 (#0)
> GET /weights HTTP/1.1
> User-Agent: curl/7.37.1
> Host: localhost:5050
> Accept: */*
>
< HTTP/1.1 401 Unauthorized
< Date: Tue, 22 Mar 2016 08:53:09 GMT
< WWW-Authenticate: Basic realm="mesos"
< Content-Length: 0
<
  0 00 00 0  0  0 --:--:-- --:--:-- --:--:-- 0
* Connection #0 to host localhost left intact
No JSON object could be decoded
```


- Yongqiao


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


On March 9, 2016, 6:36 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44512/
> ---
> 
> (Updated March 9, 2016, 6:36 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-4316
> https://issues.apache.org/jira/browse/MESOS-4316
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support to get weights info by /weights.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 3d22ec32655dca741169e1f0e382303e061c38b7 
>   src/master/http.cpp a3ad57a1c3f8a01aa609b28c12825670bb243387 
>   src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
>   src/master/weights_handler.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44512/diff/
> 
> 
> Testing
> ---
> 
> make && make check.
> 
> $ (./mesos-master.sh --ip=127.0.0.1 --work_dir=/tmp/mesos-master  
> --authenticate_http --credentials=/opt/credentials.json  >> 
> /tmp/mesos-master.log 2>&1 &)
> 
> $ curl --user framework1:secret_string1 http://localhost:5050/weights | 
> python -mjson.tool
> {}
> 
> $ curl --user framework1:secret_string1 --data 
> "[{\"weight\":1.8,\"role\":\"role1\"},{\"weight\":1.0,\"role\":\"role2\"},{\"weight\":3.4,\"role\":\"role3\"}]"
>  -X PUT http://127.0.0.1:5050/weights
> 
> $ curl --user framework1:secret_string1 http://localhost:5050/weights | 
> python -mjson.tool
> {
> "infos": [
> {
> "role": "role3",
> "weight": 3.4
> },
> {
> "role": "role2",
> "weight": 1.0
> },
> {
> "role": "role1",
> "weight": 1.8
> }
> ]
> }
> 
> Has updated all tests of /weights endpoint to check the update results with 
> /weights GET request.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 44707: Added validation for task's kill policy.

[Alexander Rukletsov]

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

(Updated March 22, 2016, 10:06 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

Updated testing section.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/master/validation.hpp 29dbdf1da540db4966ff9e86037badde06e69a4b 
  src/master/validation.cpp 820a9faee6fde09a98317854b181fe897167e2ca 

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


Testing (updated)
---

On Mac OS 10.10.4:
`make check`
`GTEST_FILTER="TaskValidationTest.KillPolicyGracePeriodIsNonNegative" 
./bin/mesos-tests.sh --gtest_repeat=100 --gtest_break_on_failure`


Thanks,

Alexander Rukletsov

Re: Review Request 44854: Added validation for executor's shutdown grace period.

[Alexander Rukletsov]

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

(Updated March 22, 2016, 10:05 a.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/master/validation.cpp 820a9faee6fde09a98317854b181fe897167e2ca 
  src/tests/master_validation_tests.cpp 
8d0070a1b8e8dcc7fe6360f8c6c6182ba9edef7d 

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


Testing (updated)
---

On Mac OS 10.10.4:
`make check`
`GTEST_FILTER="TaskValidationTest.ExecutorShutdownGracePeriodIsNonNegative" 
./bin/mesos-tests.sh --gtest_repeat=100 --gtest_break_on_failure`


Thanks,

Alexander Rukletsov

Re: Review Request 44512: Support to get weights info by /weights.

[Yongqiao Wang]

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

(Updated March 22, 2016, 10:02 a.m.)


Review request for mesos and Adam B.


Changes
---

Addressed comments of adam.


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


Repository: mesos


Description
---

Support to get weights info by /weights.


Diffs (updated)
-

  docs/weights.md a94251340d5193c9474b2e5e85328d62fe4edd8f 
  src/master/http.cpp bfea8fa811c24d37b2d7a8109728e1f40217e02f 
  src/master/master.hpp 124d43931a5c8a00ee0aaa604feb1761795209f2 
  src/master/weights_handler.cpp c9a1b0d9adbeb1e165999cdbb4f295b24f10b18f 

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


Testing (updated)
---

make && make check.

$ (./mesos-master.sh --ip=127.0.0.1 --work_dir=/tmp/mesos-master  
--authenticate_http --credentials=/opt/credentials.json  >> 
/tmp/mesos-master.log 2>&1 &)

$ curl --user framework1:secret_string1 http://localhost:5050/weights | python 
-mjson.tool
[]

$ curl --user framework1:secret_string1 --data 
"[{\"weight\":1.8,\"role\":\"role1\"},{\"weight\":1.0,\"role\":\"role2\"},{\"weight\":3.4,\"role\":\"role3\"}]"
 -X PUT http://127.0.0.1:5050/weights

$ curl --user framework1:secret_string1 http://localhost:5050/weights | python 
-mjson.tool
[
{
"role": "role3",
"weight": 3.4
},
{
"role": "role2",
"weight": 1.0
},
{
"role": "role1",
"weight": 1.8
}
]


Thanks,

Yongqiao Wang

Re: Review Request 44514: Implemented prepare() method of "network/cni" isolator.

[Qian Zhang]

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

(Updated March 22, 2016, 5:50 p.m.)


Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.


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


Repository: mesos


Description
---

Implemented prepare() method of "network/cni" isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Qian Zhang

Re: Review Request 44372: Upgrade http-parser to 2.6.1 to support PowerPC LE platform.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [44372]

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

Error:
2016-03-22 09:46:05 URL:https://reviews.apache.org/r/44372/diff/raw/ 
[10748/10748] -> "44372.patch" [1]
error: missing binary patch data for 
'3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz'
error: binary patch does not apply to 
'3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz'
error: 3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz: patch does not 
apply

Full log: https://builds.apache.org/job/mesos-reviewbot/12088/console

- Mesos ReviewBot


On March 22, 2016, 2:22 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44372/
> ---
> 
> (Updated March 22, 2016, 2:22 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Alex Clemmer, Kapil Arya, Neil Conway, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-4805
> https://issues.apache.org/jira/browse/MESOS-4805
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade http-parser to 2.6.1 to support PowerPC LE platform.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake 24490d399f20b31b6336e92d1bd5d9a7230f31f7 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 6b07aefc58a1daa235b35e83832e47d1878e2e94 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> ddf7e3d9bf76d4a03c33f02d52ec29812aef8509 
>   3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz PRE-CREATION 
>   3rdparty/libprocess/3rdparty/ry-http-parser-1c3624a.patch 
> f9fac12437a6bedc66353fda1ce9c0d7a383225a 
>   3rdparty/libprocess/3rdparty/ry-http-parser-1c3624a.tar.gz 
> b811b63ce0ad6d71d9d296fed76656c023c76fc5 
>   3rdparty/libprocess/3rdparty/versions.am 
> 98195b8eb60b2673d610d8ab7ea31103f137debf 
>   3rdparty/libprocess/Makefile.am ac8cc8d29baccf6e3a17367540ddd1f28585ef6d 
>   3rdparty/libprocess/src/decoder.hpp 
> a20b5ba8fc50d834573d253948645cc863f030dd 
>   LICENSE 66a99b8a84e614cc89a22df02e3c47d01e26cd39 
> 
> Diff: https://reviews.apache.org/r/44372/diff/
> 
> 
> Testing
> ---
> 
> sudo make dist check
> 
> sudo ./src/mesos-tests --benchmark
> 
> sudo ./support/run-upgrade.py --prev=../mesos-0.27.0/build --next=./build
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 44514: Implemented prepare() method of "network/cni" isolator.

[Qian Zhang]

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

(Updated March 22, 2016, 5:30 p.m.)


Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.


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


Repository: mesos


Description
---

Implemented prepare() method of "network/cni" isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Qian Zhang

Re: Review Request 43935: Allow setting role in mesos-execute.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43935]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 22, 2016, 2:57 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43935/
> ---
> 
> (Updated March 22, 2016, 2:57 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.
> 
> 
> Bugs: MESOS-4744
> https://issues.apache.org/jira/browse/MESOS-4744
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow setting role in mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/43935/diff/
> 
> 
> Testing
> ---
> 
> make & make check
> 
> start master.
> ./bin/mesos-master.sh --work_dir=/tmp/mesos
> 
> start slave.
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
> --resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"
> 
> running mesos-execute without specifying role succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --resources="cpus:1;mem:512"
> 
> running mesos-execute with role test succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test" --resources="cpus:2;mem:512"
> 
> running mesos-execute with role test1 fails.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test1" --resources="cpus:2;mem:512"
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 45151: Updated FrameworkInfo::Capability::Type enum for upgradability.

[Guangya Liu]

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



The bug should be MESOS-4997 here

- Guangya Liu


On 三月 22, 2016, 7:32 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45151/
> ---
> 
> (Updated 三月 22, 2016, 7:32 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Vinod Kone.
> 
> 
> Bugs: MESOS-4977
> https://issues.apache.org/jira/browse/MESOS-4977
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Per [MESOS-4977](https://issues.apache.org/jira/browse/MESOS-4977), having a 
> required enum field is problematic for supporting adding new enum values.
> 
> This updates FrameworkInfo::Capability to support the addition of new 
> capabilities while allowing old masters to deserialize the new messages.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b965f5a6b1aff28feb2ab088c30ea927b9247403 
>   include/mesos/v1/mesos.proto d2ab6ed187ec0d0e5dbac92607b612bb55b1a682 
> 
> Diff: https://reviews.apache.org/r/45151/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> manually validated the upgrade / backwards compatibility of enum fields
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 44514: Implemented prepare() method of "network/cni" isolator.

[Qian Zhang]

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

(Updated March 22, 2016, 5:06 p.m.)


Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.


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


Repository: mesos


Description
---

Implemented prepare() method of "network/cni" isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Qian Zhang

Re: Review Request 44382: Update leveldb-1.4.patch to support PowerPC LE platform.

[Zhiwei Chen]

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

(Updated March 22, 2016, 4:44 p.m.)


Review request for mesos, Ben Mahler, Alex Clemmer, Kapil Arya, Neil Conway, 
and Vinod Kone.


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


Repository: mesos


Description
---

Update leveldb-1.4.patch to support PowerPC LE platform.


Diffs (updated)
-

  3rdparty/leveldb-1.4.patch ad8c19b9caa856ff85978ba832d48df11b3a83f0 

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


Testing (updated)
---

sudo make dist check

sudo ./src/mesos-tests --benchmark

sudo ./support/run-upgrade.py --prev=../mesos-0.27.0/build --next=./build


Thanks,

Zhiwei Chen

Re: Review Request 45134: Skip FetcherTest zip tests when `unzip` is uninstalled.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45134]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 22, 2016, 12:14 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45134/
> ---
> 
> (Updated March 22, 2016, 12:14 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4993
> https://issues.apache.org/jira/browse/MESOS-4993
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Skip FetcherTest zip tests when `unzip` is uninstalled.
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp 375e690ead4f954a66a6efc2f2daa7fb1f1a024d 
> 
> Diff: https://reviews.apache.org/r/45134/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 45123: Updated docs for deletion of persistent volumes.

[Guangya Liu]

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



What about 
https://github.com/apache/mesos/blob/master/docs/multiple-disk.md#storage-management
 ? I think that we should also update here as well.

- Guangya Liu


On 三月 22, 2016, 6:24 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45123/
> ---
> 
> (Updated 三月 22, 2016, 6:24 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2408
> https://issues.apache.org/jira/browse/MESOS-2408
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated docs for deletion of persistent volumes.
> 
> 
> Diffs
> -
> 
>   docs/persistent-volume.md 4b9c59daf6fdcee4a102e19d6eb4df9b5eddfa54 
>   docs/reservation.md 55924adb94028702e15db7e191915157552981d0 
>   docs/upgrades.md 83b839f7fb996385baaa6ef1081dc1116cd6e338 
> 
> Diff: https://reviews.apache.org/r/45123/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 44656: Introduced `KillPolicy` protobuf.

[Alexander Rukletsov]

> On March 19, 2016, 12:38 a.m., Ben Mahler wrote:
> > include/mesos/mesos.proto, lines 366-370
> > 
> >
> > How about following similar language from the executor info change?
> > 
> > ```
> >   // The grace period specifies how long to wait before forcibly
> >   // killing the task. It is recommended to attempt to gracefully
> >   // kill the task (and send TASK_KILLING) to indicate that the
> >   // graceful kill is in progress. Once the grace period elapses,
> >   // if the task has not terminated, a forcible kill should occur.
> >   // The task should not assume that it will *always* be alloted
> >   // the full grace period. For example, the executor may be
> >   // shutdown more quickly by the agent, or failures / forcible
> >   // terminations may occur.
> > ```
> > 
> > However, you already have a NOTE above, so I'd rather not call it out 
> > again here.

I'd rather have this comment because the one for `KillPolicy` may change once 
we add more policies.


- Alexander


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


On March 18, 2016, 5:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44656/
> ---
> 
> (Updated March 18, 2016, 5:14 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4909
> https://issues.apache.org/jira/browse/MESOS-4909
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Describes a kill policy for a task. Currently does not express
> different policies (e.g. hitting HTTP endpoints). For now, this
> controls how long to wait between SIGTERM and SIGKILL.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
>   include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
> 
> Diff: https://reviews.apache.org/r/44656/diff/
> 
> 
> Testing
> ---
> 
> The complete chain was tested. See https://reviews.apache.org/r/44662/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 44622: Introduced a protobuf message "NetworkInfo".

[Qian Zhang]

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

(Updated March 22, 2016, 3:33 p.m.)


Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.


Changes
---

Rename NetworkResult to NetworkInfo.


Summary (updated)
-

Introduced a protobuf message "NetworkInfo".


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


Repository: mesos


Description (updated)
---

Introduced a protobuf message "NetworkInfo".


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/cni/spec.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/cni/spec.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/cni/spec.proto PRE-CREATION 

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


Testing
---

make check


Thanks,

Qian Zhang

Review Request 45151: Updated FrameworkInfo::Capability::Type enum for upgradability.

[Ben Mahler]

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

Review request for mesos, Joris Van Remoortere and Vinod Kone.


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


Repository: mesos


Description
---

Per [MESOS-4977](https://issues.apache.org/jira/browse/MESOS-4977), having a 
required enum field is problematic for supporting adding new enum values.

This updates FrameworkInfo::Capability to support the addition of new 
capabilities while allowing old masters to deserialize the new messages.


Diffs
-

  include/mesos/mesos.proto b965f5a6b1aff28feb2ab088c30ea927b9247403 
  include/mesos/v1/mesos.proto d2ab6ed187ec0d0e5dbac92607b612bb55b1a682 

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


Testing
---

make check

manually validated the upgrade / backwards compatibility of enum fields


Thanks,

Ben Mahler

Re: Review Request 44511: Add registry tests for /weights endpoint.

[Yongqiao Wang]

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

(Updated March 22, 2016, 7:22 a.m.)


Review request for mesos and Adam B.


Changes
---

Address comments of Adam.


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


Repository: mesos


Description
---

Add registry tests for /weights endpoint.


Diffs (updated)
-

  src/tests/dynamic_weights_tests.cpp ee0c4b1cc20e76a35a8e4e445f6827a1fc33e6c6 
  src/tests/registrar_tests.cpp c330af2a190282e159d9ab477cdc36a7881842cd 

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


Testing
---

Add three sub tests for /weights endpoint registry in this patch:

1. Tests whether the weights replicated log is initialized with the `--weights` 
flag when bootstrapping the cluster.
2. Tests whether the weights replicated log can be updated with `/weights` 
endpoint.
3. Tests whether the `--weights` flag is ignored and use the registry value 
instead when Mesos master subsequently starts with `--weights` flag still 
specified.

./src/mesos-tests --gtest_filter=Strict/RegistrarTest.UpdateWeights/*
[==] Running 2 tests from 1 test case.
[--] Global test environment set-up.
[--] 2 tests from Strict/RegistrarTest
[ RUN  ] Strict/RegistrarTest.UpdateWeights/0
[   OK ] Strict/RegistrarTest.UpdateWeights/0 (103 ms)
[ RUN  ] Strict/RegistrarTest.UpdateWeights/1
[   OK ] Strict/RegistrarTest.UpdateWeights/1 (67 ms)
[--] 2 tests from Strict/RegistrarTest (171 ms total)

[--] Global test environment tear-down
[==] 2 tests from 1 test case ran. (178 ms total)
[  PASSED  ] 2 tests.


make && make check.


Thanks,

Yongqiao Wang

Re: Review Request 45096: Introduce 'minus' operator for class Counter.

[Benjamin Bannier]

Hi Ben,

thanks for the link. I had the feeling I saw this being discussed, but could 
not find the RR.

Cheers,

Benjamin

> On Mar 22, 2016, at 8:06 AM, Benjamin Mahler  wrote:
> 
> Hey guys, we intentionally removed the ability to decrement Counters, please 
> see the context here on another recent patch:
> https://reviews.apache.org/r/44473/
> 
> On Tue, Mar 22, 2016 at 12:00 AM, Benjamin Bannier 
>  wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45096/#review124745
> ---
> 
> 
> 
> 
> 3rdparty/libprocess/include/process/metrics/counter.hpp (line 57)
> 
> 
> Could you add a post-decrement operator as well? It might not be as 
> useful as pre-decrement, but as this class provides post-increment this its 
> absence seems surprising.
> 
> 
> 
> 3rdparty/libprocess/include/process/metrics/counter.hpp (line 78)
> 
> 
> Did you mean to use `fetch_sub` here?
> 
> Do you plan to add unit tests for this class?
> 
> 
> - Benjamin Bannier
> 
> 
> On March 22, 2016, 7:42 a.m., fan du wrote:
> >
> > ---
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/45096/
> > ---
> >
> > (Updated March 22, 2016, 7:42 a.m.)
> >
> >
> > Review request for mesos, Anand Mazumdar and Vinod Kone.
> >
> >
> > Bugs: MESOS-4981
> > https://issues.apache.org/jira/browse/MESOS-4981
> >
> >
> > Repository: mesos
> >
> >
> > Description
> > ---
> >
> > Introduce 'minus' operator for class Counter.
> >
> >
> > Diffs
> > -
> >
> >   3rdparty/libprocess/include/process/metrics/counter.hpp 
> > a13cc7e18c8b23eae83c326d63874d9d2aaedc0d
> >
> > Diff: https://reviews.apache.org/r/45096/diff/
> >
> >
> > Testing
> > ---
> >
> >
> > Thanks,
> >
> > fan du
> >
> >
> 
> 

Re: Review Request 45096: Introduce 'minus' operator for class Counter.

[Benjamin Mahler]

Hey guys, we intentionally removed the ability to decrement Counters,
please see the context here on another recent patch:
https://reviews.apache.org/r/44473/

On Tue, Mar 22, 2016 at 12:00 AM, Benjamin Bannier <
benjamin.bann...@mesosphere.io> wrote:

>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45096/#review124745
> ---
>
>
>
>
> 3rdparty/libprocess/include/process/metrics/counter.hpp (line 57)
> 
>
> Could you add a post-decrement operator as well? It might not be as
> useful as pre-decrement, but as this class provides post-increment this its
> absence seems surprising.
>
>
>
> 3rdparty/libprocess/include/process/metrics/counter.hpp (line 78)
> 
>
> Did you mean to use `fetch_sub` here?
>
> Do you plan to add unit tests for this class?
>
>
> - Benjamin Bannier
>
>
> On March 22, 2016, 7:42 a.m., fan du wrote:
> >
> > ---
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/45096/
> > ---
> >
> > (Updated March 22, 2016, 7:42 a.m.)
> >
> >
> > Review request for mesos, Anand Mazumdar and Vinod Kone.
> >
> >
> > Bugs: MESOS-4981
> > https://issues.apache.org/jira/browse/MESOS-4981
> >
> >
> > Repository: mesos
> >
> >
> > Description
> > ---
> >
> > Introduce 'minus' operator for class Counter.
> >
> >
> > Diffs
> > -
> >
> >   3rdparty/libprocess/include/process/metrics/counter.hpp
> a13cc7e18c8b23eae83c326d63874d9d2aaedc0d
> >
> > Diff: https://reviews.apache.org/r/45096/diff/
> >
> >
> > Testing
> > ---
> >
> >
> > Thanks,
> >
> > fan du
> >
> >
>
>

Re: Review Request 45096: Introduce 'minus' operator for class Counter.

[Benjamin Bannier]

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




3rdparty/libprocess/include/process/metrics/counter.hpp (line 57)


Could you add a post-decrement operator as well? It might not be as useful 
as pre-decrement, but as this class provides post-increment this its absence 
seems surprising.



3rdparty/libprocess/include/process/metrics/counter.hpp (line 78)


Did you mean to use `fetch_sub` here?

Do you plan to add unit tests for this class?


- Benjamin Bannier


On March 22, 2016, 7:42 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45096/
> ---
> 
> (Updated March 22, 2016, 7:42 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-4981
> https://issues.apache.org/jira/browse/MESOS-4981
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduce 'minus' operator for class Counter.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/counter.hpp 
> a13cc7e18c8b23eae83c326d63874d9d2aaedc0d 
> 
> Diff: https://reviews.apache.org/r/45096/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> fan du
> 
>