----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33761/#review82604 -----------------------------------------------------------
Overall looks good. Just some minor comments. samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStore.scala <https://reviews.apache.org/r/33761/#comment133359> Refer to in-memory KV-store class, we can merge those two in one function: getIter(iter) and make range() and all() referring to the same function like: def range(from: K, to: K) = { metrics.ranges.inc flush() getIter(store.range(from, to)) } samza-kv/src/test/scala/org/apache/samza/storage/kv/TestCachedStore.scala <https://reviews.apache.org/r/33761/#comment133362> It would be better to test iter.remove() for both range() and all() - Yi Pan (Data Infrastructure) On May 4, 2015, 9:07 p.m., Guozhang Wang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33761/ > ----------------------------------------------------------- > > (Updated May 4, 2015, 9:07 p.m.) > > > Review request for samza. > > > Bugs: SAMZA-658 > https://issues.apache.org/jira/browse/SAMZA-658 > > > Repository: samza > > > Description > ------- > > Address Yi's comments, add unit test > > > Diffs > ----- > > > samza-kv/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala > b3624e6057ee1a86090f00d2853035b06f63358d > samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStore.scala > 61bb3f6acb080b653f8b11176538549738255acc > > samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngine.scala > 3a23daf053f0b8dec3a7ec83a51c9c5527078a3b > samza-kv/src/test/scala/org/apache/samza/storage/kv/MockKeyValueStore.scala > PRE-CREATION > samza-kv/src/test/scala/org/apache/samza/storage/kv/TestCachedStore.scala > d03ec925b103ccf3c1561de0461fbc39cbe9d9ca > > Diff: https://reviews.apache.org/r/33761/diff/ > > > Testing > ------- > > > Thanks, > > Guozhang Wang > >
