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

2017-03-01 Thread Joris Van Remoortere

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



Closing this review due to inactivity. Please see our 
[guidelines](https://github.com/apache/mesos/blob/master/docs/reopening-reviews.md)
 for reopening reviews.

- Joris Van Remoortere


On March 24, 2016, 1:54 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44255/
> ---
> 
> (Updated March 24, 2016, 1:54 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/5/
> 
> 
> 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 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

2016-10-03 Thread Vinod Kone


> On April 27, 2016, 9:17 p.m., Benjamin Mahler wrote:
> > The `messages_*` metrics were originally added to track the number of each 
> > message type received by the master in the old libprocess message-passing 
> > API:
> > https://github.com/apache/mesos/blob/master/src/messages/messages.proto
> > 
> > However, we then introduced the Event/Call based API for 
> > schedulers/executors:
> > https://github.com/apache/mesos/blob/master/include/mesos/scheduler/scheduler.proto
> > 
> > Since these metrics are related to Accept-type Calls rather than the old 
> > message passing style, it seems we should add Call-based metrics to 
> > incorporate these:
> > 
> > ```
> > master/scheduler_calls: 1024
> > master/scheduler_calls/type/accept: 512
> > master/scheduler_calls/type/accept/operations/launch: 256
> > master/scheduler_calls/type/accept/operations/reserve: 64
> > master/scheduler_calls/type/accept/operations/unreserve: 64
> > master/scheduler_calls/type/accept/operations/create: 64
> > master/scheduler_calls/type/accept/operations/destroy: 64
> > master/scheduler_calls/type/decline: 512
> > master/scheduler_calls/type/kill: 512
> > ...
> > ```
> > 
> > Also, this lets us distinguish between schedulers making calls and 
> > operators hitting the endpoints (the current patch treats these the same). 
> > We should get some feedback from Vinod Kone and Anand Mazumdar since 
> > they've been working on the HTTP API.

This suggestion sounds good to me.


- Vinod


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


On March 24, 2016, 1:54 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44255/
> ---
> 
> (Updated March 24, 2016, 1:54 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 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

2016-04-27 Thread Ben Mahler

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



The `messages_*` metrics were originally added to track the number of each 
message type received by the master in the old libprocess message-passing API:
https://github.com/apache/mesos/blob/master/src/messages/messages.proto

However, we then introduced the Event/Call based API for schedulers/executors:
https://github.com/apache/mesos/blob/master/include/mesos/scheduler/scheduler.proto

Since these metrics are related to Accept-type Calls rather than the old 
message passing style, it seems we should add Call-based metrics to incorporate 
these:

```
master/scheduler_calls: 1024
master/scheduler_calls/type/accept: 512
master/scheduler_calls/type/accept/operations/launch: 256
master/scheduler_calls/type/accept/operations/reserve: 64
master/scheduler_calls/type/accept/operations/unreserve: 64
master/scheduler_calls/type/accept/operations/create: 64
master/scheduler_calls/type/accept/operations/destroy: 64
master/scheduler_calls/type/decline: 512
master/scheduler_calls/type/kill: 512
...
```

Also, this lets us distinguish between schedulers making calls and operators 
hitting the endpoints (the current patch treats these the same). We should get 
some feedback from Vinod Kone and Anand Mazumdar since they've been working on 
the HTTP API.

- Ben Mahler


On March 24, 2016, 1:54 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44255/
> ---
> 
> (Updated March 24, 2016, 1:54 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 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

2016-03-24 Thread Mesos ReviewBot

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



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 24, 2016, 1:54 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44255/
> ---
> 
> (Updated March 24, 2016, 1:54 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 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

2016-03-23 Thread fan du

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

(Updated 三月 24, 2016, 1:54 a.m.)


Review request for mesos, Greg Mann, Guangya Liu, haosdent huang, and Jie Yu.


Changes
---

Fix coding sytle by adding blank line.


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


Repository: mesos


Description
---

Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.


Diffs (updated)
-

  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 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

2016-03-22 Thread 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 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

2016-03-22 Thread 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 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

2016-03-13 Thread fan du

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

(Updated 三月 14, 2016, 5:50 a.m.)


Review request for mesos, Greg Mann, Guangya Liu, haosdent huang, and Jie Yu.


Changes
---

Makethe metrics counting for both NG cases(sanity check and validation check) 
and OK cases


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


Repository: mesos


Description
---

Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.


Diffs (updated)
-

  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 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

2016-03-13 Thread fan du


> On 三月 12, 2016, 2:11 a.m., Greg Mann wrote:
> > src/master/http.cpp, line 787
> > 
> >
> > This increment statement occurs after some invalidation logic, and 
> > directly before the authorization call; is this precisely where we want to 
> > be incrementing? We end up not counting invalid operations, but counting 
> > unauthorized operations.
> > 
> > Looking through the other metrics code in master.cpp, it seems we 
> > aren't entirely consistent with respect to when we increment the 
> > `messages_XXX` metrics. We increment them both before and after operation 
> > validation, for example.
> > 
> > In master.cpp, there are some existing `messages_XXX` metrics in the 
> > `accept()` function which get incremented together; those occur before 
> > validation and before authorization. To be consistent, you should probably 
> > do the same here. Currently, the metric gets incremented after `validate()` 
> > is called.
> > 
> > We should create a separate JIRA for making all of the messages metrics 
> > consistent with respect to when they get incremented.

I think the rule is clear to do the counting at the function entry call to 
include all other NG cases(sanity check and validation) as well.
Thanks for the point, and pls review the updated RR.

I will create another JIRA to track other metrics issue, and discuss with you 
later.


- fan


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


On 三月 10, 2016, 2:42 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44255/
> ---
> 
> (Updated 三月 10, 2016, 2:42 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 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

2016-03-11 Thread Greg Mann


On March 12, 2016, 2:11 a.m., fan du wrote:
> > In a separate patch, could you add documentation for these metrics to 
> > docs/monitoring.md?

Whoops, sorry! I see that you already have the documentation :-)


- Greg


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


On March 10, 2016, 2:42 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44255/
> ---
> 
> (Updated March 10, 2016, 2:42 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 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

2016-03-11 Thread Greg Mann

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



Thanks for the edits! A few comments below.


src/master/master.cpp (line 3027)


You could move your incrementing statements here to be more consistent with 
the existing code.



src/master/master.cpp (line 3214)


Remove the space after ->



src/master/http.cpp (line 787)


This increment statement occurs after some invalidation logic, and directly 
before the authorization call; is this precisely where we want to be 
incrementing? We end up not counting invalid operations, but counting 
unauthorized operations.

Looking through the other metrics code in master.cpp, it seems we aren't 
entirely consistent with respect to when we increment the `messages_XXX` 
metrics. We increment them both before and after operation validation, for 
example.

In master.cpp, there are some existing `messages_XXX` metrics in the 
`accept()` function which get incremented together; those occur before 
validation and before authorization. To be consistent, you should probably do 
the same here. Currently, the metric gets incremented after `validate()` is 
called.

We should create a separate JIRA for making all of the messages metrics 
consistent with respect to when they get incremented.


In a separate patch, could you add documentation for these metrics to 
docs/monitoring.md?

- Greg Mann


On March 10, 2016, 2:42 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44255/
> ---
> 
> (Updated March 10, 2016, 2:42 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 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

2016-03-09 Thread Mesos ReviewBot

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



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 10, 2016, 2:42 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44255/
> ---
> 
> (Updated March 10, 2016, 2:42 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 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

2016-03-09 Thread fan du


> On 三月 7, 2016, 10:43 p.m., Jie Yu wrote:
> > src/master/master.cpp, line 2846
> > 
> >
> > I looked weird to me that we increase the metrics for reserve resources 
> > in 'authorizeXXX' function. Can you do that in the callers of authorizeXXX 
> > function instead?
> 
> fan du wrote:
> Thanks for the reviewing :)
> It intended to reduce code duplication, do the statistics counting here 
> in the common path from both http endpoint and normal framework RECEIVE 
> message. If you insist, I'm willing to do it seprately.
> 
> Greg Mann wrote:
> I think Jie's reasoning is that semantically, it is strange for the 
> `authorizeXXX` function to have the side-effect of incrementing the metrics. 
> I missed this in my reviews, but on further consideration this seems like a 
> case where duplicating code is worth it in order to maintain the proper 
> separation of concerns in the `authorizeXXX` functions.

No problem! I've updated the patch as you and Jieyu suggested :)


- fan


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


On 三月 10, 2016, 2:42 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44255/
> ---
> 
> (Updated 三月 10, 2016, 2:42 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 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

2016-03-09 Thread fan du

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

(Updated 三月 10, 2016, 2:42 a.m.)


Review request for mesos, Greg Mann, Guangya Liu, haosdent huang, and Jie Yu.


Changes
---

Move the couting out of common code path to http endpoint and master accept 
call separately to reflect its logic.


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


Repository: mesos


Description
---

Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.


Diffs (updated)
-

  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 (updated)
---

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 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

2016-03-09 Thread Greg Mann


> On March 7, 2016, 10:43 p.m., Jie Yu wrote:
> > src/master/master.cpp, line 2846
> > 
> >
> > I looked weird to me that we increase the metrics for reserve resources 
> > in 'authorizeXXX' function. Can you do that in the callers of authorizeXXX 
> > function instead?
> 
> fan du wrote:
> Thanks for the reviewing :)
> It intended to reduce code duplication, do the statistics counting here 
> in the common path from both http endpoint and normal framework RECEIVE 
> message. If you insist, I'm willing to do it seprately.

I think Jie's reasoning is that semantically, it is strange for the 
`authorizeXXX` function to have the side-effect of incrementing the metrics. I 
missed this in my reviews, but on further consideration this seems like a case 
where duplicating code is worth it in order to maintain the proper separation 
of concerns in the `authorizeXXX` functions.


- Greg


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


On March 3, 2016, 5:40 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44255/
> ---
> 
> (Updated March 3, 2016, 5:40 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/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:
> 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 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

2016-03-08 Thread fan du


> On March 7, 2016, 10:43 p.m., Jie Yu wrote:
> > src/master/master.cpp, line 2846
> > 
> >
> > I looked weird to me that we increase the metrics for reserve resources 
> > in 'authorizeXXX' function. Can you do that in the callers of authorizeXXX 
> > function instead?

Thanks for the reviewing :)
It intended to reduce code duplication, do the statistics counting here in the 
common path from both http endpoint and normal framework RECEIVE message. If 
you insist, I'm willing to do it seprately.


- fan


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


On March 3, 2016, 5:40 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44255/
> ---
> 
> (Updated March 3, 2016, 5:40 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/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:
> 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 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

2016-03-07 Thread Jie Yu

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




src/master/master.cpp (line 2846)


I looked weird to me that we increase the metrics for reserve resources in 
'authorizeXXX' function. Can you do that in the callers of authorizeXXX 
function instead?


- Jie Yu


On March 3, 2016, 5:40 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44255/
> ---
> 
> (Updated March 3, 2016, 5:40 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/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:
> 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 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

2016-03-03 Thread Greg Mann

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


Ship it!




Ship It!

- Greg Mann


On March 3, 2016, 5:40 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44255/
> ---
> 
> (Updated March 3, 2016, 5:40 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/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:
> 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 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

2016-03-02 Thread fan du


> On 三月 2, 2016, 7:40 p.m., Greg Mann wrote:
> > Thanks Fan! I'm having a look at this now. Regarding the problems with 
> > review board not wanting to update your previous RR, it's probably because 
> > it didn't find the review URL in the commit message. In order to determine 
> > which RR it should update, the script looks for a string like "Review: 
> > https://reviews.apache.org/r/44255/; in the commit message of the commit 
> > you're pushing to review board. If you make sure that string (with the 
> > correct URL) is in your commit message locally, review board should be able 
> > to find your previous review and update it.

yes, it seems I recreated the commit message when rebasing without adding 
"Review: https://reviews.apache.org/r/44255;.
squash the commits together should also works ok :) Thanks for your kindness.


- fan


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


On 三月 3, 2016, 5:40 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44255/
> ---
> 
> (Updated 三月 3, 2016, 5:40 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/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:
> 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 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

2016-03-02 Thread fan du

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

(Updated 三月 3, 2016, 5:40 a.m.)


Review request for mesos, Greg Mann, Guangya Liu, haosdent huang, and Jie Yu.


Changes
---

Incorperated comments from Greg Mann
1. Fix few typos
2. Drop signed-off-by


Summary (updated)
-

Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.


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


Repository: mesos


Description (updated)
---

Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.


Diffs (updated)
-

  docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
  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:
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 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation

2016-03-02 Thread Greg Mann

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



Looks good! A couple small comments.


docs/monitoring.md (line 605)


s/distroy/destroy/



docs/monitoring.md (line 607)


s/distroy/destroy/



src/master/master.cpp (line 2942)


s/distroy/destroy/



src/master/metrics.hpp (line 131)


It's a bit more consistent with the other existing metrics here to make the 
names of these plural - i.e., `messages_reserve_resources`, 
`messages_unreserve_resources`, etc.



src/master/metrics.hpp (line 134)


s/distroy/destroy/, here and below


- Greg Mann


On March 2, 2016, 5:18 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44255/
> ---
> 
> (Updated March 2, 2016, 5:18 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
> ---
> 
> Signed-off-by: Fan Du 
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
>   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:
> 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 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation

2016-03-02 Thread Greg Mann

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



Thanks Fan! I'm having a look at this now. Regarding the problems with review 
board not wanting to update your previous RR, it's probably because it didn't 
find the review URL in the commit message. In order to determine which RR it 
should update, the script looks for a string like "Review: 
https://reviews.apache.org/r/44255/; in the commit message of the commit you're 
pushing to review board. If you make sure that string (with the correct URL) is 
in your commit message locally, review board should be able to find your 
previous review and update it.

- Greg Mann


On March 2, 2016, 5:18 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44255/
> ---
> 
> (Updated March 2, 2016, 5:18 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
> ---
> 
> Signed-off-by: Fan Du 
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
>   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:
> 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 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation

2016-03-02 Thread Mesos ReviewBot

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



Bad patch!

Reviews applied: [44255]

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

Error:
2016-03-02 11:13:00 URL:https://reviews.apache.org/r/44255/diff/raw/ 
[6120/6120] -> "44255.patch" [1]
Total errors found: 0
Checking 4 files
Error: Commit message summary (the first line) must end in a period.

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

- Mesos ReviewBot


On March 2, 2016, 5:18 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44255/
> ---
> 
> (Updated March 2, 2016, 5:18 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
> ---
> 
> Signed-off-by: Fan Du 
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
>   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:
> 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 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation

2016-03-01 Thread fan du

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

(Updated 三月 2, 2016, 5:18 a.m.)


Review request for mesos, Greg Mann, Guangya Liu, haosdent huang, and Jie Yu.


Changes
---

add tiny changelog


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


Repository: mesos


Description
---

Signed-off-by: Fan Du 


Diffs
-

  docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
  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 (updated)
---

ChangLog:
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



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

2016-03-01 Thread fan du

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

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
---

Signed-off-by: Fan Du 


Diffs (updated)
-

  docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
  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 (updated)
---

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