> On Aug. 5, 2016, 6:31 p.m., Yi Pan (Data Infrastructure) wrote: > > samza-kv/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala, > > line 135 > > <https://reviews.apache.org/r/50619/diff/1/?file=1458229#file1458229line135> > > > > Why do we need to add the default function, if you already defined the > > default in KeyValueStorageEngine.scala?
Thanks a lot. My initial consideration was to minimize potential errors if somebody would like to add new paramters for this constructor since there are a lot already. I also saw some similar uses in the existing samza code base as well. I am OK on either one. I can follow whatever code standard our team has. - Fred ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50619/#review144968 ----------------------------------------------------------- On July 29, 2016, 10:24 p.m., Fred Ji wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50619/ > ----------------------------------------------------------- > > (Updated July 29, 2016, 10:24 p.m.) > > > Review request for samza, Chris Pettitt, Jake Maes, and Yi Pan (Data > Infrastructure). > > > Bugs: SAMZA-963 > https://issues.apache.org/jira/browse/SAMZA-963 > > > Repository: samza > > > Description > ------- > > SAMZA-963: add KV storage engine timers to help identify the issues on kv > stores and also add unit test > > > Diffs > ----- > > > samza-kv/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala > c975893a42689732c39c39600fecacee843bf9d6 > > samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngine.scala > a3ffc421020b7a84c40b2101f2e37db8a20690cb > > samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineMetrics.scala > 233fba91caf041bfb78189efef00ce8fc56f9f15 > > samza-kv/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStorageEngine.scala > PRE-CREATION > > Diff: https://reviews.apache.org/r/50619/diff/ > > > Testing > ------- > > 1. unit test is successful on a newly created test file for > KeyValueStorageEngine: ./gradlew clean :samza-kv_2.10:test > -Dtest.single=TestKeyValueStorageEngine > 2. build and all unit tests are successful: ./gradlew clean build > 3. ./gradlew checkstyleMain checkstyleTest passed > 4. manually tested on local machine for a stateful sample job depending on > KVStore, and from jconsole, the corresponding metrics were seen in mbeans > (see attached file) and the metrics were updated as expected. > > > File Attachments > ---------------- > > snapshot of KeyValueStorageEngineMetrics bean from jconsole on local test > > https://reviews.apache.org/media/uploaded/files/2016/07/29/ce4b0456-73f9-44d5-af7d-0c45bf91bcaa__KeyValueStorageEngineMetricsFromJconsole.png > > > Thanks, > > Fred Ji > >
