mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chun-Hung Hsiao <chhs...@apache.org>
Subject Re: Review Request 65665: Added operation state metrics in SLRP.
Date Thu, 12 Apr 2018 05:11:13 GMT


> On March 21, 2018, 1:12 p.m., Jan Schlicht wrote:
> > src/resource_provider/storage/provider.cpp
> > Lines 503 (patched)
> > <https://reviews.apache.org/r/65665/diff/3/?file=1982876#file1982876line503>
> >
> >     As this is only used as a helper in the constructor of `Metrics`, how about
introducing a small helper function instead of leaking this implementation detail?

I'll revert to the original style and pass the prefix to the `Metrics` constructor, which
follows the pattern we used in `src/log/metrics.cpp`.


> On March 21, 2018, 1:12 p.m., Jan Schlicht wrote:
> > src/resource_provider/storage/provider.cpp
> > Lines 2713 (patched)
> > <https://reviews.apache.org/r/65665/diff/3/?file=1982876#file1982876line2717>
> >
> >     `statusUpdateManager.update` below might still fail to drop the message. In
that case we would count the operation as dropped in metrics when we increment here. Increment
in an `onReady` handler instead.

If the `update()` call fails, the SLRP will immediately die anyway, so at least there will
be no double counting. But your suggestion makes sense so I'll do the change, and also change
where we increment the `operations_finished` or `operations_failed` metrics.


- Chun-Hung


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


On March 20, 2018, 3:24 a.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65665/
> -----------------------------------------------------------
> 
> (Updated March 20, 2018, 3:24 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Greg Mann, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8383
>     https://issues.apache.org/jira/browse/MESOS-8383
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds `operations_pending`, `operations_finished`,
> `operations_failed`, `operations_error` (currently unused), and
> `operations_dropped` metrics to count the occurances of these operation
> states.
> 
> Additionally, An error log in `_applyOperation()` is removed because the
> error is already logged at the call site.
> 
> 
> Diffs
> -----
> 
>   src/resource_provider/storage/provider.cpp bb19ed4b6b1b8f5f6b327461a737517497c8c38e

> 
> 
> Diff: https://reviews.apache.org/r/65665/diff/3/
> 
> 
> Testing
> -------
> 
> sudo make check
> A unit test is added in the next patch in chain.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message