Re: Review Request 35933: SAMZA-449 Expose RocksDB statistic
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35933/ --- (Updated Julho 3, 2015, 11:05 p.m.) Review request for samza. Repository: samza Description (updated) --- RocksDb expose statistic Diffs (updated) - 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/35933/diff/ Testing --- Thanks, Gustavo Anatoly F. V. Solís
Re: Review Request 35933: SAMZA-449 Expose RocksDB statistic
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35933/#review90373
---
samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
(line 133)
https://reviews.apache.org/r/35933/#comment143410
we can remove the KeyValueStoreMetrics here, right? change to new
RocksDbStatisticMetrics(storeName, options, metrics)
samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
(line 245)
https://reviews.apache.org/r/35933/#comment143411
the [[]] is not very common. And I do not see anywere in Samza we are
using. It's better to remove them, or use {@link} if you want to link to the
class.
samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbStatisticMetrics.scala
(line 213)
https://reviews.apache.org/r/35933/#comment143409
is any code is using those methods (line 213-219)? If not, it's safe to
remove them. Also, users can directly call the variable name to get the value,
if they want.
- Yan Fang
On July 3, 2015, 11: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/35933/
---
(Updated July 3, 2015, 11:05 p.m.)
Review request for samza.
Repository: samza
Description
---
RocksDb expose statistic
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/35933/diff/
Testing
---
Thanks,
Gustavo Anatoly F. V. Solís
Re: Review Request 35933: SAMZA-449 Expose RocksDB statistic
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35933/#review89902 --- samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala (line 150) https://reviews.apache.org/r/35933/#comment142781 it should accept the metrics (from line 143) as the third parameter. samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala (line 243) https://reviews.apache.org/r/35933/#comment142780 add some doc here, saying something like, calling this method will expose RocksDB statistic to a metric, etc. samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbStatistic.scala (line 25) https://reviews.apache.org/r/35933/#comment142776 It's better to use the name RocksDbStatisticMetrics to indicate it is a metrics. samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbStatistic.scala (line 40) https://reviews.apache.org/r/35933/#comment142777 can we use the lower case for the all names of metrics? It's better to keep the name convension consistent in the Samza. - Yan Fang On June 26, 2015, 5:56 p.m., Gustavo Anatoly F. V. Solís wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35933/ --- (Updated June 26, 2015, 5:56 p.m.) Review request for samza. Repository: samza Description --- RocksDB statistic 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/RocksDbStatistic.scala PRE-CREATION samza-kv-rocksdb/src/test/scala/org/apache/samza/storage/kv/TestRocksDbKeyValueStore.scala a428a16bc1e9ab4980a6f17db4fd810057d31136 Diff: https://reviews.apache.org/r/35933/diff/ Testing --- Thanks, Gustavo Anatoly F. V. Solís
Re: Review Request 35933: SAMZA-449 Expose RocksDB statistic
On Junho 30, 2015, 3:25 p.m., Yan Fang wrote: samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbStatistic.scala, line 25 https://reviews.apache.org/r/35933/diff/1/?file=993429#file993429line25 It's better to use the name RocksDbStatisticMetrics to indicate it is a metrics. I going to rename the class. Thanks. On Junho 30, 2015, 3:25 p.m., Yan Fang wrote: samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala, line 150 https://reviews.apache.org/r/35933/diff/1/?file=993428#file993428line150 it should accept the metrics (from line 143) as the third parameter. The third parameter (metrics) will be added. On Junho 30, 2015, 3:25 p.m., Yan Fang wrote: samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala, line 243 https://reviews.apache.org/r/35933/diff/1/?file=993428#file993428line243 add some doc here, saying something like, calling this method will expose RocksDB statistic to a metric, etc. The doc will be written On Junho 30, 2015, 3:25 p.m., Yan Fang wrote: samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbStatistic.scala, line 40 https://reviews.apache.org/r/35933/diff/1/?file=993429#file993429line40 can we use the lower case for the all names of metrics? It's better to keep the name convension consistent in the Samza. ok, this is will be changed - Gustavo Anatoly --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35933/#review89902 --- On Junho 26, 2015, 5:56 p.m., Gustavo Anatoly F. V. Solís wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35933/ --- (Updated Junho 26, 2015, 5:56 p.m.) Review request for samza. Repository: samza Description --- RocksDB statistic 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/RocksDbStatistic.scala PRE-CREATION samza-kv-rocksdb/src/test/scala/org/apache/samza/storage/kv/TestRocksDbKeyValueStore.scala a428a16bc1e9ab4980a6f17db4fd810057d31136 Diff: https://reviews.apache.org/r/35933/diff/ Testing --- Thanks, Gustavo Anatoly F. V. Solís
Review Request 35933: SAMZA-449 Expose RocksDB statistic
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35933/ --- Review request for samza. Repository: samza Description --- RocksDB statistic 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/RocksDbStatistic.scala PRE-CREATION samza-kv-rocksdb/src/test/scala/org/apache/samza/storage/kv/TestRocksDbKeyValueStore.scala a428a16bc1e9ab4980a6f17db4fd810057d31136 Diff: https://reviews.apache.org/r/35933/diff/ Testing --- Thanks, Gustavo Anatoly F. V. Solís
