-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46644/#review131844
-----------------------------------------------------------


Fix it, then Ship it!





samza-api/src/main/java/org/apache/samza/storage/StorageEngine.java (line 62)
<https://reviews.apache.org/r/46644/#comment195886>

    Since there is a change to the API, we probably shouldn't commit this until 
after the 10.1 release, for the sake of semantic versioning.



samza-api/src/main/java/org/apache/samza/storage/StoreProperties.java (line 22)
<https://reviews.apache.org/r/46644/#comment195889>

    Javadoc missing everywhere in this file except here.
    
    For example, will the cacheEnabled and loggedStore flags only be true for 
the CachedStore and LoggedStore (sub)classes, or do they have a different 
meaning? 
    
    Is a cached store one that employs any kind of cache, or only a cache that 
might require flushes to disk?
    
    I assume loggedstores are those that have changelog enabled, but to 
newcomers that may not be obvious.


- Jake Maes


On April 25, 2016, 5:25 p.m., Navina Ramesh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46644/
> -----------------------------------------------------------
> 
> (Updated April 25, 2016, 5:25 p.m.)
> 
> 
> Review request for samza, Boris Shkolnik, Chris Pettitt, and Yi Pan (Data 
> Infrastructure).
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> Adding StoreProperties that is accessible from the StorageEngine
> 
> Added a lifecycle unit test for TaskStorageManager
> 
> Question: Is there a better way to refactor the TaskStorageManager class?
> 
> 
> Diffs
> -----
> 
>   checkstyle/import-control.xml c15b8e74de8e5aac5ac83278c52ab3dba1630e50 
>   samza-api/src/main/java/org/apache/samza/storage/StorageEngine.java 
> 5463648fd01e0cba52fa9bd9a33b247e7014cfde 
>   samza-api/src/main/java/org/apache/samza/storage/StorageEngineFactory.java 
> adb62643a311e25fb4fed91c39e1a75cd5664b17 
>   samza-api/src/main/java/org/apache/samza/storage/StoreProperties.java 
> PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala 
> 5462208c08cddbfd30d886daffa8c02c82447059 
>   samza-core/src/main/scala/org/apache/samza/storage/TaskStorageManager.scala 
> c7b05203a1958a62af9dec04b215d985c4646dc4 
>   samza-core/src/test/java/org/apache/samza/storage/MockStorageEngine.java 
> b90ea87b7e575e646c58ddfb5a53ced9ed04a880 
>   
> samza-core/src/test/java/org/apache/samza/storage/MockStorageEngineFactory.java
>  c00c4547307f5a8b401c6bb6438eaa7fb8a38651 
>   samza-core/src/test/java/org/apache/samza/storage/TestStorageRecovery.java 
> 13f4fa97d42b02e54634c8de1575118ca0433fe8 
>   
> samza-core/src/test/scala/org/apache/samza/storage/TestTaskStorageManager.scala
>  c8ea64c7c67dd6bf789d2a3445d620ccef1beac0 
>   
> samza-kv-inmemory/src/main/scala/org/apache/samza/storage/kv/inmemory/InMemoryKeyValueStorageEngineFactory.scala
>  53147ad4faae2fae24a5bc0677167d06c64afead 
>   
> samza-kv-inmemory/src/main/scala/org/apache/samza/storage/kv/inmemory/InMemoryKeyValueStore.scala
>  661a83517c5a603c841d4a373ac979724457471c 
>   
> samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStorageEngineFactory.scala
>  dae6e35d1ba75daf5c816bccbc625c623a44d3b2 
>   
> samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
>  b896810ccf7f12d72195f07cac27ba5cc510077d 
>   
> samza-kv/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala
>  391cf89b05f90ececae63160cd3cb9c811e4ab66 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStore.scala 
> 4c4e82eb5be82d469fe3c5f85b92523faeb0a193 
>   
> samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngine.scala
>  defc91e7e28b1d9419b98f363f9989d58511e923 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LoggedStore.scala 
> e293bfc54fda5e302c22cc85b8fee384d2fe8f35 
>   
> samza-kv/src/main/scala/org/apache/samza/storage/kv/SerializedKeyValueStore.scala
>  cf1a2cc16a5c2fb547012359a32b22c207d64d8e 
>   samza-kv/src/test/scala/org/apache/samza/storage/kv/MockKeyValueStore.scala 
> e14a4617ef23b2ae7b89aee93219677e29455d6c 
>   
> samza-test/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala
>  23f8a1a6bee8ef38e0640a4e90778e53d982deeb 
> 
> Diff: https://reviews.apache.org/r/46644/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew clean build
> TODO: Test with sample job to verify that the behavior reported in SAMZA-889 
> is fixed
> 
> 
> Thanks,
> 
> Navina Ramesh
> 
>

Reply via email to