-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33146/
-----------------------------------------------------------
(Updated April 16, 2015, 10:43 a.m.)
Review request for samza.
Changes
-------
Please see the new description for details about this iteration.
Summary (updated)
-----------------
New KeyValueStore Features
Bugs: SAMZA-647
https://issues.apache.org/jira/browse/SAMZA-647
Repository: samza
Description (updated)
-------
* Adding new KeyValueStore methods: Map<K, V> getAll(List<K>), and void
deleteAll(List<K>).
**Please note: "Backwards incompatible API changes, config changes, or library
upgrades should only happen between major revision changes, or when the major
revision is 0." -- since the latter is true, I found that adding the new
methods to the already-existing interface to be a better solution, even though
it breaks backward compatability (please see the first iteration for
context).** Alternatively, to maintain backward compat., I would have added a
new contract and a new class for each class that implements KeyValueStore (to
implement the new contract and pass calls throught to the underlying store,
whose type needs to change in the constructor to the new contract).
* Improved the javadoc of KeyValueStore to have the same voice, to add API
notes, and to follow the javadoc standards.
* Making the KeyValueStore extend AutoClosable and Flushable (since we can use
Java 1.7 now).
* Removing stress tests from TestKeyValueStores.scala because:
* * Unit tests are not meant to be stress-testing the system,
* * The test load looked arbitrary,
* * It wasn't measuring anything (just testing it doesn't crash),
* * Stress testing requires extended periods of testing, and
* * My machine, a MacBook Air, is not beefy enough to survive stress tests;
they should be run on a typical production machine and not a dev one -- a flaky
test is not a good test.
* * There's a test class dedicated for KV stores' performance testing:
TestKeyValuePerformance
* Last, but definitely not least, the main motiviation behind this change:
Allowing RocksDbKeyValueStore to implement getAll(List<K>) to call
multiGet(List<K>); Preliminary tests showed that multiGet is at least 1.25x
faster per key than get (see
https://reviews.facebook.net/rROCKSDB4985a9f73b9fb8a0323fbbb06222ae1f758a6b1d).
Diffs (updated)
-----
samza-kv-inmemory/src/main/scala/org/apache/samza/storage/kv/inmemory/InMemoryKeyValueStore.scala
217333c84c696c0cc1bc3eeabf1c4066a6e89795
samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
66c2a0dc2e38e21f951727a30f0987776ac52fe2
samza-kv/src/main/java/org/apache/samza/storage/kv/KeyValueStore.java
b708341abed15aaad34df5934f5f310bc1feb87a
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/main/scala/org/apache/samza/storage/kv/KeyValueStoreMetrics.scala
79092b91c9498e55f1c4e28661b7280c6c19cef7
samza-kv/src/main/scala/org/apache/samza/storage/kv/LoggedStore.scala
26f4cd9cfef305546c85ef9330f3e8b8be5336f7
samza-kv/src/main/scala/org/apache/samza/storage/kv/NullSafeKeyValueStore.scala
4f48cf490d6c1012591a602c0d29dcc71473090f
samza-kv/src/main/scala/org/apache/samza/storage/kv/SerializedKeyValueStore.scala
531e8bef2069a77fa9ceab36fa738bbaa162fe8c
samza-test/src/main/config/perf/kv-perf.properties
33fcd8d1aea14ecea47bbadb24936f737feedb39
samza-test/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala
50dfc10bb053d74dba70fdbce0ef87609ba447ea
Diff: https://reviews.apache.org/r/33146/diff/
Testing
-------
Unit-tested.
Thanks,
Mohamed Mahmoud (El-Geish)
