You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@samza.apache.org by Navina Ramesh <nr...@linkedin.com> on 2016/04/25 19:16:41 UTC
Review Request 46644: SAMZA-889 - Change log not working properly
with In memory Store
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46644/
-----------------------------------------------------------
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
Thanks,
Navina Ramesh
Re: Review Request 46644: SAMZA-889 - Change log not working properly
with In memory Store
Posted by Navina Ramesh <nr...@linkedin.com>.
> On May 5, 2016, 4:18 p.m., Jake Maes wrote:
> > samza-api/src/main/java/org/apache/samza/storage/StorageEngine.java, line 62
> > <https://reviews.apache.org/r/46644/diff/1/?file=1360200#file1360200line62>
> >
> > 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.
Yeah I agree. Strictly speaking, we shouldn't introduce api changes in bug-fix version. But this patch is a "bug-fix" that is better solved by introducing a change to the interface. That said, I don't believe there will be any users implementing custom "StorageEngine". If at all, there may be custom implementations of KeyValueStores. Given our release timelines and the fact that this bug needs to be addressed asap, I think we should document the interface change and continue with the patch.
> On May 5, 2016, 4:18 p.m., Jake Maes wrote:
> > samza-api/src/main/java/org/apache/samza/storage/StoreProperties.java, line 22
> > <https://reviews.apache.org/r/46644/diff/1/?file=1360202#file1360202line22>
> >
> > 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.
I will add the Javadocs. The properties in StoreProperties are a collective set of properties for a given store. Not a particular subclass.
By cacheEnabled, I only meant to imply whether the "CachedStored" wrapper is enabled or not. However, as Chris pointed out, that is not used anywhere. So, I am removing it from the list of properties.
- Navina
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46644/#review131844
-----------------------------------------------------------
On May 5, 2016, 9:13 p.m., Navina Ramesh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46644/
> -----------------------------------------------------------
>
> (Updated May 5, 2016, 9:13 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 9afab881f2a822925ae716e1dbd744b86321c34e
> 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-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 f0965aec5f3ec2a214dc40c70832c58273623749
> samza-kv/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala 391cf89b05f90ececae63160cd3cb9c811e4ab66
> samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngine.scala e5a66a4770b9553a1cc48fbb505f52d123c6c754
> 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
>
>
Re: Review Request 46644: SAMZA-889 - Change log not working properly
with In memory Store
Posted by "Yi Pan (Data Infrastructure)" <yi...@linkedin.com>.
> On May 5, 2016, 4:18 p.m., Jake Maes wrote:
> > samza-api/src/main/java/org/apache/samza/storage/StorageEngine.java, line 62
> > <https://reviews.apache.org/r/46644/diff/1/?file=1360200#file1360200line62>
> >
> > 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.
>
> Navina Ramesh wrote:
> Yeah I agree. Strictly speaking, we shouldn't introduce api changes in bug-fix version. But this patch is a "bug-fix" that is better solved by introducing a change to the interface. That said, I don't believe there will be any users implementing custom "StorageEngine". If at all, there may be custom implementations of KeyValueStores. Given our release timelines and the fact that this bug needs to be addressed asap, I think we should document the interface change and continue with the patch.
>
> Jake Maes wrote:
> Can we please merge SAMZA-932 and SAMZA-943 first? We didn't think anyone would be impacted by SAMZA-813 either but 5 internal teams were. So, I'd like to be able to have an easy way to roll back if we run into the same scenario here.
Looks like SAMZA-932 and SAMZA-943 are merged. Let's merge this one in, since really almost no one extends StorageEngine now. SAMZA-423 may be the first of this kind, but it has not been merged yet.
- Yi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46644/#review131844
-----------------------------------------------------------
On May 6, 2016, 2:07 a.m., Navina Ramesh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46644/
> -----------------------------------------------------------
>
> (Updated May 6, 2016, 2:07 a.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 9afab881f2a822925ae716e1dbd744b86321c34e
> 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-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 f0965aec5f3ec2a214dc40c70832c58273623749
> samza-kv/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala 391cf89b05f90ececae63160cd3cb9c811e4ab66
> samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngine.scala e5a66a4770b9553a1cc48fbb505f52d123c6c754
> 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
>
>
Re: Review Request 46644: SAMZA-889 - Change log not working properly
with In memory Store
Posted by Jake Maes <ja...@gmail.com>.
> On May 5, 2016, 4:18 p.m., Jake Maes wrote:
> > samza-api/src/main/java/org/apache/samza/storage/StorageEngine.java, line 62
> > <https://reviews.apache.org/r/46644/diff/1/?file=1360200#file1360200line62>
> >
> > 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.
>
> Navina Ramesh wrote:
> Yeah I agree. Strictly speaking, we shouldn't introduce api changes in bug-fix version. But this patch is a "bug-fix" that is better solved by introducing a change to the interface. That said, I don't believe there will be any users implementing custom "StorageEngine". If at all, there may be custom implementations of KeyValueStores. Given our release timelines and the fact that this bug needs to be addressed asap, I think we should document the interface change and continue with the patch.
Can we please merge SAMZA-932 and SAMZA-943 first? We didn't think anyone would be impacted by SAMZA-813 either but 5 internal teams were. So, I'd like to be able to have an easy way to roll back if we run into the same scenario here.
- Jake
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46644/#review131844
-----------------------------------------------------------
On May 5, 2016, 9:13 p.m., Navina Ramesh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46644/
> -----------------------------------------------------------
>
> (Updated May 5, 2016, 9:13 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 9afab881f2a822925ae716e1dbd744b86321c34e
> 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-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 f0965aec5f3ec2a214dc40c70832c58273623749
> samza-kv/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala 391cf89b05f90ececae63160cd3cb9c811e4ab66
> samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngine.scala e5a66a4770b9553a1cc48fbb505f52d123c6c754
> 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
>
>
Re: Review Request 46644: SAMZA-889 - Change log not working properly
with In memory Store
Posted by Jake Maes <ja...@gmail.com>.
-----------------------------------------------------------
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
>
>
Re: Review Request 46644: SAMZA-889 - Change log not working properly
with In memory Store
Posted by Navina Ramesh <nr...@linkedin.com>.
> On May 5, 2016, 3:22 p.m., Chris Pettitt wrote:
> > samza-api/src/main/java/org/apache/samza/storage/StoreProperties.java, line 40
> > <https://reviews.apache.org/r/46644/diff/1/?file=1360202#file1360202line40>
> >
> > It would be great to have some documentation about what each of these properties means. It's probably already documented somewhere, so a javadoc link might be sufficient.
Done!
> On May 5, 2016, 3:22 p.m., Chris Pettitt wrote:
> > samza-api/src/main/java/org/apache/samza/storage/StoreProperties.java, lines 43-45
> > <https://reviews.apache.org/r/46644/diff/1/?file=1360202#file1360202line43>
> >
> > What is this for? It doesn't appear to be used. Generally bias towards not adding things that are unused ceteris paribus.
I added this property for completeness in the properties we may need to set. However, you are right. Not worth introducing something which we don't currently use.
> On May 5, 2016, 3:22 p.m., Chris Pettitt wrote:
> > samza-test/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala, line 18
> > <https://reviews.apache.org/r/46644/diff/1/?file=1360219#file1360219line18>
> >
> > Generally it is a best practice to seperate out non-change-related clean up to a separate commit. This helps when working with history (e.g. git bisect).
I agree.But we have so much of debt that it never gets cleaned up. Some of them were IDEA imposed auto-corrects. I have disabled them. I will try to keep the non-change-related code separate going forward. Thanks for the guidance! :)
- Navina
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46644/#review131835
-----------------------------------------------------------
On May 5, 2016, 9:13 p.m., Navina Ramesh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46644/
> -----------------------------------------------------------
>
> (Updated May 5, 2016, 9:13 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 9afab881f2a822925ae716e1dbd744b86321c34e
> 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-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 f0965aec5f3ec2a214dc40c70832c58273623749
> samza-kv/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala 391cf89b05f90ececae63160cd3cb9c811e4ab66
> samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngine.scala e5a66a4770b9553a1cc48fbb505f52d123c6c754
> 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
>
>
Re: Review Request 46644: SAMZA-889 - Change log not working properly
with In memory Store
Posted by Chris Pettitt <cp...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46644/#review131835
-----------------------------------------------------------
Fix it, then Ship it!
Looks good. I'd suggest moving non-change-related code to a separate cleanup RB.
samza-api/src/main/java/org/apache/samza/storage/StoreProperties.java (line 29)
<https://reviews.apache.org/r/46644/#comment195875>
Generally you make the constructor private, but this is such a trivial value object that I wouldn't have a strong opinion either way.
samza-api/src/main/java/org/apache/samza/storage/StoreProperties.java (lines 31 - 33)
<https://reviews.apache.org/r/46644/#comment195873>
Prefer boolean to Boolean through out. Boolean is a boxed type (thus nullable), introducing a third state whereas boolean has only two states to manage.
samza-api/src/main/java/org/apache/samza/storage/StoreProperties.java (line 40)
<https://reviews.apache.org/r/46644/#comment195879>
It would be great to have some documentation about what each of these properties means. It's probably already documented somewhere, so a javadoc link might be sufficient.
samza-api/src/main/java/org/apache/samza/storage/StoreProperties.java (lines 43 - 45)
<https://reviews.apache.org/r/46644/#comment195876>
What is this for? It doesn't appear to be used. Generally bias towards not adding things that are unused ceteris paribus.
samza-test/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala (line 18)
<https://reviews.apache.org/r/46644/#comment195880>
Generally it is a best practice to seperate out non-change-related clean up to a separate commit. This helps when working with history (e.g. git bisect).
- Chris Pettitt
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
>
>
Re: Review Request 46644: SAMZA-889 - Change log not working properly
with In memory Store
Posted by Navina Ramesh <nr...@linkedin.com>.
> On May 5, 2016, 9:19 p.m., Chris Pettitt wrote:
> > samza-api/src/main/java/org/apache/samza/storage/StoreProperties.java, lines 25-31
> > <https://reviews.apache.org/r/46644/diff/2/?file=1374033#file1374033line25>
> >
> > I would actually move this doc to the interface. I would imagine folks would be looking at methods versus the private fields (in fact, I believe javadoc is not generated for private members by default).
I made the change. Hope that's what you meant :)
- Navina
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46644/#review131932
-----------------------------------------------------------
On May 6, 2016, 2:07 a.m., Navina Ramesh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46644/
> -----------------------------------------------------------
>
> (Updated May 6, 2016, 2:07 a.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 9afab881f2a822925ae716e1dbd744b86321c34e
> 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-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 f0965aec5f3ec2a214dc40c70832c58273623749
> samza-kv/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala 391cf89b05f90ececae63160cd3cb9c811e4ab66
> samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngine.scala e5a66a4770b9553a1cc48fbb505f52d123c6c754
> 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
>
>
Re: Review Request 46644: SAMZA-889 - Change log not working properly
with In memory Store
Posted by Chris Pettitt <cp...@linkedin.com>.
> On May 5, 2016, 9:19 p.m., Chris Pettitt wrote:
> > samza-api/src/main/java/org/apache/samza/storage/StoreProperties.java, lines 25-31
> > <https://reviews.apache.org/r/46644/diff/2/?file=1374033#file1374033line25>
> >
> > I would actually move this doc to the interface. I would imagine folks would be looking at methods versus the private fields (in fact, I believe javadoc is not generated for private members by default).
>
> Navina Ramesh wrote:
> I made the change. Hope that's what you meant :)
That was it. Thanks!
- Chris
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46644/#review131932
-----------------------------------------------------------
On May 6, 2016, 2:07 a.m., Navina Ramesh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46644/
> -----------------------------------------------------------
>
> (Updated May 6, 2016, 2:07 a.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 9afab881f2a822925ae716e1dbd744b86321c34e
> 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-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 f0965aec5f3ec2a214dc40c70832c58273623749
> samza-kv/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala 391cf89b05f90ececae63160cd3cb9c811e4ab66
> samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngine.scala e5a66a4770b9553a1cc48fbb505f52d123c6c754
> 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
>
>
Re: Review Request 46644: SAMZA-889 - Change log not working properly
with In memory Store
Posted by Chris Pettitt <cp...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46644/#review131932
-----------------------------------------------------------
Fix it, then Ship it!
Looks good. Minor tweak suggested below.
samza-api/src/main/java/org/apache/samza/storage/StoreProperties.java (lines 25 - 31)
<https://reviews.apache.org/r/46644/#comment195978>
I would actually move this doc to the interface. I would imagine folks would be looking at methods versus the private fields (in fact, I believe javadoc is not generated for private members by default).
- Chris Pettitt
On May 5, 2016, 9:13 p.m., Navina Ramesh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46644/
> -----------------------------------------------------------
>
> (Updated May 5, 2016, 9:13 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 9afab881f2a822925ae716e1dbd744b86321c34e
> 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-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 f0965aec5f3ec2a214dc40c70832c58273623749
> samza-kv/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala 391cf89b05f90ececae63160cd3cb9c811e4ab66
> samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngine.scala e5a66a4770b9553a1cc48fbb505f52d123c6c754
> 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
>
>
Re: Review Request 46644: SAMZA-889 - Change log not working properly
with In memory Store
Posted by "Yi Pan (Data Infrastructure)" <yi...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46644/#review132109
-----------------------------------------------------------
Ship it!
Ship It!
- Yi Pan (Data Infrastructure)
On May 6, 2016, 2:07 a.m., Navina Ramesh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46644/
> -----------------------------------------------------------
>
> (Updated May 6, 2016, 2:07 a.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 9afab881f2a822925ae716e1dbd744b86321c34e
> 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-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 f0965aec5f3ec2a214dc40c70832c58273623749
> samza-kv/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala 391cf89b05f90ececae63160cd3cb9c811e4ab66
> samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngine.scala e5a66a4770b9553a1cc48fbb505f52d123c6c754
> 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
>
>
Re: Review Request 46644: SAMZA-889 - Change log not working properly
with In memory Store
Posted by Navina Ramesh <nr...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46644/
-----------------------------------------------------------
(Updated May 6, 2016, 2:07 a.m.)
Review request for samza, Boris Shkolnik, Chris Pettitt, and Yi Pan (Data Infrastructure).
Changes
-------
Fixing the documentation
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 (updated)
-----
checkstyle/import-control.xml 9afab881f2a822925ae716e1dbd744b86321c34e
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-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 f0965aec5f3ec2a214dc40c70832c58273623749
samza-kv/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala 391cf89b05f90ececae63160cd3cb9c811e4ab66
samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngine.scala e5a66a4770b9553a1cc48fbb505f52d123c6c754
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
Re: Review Request 46644: SAMZA-889 - Change log not working properly
with In memory Store
Posted by Navina Ramesh <nr...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46644/
-----------------------------------------------------------
(Updated May 5, 2016, 9:13 p.m.)
Review request for samza, Boris Shkolnik, Chris Pettitt, and Yi Pan (Data Infrastructure).
Changes
-------
Addressing feedback from Chris & Jake
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 (updated)
-----
checkstyle/import-control.xml 9afab881f2a822925ae716e1dbd744b86321c34e
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-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 f0965aec5f3ec2a214dc40c70832c58273623749
samza-kv/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala 391cf89b05f90ececae63160cd3cb9c811e4ab66
samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngine.scala e5a66a4770b9553a1cc48fbb505f52d123c6c754
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
Re: Review Request 46644: SAMZA-889 - Change log not working properly
with In memory Store
Posted by Navina Ramesh <nr...@linkedin.com>.
-----------------------------------------------------------
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 (updated)
-------
./gradlew clean build
TODO: Test with sample job to verify that the behavior reported in SAMZA-889 is fixed
Thanks,
Navina Ramesh