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

Reply via email to