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



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbStatisticMetrics.scala
 (line 25)
<https://reviews.apache.org/r/39422/#comment160937>

    storeName is not used anywhere in RocksDbStatisticMetrics. I think the 
store name is prefixed to all metrics defined using KeyValueStoreMetrics.



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbStatisticMetrics.scala
 (line 97)
<https://reviews.apache.org/r/39422/#comment160936>

    Please add documentation for metrics as a part of the code or in the 
configuration.md page. If it is redundant documentation, then providing a link 
to the RocksDB page with the necessary documentation will be sufficient.



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbStatisticMetrics.scala
 (line 210)
<https://reviews.apache.org/r/39422/#comment160935>

    Can we filter this list of metrics to what is relevant in Samza? For 
example, we don't use (at least not yet) the multiget rocksdb api in Samza and 
we have intentionally turned off WAL in RocksDB as it is redundant.  
    
    The reason for my concern is that often, the metrics reporting system is 
sensitive to the number of metrics a job emits. So, it might be better to keep 
this list to only what is necessary.
    
    Seems like most of these metrics are only useful for someone trying to 
benchmark the application's local state performance. Can we make turning on 
these metrics optional? We can perhaps keep the basics such as number of keys 
read/written as a default and the rest, optional. What do you think?


- Navina Ramesh


On Oct. 18, 2015, 6:05 p.m., Gustavo Anatoly F. V. Solís wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39422/
> -----------------------------------------------------------
> 
> (Updated Oct. 18, 2015, 6:05 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> RocksDb expose statistic - Increased numberOfOperationsAdded
> 
> 
> Diffs
> -----
> 
>   
> samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
>  a423f7bd6c43461e051b5fd1f880dd01db785991 
>   
> samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbStatisticMetrics.scala
>  PRE-CREATION 
>   
> samza-kv-rocksdb/src/test/scala/org/apache/samza/storage/kv/TestRocksDbKeyValueStore.scala
>  a428a16bc1e9ab4980a6f17db4fd810057d31136 
> 
> Diff: https://reviews.apache.org/r/39422/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Gustavo Anatoly F. V. Solís
> 
>

Reply via email to