> On June 6, 2016, 8:36 p.m., Navina Ramesh wrote: > > samza-kv-rocksdb/src/test/scala/org/apache/samza/storage/kv/TestRocksDbKeyValueStore.scala, > > line 90 > > <https://reviews.apache.org/r/48182/diff/2/?file=1406146#file1406146line90> > > > > I don't understand the point of this test. If you testing removal, what > > is the need to iterate? > > > > If you are testing iterator behavior, then, perhaps the method should > > be renamed?? > > > > Either ways, some javadocs about this test can help! :)
Change the name of the test to match its purpose. > On June 6, 2016, 8:36 p.m., Navina Ramesh wrote: > > samza-kv-rocksdb/src/test/scala/org/apache/samza/storage/kv/TestRocksDbKeyValueStore.scala, > > line 109 > > <https://reviews.apache.org/r/48182/diff/2/?file=1406146#file1406146line109> > > > > nit: unused variable? Good catch! Removed it. - Xinyu ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48182/#review135984 ----------------------------------------------------------- On June 9, 2016, 12:33 a.m., Xinyu Liu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48182/ > ----------------------------------------------------------- > > (Updated June 9, 2016, 12:33 a.m.) > > > Review request for samza, Chris Pettitt, Navina Ramesh, and Yi Pan (Data > Infrastructure). > > > Repository: samza > > > Description > ------- > > All the existing stores/cache need to be thread safe in order to be used by > multithreaded tasks. The following changes are made to ensure the thread > safety of the stores: > > For CachedStore, use sychronized lock for each public function; > For current InMemoryKeyValueStore, use ConcurrentSkipListMap as underlying > map for thread safety. > For store Iterator, do not support remove functionality (throw > UnsupportedOperationException like RocksDb does today). > > > Diffs > ----- > > > samza-kv-inmemory/src/main/scala/org/apache/samza/storage/kv/inmemory/InMemoryKeyValueStore.scala > 72f25a354eaa98e8df379d07d9cc8613dfafd13a > > samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala > f0965aec5f3ec2a214dc40c70832c58273623749 > > samza-kv-rocksdb/src/test/scala/org/apache/samza/storage/kv/TestRocksDbKeyValueStore.scala > b7f1cdc4dbaeea2413cee2ad60d74528f3950513 > samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStore.scala > c28f8db8cb59bd5415e78535877acc1e5bee0f67 > samza-kv/src/test/scala/org/apache/samza/storage/kv/TestCachedStore.scala > eee74473726cb2a36d0b75fe5c9df737440980bc > > samza-test/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala > 23f8a1a6bee8ef38e0640a4e90778e53d982deeb > > Diff: https://reviews.apache.org/r/48182/diff/ > > > Testing > ------- > > Unit tests and local deployment. > > > Thanks, > > Xinyu Liu > >
