Re: Review Request 48601: Exposed metrics information via `process::metrics::snapshot`.

2016-06-16 Thread Vinod Kone 

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


Ship it!




Ship It!

- Vinod Kone


On June 16, 2016, 5:58 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48601/
> ---
> 
> (Updated June 16, 2016, 5:58 p.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta, Anand Mazumdar, zhou xing, Jay 
> Guo, Michael Park, and Vinod Kone.
> 
> 
> Bugs: MESOS-5484
> https://issues.apache.org/jira/browse/MESOS-5484
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed metrics information via `process::metrics::snapshot`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> 25e8fcaeb9c71eaa43096165745e79c8198c139a 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> be03406c58439b7ab7c1ba12c19dd9f30aff109e 
> 
> Diff: https://reviews.apache.org/r/48601/diff/
> 
> 
> Testing
> ---
> 
> ## Performance test:
> 
> I start a master and three agents in my test server. Use 
> 
> ```
> $ time for i in {1..200}; do curl 'http://localhost:5050/metrics/snapshot' -o 
> /dev/null 2>/dev/null; done
> ```
> 
> to collect the performance data.
> 
> Before apply this patch
> ```
> real1m39.684s
> user0m0.622s
> sys 0m0.776s
> ```
> 
> After apply this patch
> ```
> real1m39.674s
> user0m0.586s
> sys 0m0.817s
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48601: Exposed metrics information via `process::metrics::snapshot`.

2016-06-16 Thread Vinod Kone 


> On June 16, 2016, 12:59 a.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/metrics/metrics.cpp, line 262
> > 
> >
> > hmm. you are still copying all the metrics into a hashmap and returning 
> > it, incuring an extra copy, which is not the same as what we were doing 
> > before. is there a way to refactor this without incurring extra copies for 
> > the /metrics/snapshot handler?
> 
> Vinod Kone wrote:
> actually it's not probably that bad because we are already doing a copy 
> in `snapshot()`
> 
> haosdent huang wrote:
> Got it, let me post a performance comparision about `/metrics/snapshot` 
> if apply this patch.
> 
> haosdent huang wrote:
> @vinodkone, I describe my test about performance above. It looks like not 
> big different after apply this patch.

great. thanks for adding the benchmark results.


- Vinod


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


On June 16, 2016, 5:58 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48601/
> ---
> 
> (Updated June 16, 2016, 5:58 p.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta, Anand Mazumdar, zhou xing, Jay 
> Guo, Michael Park, and Vinod Kone.
> 
> 
> Bugs: MESOS-5484
> https://issues.apache.org/jira/browse/MESOS-5484
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed metrics information via `process::metrics::snapshot`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> 25e8fcaeb9c71eaa43096165745e79c8198c139a 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> be03406c58439b7ab7c1ba12c19dd9f30aff109e 
> 
> Diff: https://reviews.apache.org/r/48601/diff/
> 
> 
> Testing
> ---
> 
> ## Performance test:
> 
> I start a master and three agents in my test server. Use 
> 
> ```
> $ time for i in {1..200}; do curl 'http://localhost:5050/metrics/snapshot' -o 
> /dev/null 2>/dev/null; done
> ```
> 
> to collect the performance data.
> 
> Before apply this patch
> ```
> real1m39.684s
> user0m0.622s
> sys 0m0.776s
> ```
> 
> After apply this patch
> ```
> real1m39.674s
> user0m0.586s
> sys 0m0.817s
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48601: Exposed metrics information via `process::metrics::snapshot`.

2016-06-16 Thread haosdent huang 


> On June 16, 2016, 12:59 a.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/metrics/metrics.cpp, line 262
> > 
> >
> > hmm. you are still copying all the metrics into a hashmap and returning 
> > it, incuring an extra copy, which is not the same as what we were doing 
> > before. is there a way to refactor this without incurring extra copies for 
> > the /metrics/snapshot handler?
> 
> Vinod Kone wrote:
> actually it's not probably that bad because we are already doing a copy 
> in `snapshot()`
> 
> haosdent huang wrote:
> Got it, let me post a performance comparision about `/metrics/snapshot` 
> if apply this patch.

@vinodkone, I describe my test about performance above. It looks like not big 
different after apply this patch.


- haosdent


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


On June 16, 2016, 5:58 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48601/
> ---
> 
> (Updated June 16, 2016, 5:58 p.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta, Anand Mazumdar, zhou xing, Jay 
> Guo, Michael Park, and Vinod Kone.
> 
> 
> Bugs: MESOS-5484
> https://issues.apache.org/jira/browse/MESOS-5484
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed metrics information via `process::metrics::snapshot`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> 25e8fcaeb9c71eaa43096165745e79c8198c139a 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> be03406c58439b7ab7c1ba12c19dd9f30aff109e 
> 
> Diff: https://reviews.apache.org/r/48601/diff/
> 
> 
> Testing
> ---
> 
> ## Performance test:
> 
> I start a master and three agents in my test server. Use 
> 
> ```
> $ time for i in {1..200}; do curl 'http://localhost:5050/metrics/snapshot' -o 
> /dev/null 2>/dev/null; done
> ```
> 
> to collect the performance data.
> 
> Before apply this patch
> ```
> real1m39.684s
> user0m0.622s
> sys 0m0.776s
> ```
> 
> After apply this patch
> ```
> real1m39.674s
> user0m0.586s
> sys 0m0.817s
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48601: Exposed metrics information via `process::metrics::snapshot`.

2016-06-16 Thread haosdent huang 

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

(Updated June 16, 2016, 5:58 p.m.)


Review request for mesos, Abhishek Dasgupta, Anand Mazumdar, zhou xing, Jay 
Guo, Michael Park, and Vinod Kone.


Changes
---

Add performance test result.


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


Repository: mesos


Description
---

Exposed metrics information via `process::metrics::snapshot`.


Diffs
-

  3rdparty/libprocess/include/process/metrics/metrics.hpp 
25e8fcaeb9c71eaa43096165745e79c8198c139a 
  3rdparty/libprocess/src/metrics/metrics.cpp 
be03406c58439b7ab7c1ba12c19dd9f30aff109e 

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


Testing (updated)
---

## Performance test:

I start a master and three agents in my test server. Use 

```
$ time for i in {1..200}; do curl 'http://localhost:5050/metrics/snapshot' -o 
/dev/null 2>/dev/null; done
```

to collect the performance data.

Before apply this patch
```
real1m39.684s
user0m0.622s
sys 0m0.776s
```

After apply this patch
```
real1m39.674s
user0m0.586s
sys 0m0.817s
```


Thanks,

haosdent huang

Re: Review Request 48601: Exposed metrics information via `process::metrics::snapshot`.

2016-06-16 Thread haosdent huang 

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

(Updated June 16, 2016, 5:26 p.m.)


Review request for mesos, Abhishek Dasgupta, Anand Mazumdar, zhou xing, Jay 
Guo, Michael Park, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Exposed metrics information via `process::metrics::snapshot`.


Diffs (updated)
-

  3rdparty/libprocess/include/process/metrics/metrics.hpp 
25e8fcaeb9c71eaa43096165745e79c8198c139a 
  3rdparty/libprocess/src/metrics/metrics.cpp 
be03406c58439b7ab7c1ba12c19dd9f30aff109e 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48601: Exposed metrics information via `process::metrics::snapshot`.

2016-06-15 Thread haosdent huang 


> On June 16, 2016, 12:59 a.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/metrics/metrics.cpp, line 262
> > 
> >
> > hmm. you are still copying all the metrics into a hashmap and returning 
> > it, incuring an extra copy, which is not the same as what we were doing 
> > before. is there a way to refactor this without incurring extra copies for 
> > the /metrics/snapshot handler?
> 
> Vinod Kone wrote:
> actually it's not probably that bad because we are already doing a copy 
> in `snapshot()`

Got it, let me post a performance comparision about `/metrics/snapshot` if 
apply this patch.


- haosdent


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


On June 15, 2016, 4:27 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48601/
> ---
> 
> (Updated June 15, 2016, 4:27 p.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta, Anand Mazumdar, zhou xing, Jay 
> Guo, Michael Park, and Vinod Kone.
> 
> 
> Bugs: MESOS-5484
> https://issues.apache.org/jira/browse/MESOS-5484
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed metrics information via `process::metrics::snapshot`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> 25e8fcaeb9c71eaa43096165745e79c8198c139a 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> be03406c58439b7ab7c1ba12c19dd9f30aff109e 
> 
> Diff: https://reviews.apache.org/r/48601/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48601: Exposed metrics information via `process::metrics::snapshot`.

2016-06-15 Thread Vinod Kone 


> On June 16, 2016, 12:59 a.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/metrics/metrics.cpp, line 262
> > 
> >
> > hmm. you are still copying all the metrics into a hashmap and returning 
> > it, incuring an extra copy, which is not the same as what we were doing 
> > before. is there a way to refactor this without incurring extra copies for 
> > the /metrics/snapshot handler?

actually it's not probably that bad because we are already doing a copy in 
`snapshot()`


- Vinod


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


On June 15, 2016, 4:27 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48601/
> ---
> 
> (Updated June 15, 2016, 4:27 p.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta, Anand Mazumdar, zhou xing, Jay 
> Guo, Michael Park, and Vinod Kone.
> 
> 
> Bugs: MESOS-5484
> https://issues.apache.org/jira/browse/MESOS-5484
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed metrics information via `process::metrics::snapshot`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> 25e8fcaeb9c71eaa43096165745e79c8198c139a 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> be03406c58439b7ab7c1ba12c19dd9f30aff109e 
> 
> Diff: https://reviews.apache.org/r/48601/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48601: Exposed metrics information via `process::metrics::snapshot`.

2016-06-15 Thread Vinod Kone 

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




3rdparty/libprocess/src/metrics/metrics.cpp (line 252)


hmm. you are still copying all the metrics into a hashmap and returning it, 
incuring an extra copy, which is not the same as what we were doing before. is 
there a way to refactor this without incurring extra copies for the 
/metrics/snapshot handler?


- Vinod Kone


On June 15, 2016, 4:27 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48601/
> ---
> 
> (Updated June 15, 2016, 4:27 p.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta, Anand Mazumdar, zhou xing, Jay 
> Guo, Michael Park, and Vinod Kone.
> 
> 
> Bugs: MESOS-5484
> https://issues.apache.org/jira/browse/MESOS-5484
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed metrics information via `process::metrics::snapshot`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> 25e8fcaeb9c71eaa43096165745e79c8198c139a 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> be03406c58439b7ab7c1ba12c19dd9f30aff109e 
> 
> Diff: https://reviews.apache.org/r/48601/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48601: Exposed metrics information via `process::metrics::snapshot`.

2016-06-15 Thread haosdent huang 

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

(Updated June 15, 2016, 4:27 p.m.)


Review request for mesos, Abhishek Dasgupta, Anand Mazumdar, zhou xing, Jay 
Guo, Michael Park, and Vinod Kone.


Changes
---

Address @vinodkone's comment.


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


Repository: mesos


Description
---

Exposed metrics information via `process::metrics::snapshot`.


Diffs (updated)
-

  3rdparty/libprocess/include/process/metrics/metrics.hpp 
25e8fcaeb9c71eaa43096165745e79c8198c139a 
  3rdparty/libprocess/src/metrics/metrics.cpp 
be03406c58439b7ab7c1ba12c19dd9f30aff109e 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48601: Exposed metrics information via `process::metrics::snapshot`.

2016-06-15 Thread haosdent huang 


> On June 14, 2016, 10:59 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/metrics/metrics.cpp, lines 262-292
> > 
> >
> > This induces a performance regression to /metrics/snapshot REST 
> > endpoint!
> > 
> > See https://reviews.apache.org/r/48046/ for inspiration on how to 
> > refactor methods that use JSON::ObjectWriter.

Hi, @vinodkone Thank you very much to point out this. I take a look 
https://reviews.apache.org/r/48400/ and I think could return a hashmap here so 
that we don't need return `Future, 
hashmap 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48601/
> ---
> 
> (Updated June 15, 2016, 2:30 p.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta, Anand Mazumdar, zhou xing, Jay 
> Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5484
> https://issues.apache.org/jira/browse/MESOS-5484
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed metrics information via `process::metrics::snapshot`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> 25e8fcaeb9c71eaa43096165745e79c8198c139a 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> be03406c58439b7ab7c1ba12c19dd9f30aff109e 
> 
> Diff: https://reviews.apache.org/r/48601/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48601: Exposed metrics information via `process::metrics::snapshot`.

2016-06-15 Thread haosdent huang 

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

(Updated June 15, 2016, 2:30 p.m.)


Review request for mesos, Abhishek Dasgupta, Anand Mazumdar, zhou xing, Jay 
Guo, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Exposed metrics information via `process::metrics::snapshot`.


Diffs (updated)
-

  3rdparty/libprocess/include/process/metrics/metrics.hpp 
25e8fcaeb9c71eaa43096165745e79c8198c139a 
  3rdparty/libprocess/src/metrics/metrics.cpp 
be03406c58439b7ab7c1ba12c19dd9f30aff109e 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48601: Exposed metrics information via `process::metrics::snapshot`.

2016-06-14 Thread Vinod Kone 

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




3rdparty/libprocess/src/metrics/metrics.cpp (lines 251 - 279)


This induces a performance regression to /metrics/snapshot REST endpoint!

See https://reviews.apache.org/r/48046/ for inspiration on how to refactor 
methods that use JSON::ObjectWriter.


- Vinod Kone


On June 12, 2016, 6:03 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48601/
> ---
> 
> (Updated June 12, 2016, 6:03 p.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta, Anand Mazumdar, zhou xing, Jay 
> Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5484
> https://issues.apache.org/jira/browse/MESOS-5484
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed metrics information via `process::metrics::snapshot`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> 25e8fcaeb9c71eaa43096165745e79c8198c139a 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> be03406c58439b7ab7c1ba12c19dd9f30aff109e 
> 
> Diff: https://reviews.apache.org/r/48601/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48601: Exposed metrics information via `process::metrics::snapshot`.

2016-06-12 Thread haosdent huang 

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

(Updated June 12, 2016, 6:03 p.m.)


Review request for mesos, Abhishek Dasgupta, Anand Mazumdar, zhou xing, Jay 
Guo, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Exposed metrics information via `process::metrics::snapshot`.


Diffs (updated)
-

  3rdparty/libprocess/include/process/metrics/metrics.hpp 
25e8fcaeb9c71eaa43096165745e79c8198c139a 
  3rdparty/libprocess/src/metrics/metrics.cpp 
be03406c58439b7ab7c1ba12c19dd9f30aff109e 

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


Testing
---


Thanks,

haosdent huang

Review Request 48601: Exposed metrics information via `process::metrics::snapshot`.

2016-06-12 Thread haosdent huang 

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

Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.


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


Repository: mesos


Description
---

Exposed metrics information via `process::metrics::snapshot`.


Diffs
-

  3rdparty/libprocess/include/process/metrics/metrics.hpp 
25e8fcaeb9c71eaa43096165745e79c8198c139a 
  3rdparty/libprocess/src/metrics/metrics.cpp 
be03406c58439b7ab7c1ba12c19dd9f30aff109e 

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


Testing
---


Thanks,

haosdent huang