You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@samza.apache.org by Chinmay Soman <ch...@gmail.com> on 2014/07/09 19:47:40 UTC

Review Request 23372: SAMZA-256: Provide in-memory data store implementation

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

Review request for samza.


Repository: samza


Description
-------

SAMZA-256: Provide in-memory data store implementation


Diffs
-----

  build.gradle f1a458b8c4cbe6e38f28ba3c5bd9099590c7abde 
  gradle/dependency-versions-scala-2.10.gradle 7094d6cf86b9da454633525dc5da65e5a9f90f4c 
  samza-core/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala PRE-CREATION 
  samza-core/src/main/scala/org/apache/samza/storage/kv/inmemory/InMemoryKeyValueStore.scala PRE-CREATION 
  samza-kv/src/main/java/org/apache/samza/storage/kv/Entry.java  
  samza-kv/src/main/java/org/apache/samza/storage/kv/KeyValueIterator.java  
  samza-kv/src/main/java/org/apache/samza/storage/kv/KeyValueStore.java  
  samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStore.scala 429f51a4d269e3d18627c183359af672c68b8b00 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStoreMetrics.scala 8ffcfb194b981a798fd19b05f9e6bff9fc2f79e2 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStoreMetrics.scala 8ffcfb194b981a798fd19b05f9e6bff9fc2f79e2 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngine.scala f42ea026cd4a2ae633d71149af6dacef68dab352 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineFactory.scala 36b7e97b427d3d0fd18f38609e2dac2b081a8dbc 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineMetrics.scala a7958f62582a2cc998f026c6818afd24936d46d4 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStore.scala dae3c2c8570d4add7bc3c01f0fb73c867c609740 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStoreMetrics.scala 8949f2fb6d8488e5e061139299e9ea97220ea180 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStoreMetrics.scala  
  samza-kv/src/main/scala/org/apache/samza/storage/kv/LoggedStore.scala d3c1ae84b56827964ce10d2aa9dedcdc7221c596 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/LoggedStoreMetrics.scala ea56e8c2e41efa0c59bedac9e489b399fc8813e4 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/NullSafeKeyValueStore.scala 00f9af319c780d9513b735016b1c3e016714f5c8 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/SerializedKeyValueStore.scala 2d3b6e55c20172aa5f149c6d6728cab8229312c3 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/SerializedKeyValueStoreMetrics.scala 2ad21c822f3d9c08dcc1e78d4b86c3fd314b7ed2 
  samza-kv/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala ec23567570e2d7868390f570bd65d8cd7b18d4dc 
  samza-leveldb/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineFactory.scala PRE-CREATION 
  samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 0077af08ac33bd95b1d40c362f95dfd6208cf99c 
  settings.gradle 6ba628059a3168abf80d072d68942f79bd237811 

Diff: https://reviews.apache.org/r/23372/diff/


Testing
-------


Thanks,

Chinmay Soman


Re: Review Request 23372: SAMZA-256: Provide in-memory data store implementation

Posted by Chinmay Soman <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23372/#review47536
-----------------------------------------------------------



gradle/dependency-versions-scala-2.10.gradle
<https://reviews.apache.org/r/23372/#comment83470>

    Added to dependency-versions.gradle



samza-core/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala
<https://reviews.apache.org/r/23372/#comment83473>

    This is not new code, this originally belonged to KeyValueStorageEngineFactory (in samza-kv)



samza-core/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala
<https://reviews.apache.org/r/23372/#comment83474>

    Same as above



samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStore.scala
<https://reviews.apache.org/r/23372/#comment83475>

    No. SAMZA-326 is only for release-0.7.1



samza-kv/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala
<https://reviews.apache.org/r/23372/#comment83478>

    In case of in-memory store - the dir is not created at all.


- Chinmay Soman


On July 9, 2014, 5:47 p.m., Chinmay Soman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23372/
> -----------------------------------------------------------
> 
> (Updated July 9, 2014, 5:47 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> SAMZA-256: Provide in-memory data store implementation
> 
> 
> Diffs
> -----
> 
>   build.gradle f1a458b8c4cbe6e38f28ba3c5bd9099590c7abde 
>   gradle/dependency-versions-scala-2.10.gradle 7094d6cf86b9da454633525dc5da65e5a9f90f4c 
>   samza-core/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/storage/kv/inmemory/InMemoryKeyValueStore.scala PRE-CREATION 
>   samza-kv/src/main/java/org/apache/samza/storage/kv/Entry.java  
>   samza-kv/src/main/java/org/apache/samza/storage/kv/KeyValueIterator.java  
>   samza-kv/src/main/java/org/apache/samza/storage/kv/KeyValueStore.java  
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStore.scala 429f51a4d269e3d18627c183359af672c68b8b00 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStoreMetrics.scala 8ffcfb194b981a798fd19b05f9e6bff9fc2f79e2 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStoreMetrics.scala 8ffcfb194b981a798fd19b05f9e6bff9fc2f79e2 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngine.scala f42ea026cd4a2ae633d71149af6dacef68dab352 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineFactory.scala 36b7e97b427d3d0fd18f38609e2dac2b081a8dbc 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineMetrics.scala a7958f62582a2cc998f026c6818afd24936d46d4 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStore.scala dae3c2c8570d4add7bc3c01f0fb73c867c609740 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStoreMetrics.scala 8949f2fb6d8488e5e061139299e9ea97220ea180 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStoreMetrics.scala  
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LoggedStore.scala d3c1ae84b56827964ce10d2aa9dedcdc7221c596 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LoggedStoreMetrics.scala ea56e8c2e41efa0c59bedac9e489b399fc8813e4 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/NullSafeKeyValueStore.scala 00f9af319c780d9513b735016b1c3e016714f5c8 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/SerializedKeyValueStore.scala 2d3b6e55c20172aa5f149c6d6728cab8229312c3 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/SerializedKeyValueStoreMetrics.scala 2ad21c822f3d9c08dcc1e78d4b86c3fd314b7ed2 
>   samza-kv/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala ec23567570e2d7868390f570bd65d8cd7b18d4dc 
>   samza-leveldb/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineFactory.scala PRE-CREATION 
>   samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 0077af08ac33bd95b1d40c362f95dfd6208cf99c 
>   settings.gradle 6ba628059a3168abf80d072d68942f79bd237811 
> 
> Diff: https://reviews.apache.org/r/23372/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chinmay Soman
> 
>


Re: Review Request 23372: SAMZA-256: Provide in-memory data store implementation

Posted by Chinmay Soman <ch...@gmail.com>.

> On July 9, 2014, 10:37 p.m., Jakob Homan wrote:
> > samza-core/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala, line 48
> > <https://reviews.apache.org/r/23372/diff/1/?file=627054#file627054line48>
> >
> >     Does the maybePrefix end up adding anything here? Either way you get a store and there's no difference in how it's used, right? I'm concerned this might confuse people to think it's an Option wrapped value.

This is not new code, this originally belonged to KeyValueStorageEngineFactory (in samza-kv)


> On July 9, 2014, 10:37 p.m., Jakob Homan wrote:
> > samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStore.scala, line 206
> > <https://reviews.apache.org/r/23372/diff/1/?file=627065#file627065line206>
> >
> >     Is this not going to collide with SAMZA-326?

No. SAMZA-326 is only for release-0.7.1


- Chinmay


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


On July 9, 2014, 5:47 p.m., Chinmay Soman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23372/
> -----------------------------------------------------------
> 
> (Updated July 9, 2014, 5:47 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> SAMZA-256: Provide in-memory data store implementation
> 
> 
> Diffs
> -----
> 
>   build.gradle f1a458b8c4cbe6e38f28ba3c5bd9099590c7abde 
>   gradle/dependency-versions-scala-2.10.gradle 7094d6cf86b9da454633525dc5da65e5a9f90f4c 
>   samza-core/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/storage/kv/inmemory/InMemoryKeyValueStore.scala PRE-CREATION 
>   samza-kv/src/main/java/org/apache/samza/storage/kv/Entry.java  
>   samza-kv/src/main/java/org/apache/samza/storage/kv/KeyValueIterator.java  
>   samza-kv/src/main/java/org/apache/samza/storage/kv/KeyValueStore.java  
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStore.scala 429f51a4d269e3d18627c183359af672c68b8b00 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStoreMetrics.scala 8ffcfb194b981a798fd19b05f9e6bff9fc2f79e2 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStoreMetrics.scala 8ffcfb194b981a798fd19b05f9e6bff9fc2f79e2 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngine.scala f42ea026cd4a2ae633d71149af6dacef68dab352 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineFactory.scala 36b7e97b427d3d0fd18f38609e2dac2b081a8dbc 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineMetrics.scala a7958f62582a2cc998f026c6818afd24936d46d4 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStore.scala dae3c2c8570d4add7bc3c01f0fb73c867c609740 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStoreMetrics.scala 8949f2fb6d8488e5e061139299e9ea97220ea180 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStoreMetrics.scala  
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LoggedStore.scala d3c1ae84b56827964ce10d2aa9dedcdc7221c596 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LoggedStoreMetrics.scala ea56e8c2e41efa0c59bedac9e489b399fc8813e4 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/NullSafeKeyValueStore.scala 00f9af319c780d9513b735016b1c3e016714f5c8 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/SerializedKeyValueStore.scala 2d3b6e55c20172aa5f149c6d6728cab8229312c3 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/SerializedKeyValueStoreMetrics.scala 2ad21c822f3d9c08dcc1e78d4b86c3fd314b7ed2 
>   samza-kv/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala ec23567570e2d7868390f570bd65d8cd7b18d4dc 
>   samza-leveldb/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineFactory.scala PRE-CREATION 
>   samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 0077af08ac33bd95b1d40c362f95dfd6208cf99c 
>   settings.gradle 6ba628059a3168abf80d072d68942f79bd237811 
> 
> Diff: https://reviews.apache.org/r/23372/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chinmay Soman
> 
>


Re: Review Request 23372: SAMZA-256: Provide in-memory data store implementation

Posted by Jakob Homan <jg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23372/#review47530
-----------------------------------------------------------



build.gradle
<https://reviews.apache.org/r/23372/#comment83443>

    samza-kv-leveldb? Not sure how important this is.



gradle/dependency-versions-scala-2.10.gradle
<https://reviews.apache.org/r/23372/#comment83444>

    We need this in 2.9 too, right?



samza-core/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala
<https://reviews.apache.org/r/23372/#comment83446>

    javadoc for the methods too.



samza-core/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala
<https://reviews.apache.org/r/23372/#comment83445>

    This indenting is off compared to the next method.



samza-core/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala
<https://reviews.apache.org/r/23372/#comment83447>

    store's



samza-core/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala
<https://reviews.apache.org/r/23372/#comment83449>

    Does the maybePrefix end up adding anything here? Either way you get a store and there's no difference in how it's used, right? I'm concerned this might confuse people to think it's an Option wrapped value.



samza-core/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala
<https://reviews.apache.org/r/23372/#comment83450>

    same as above



samza-core/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala
<https://reviews.apache.org/r/23372/#comment83451>

    Should this be a TODO?



samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStore.scala
<https://reviews.apache.org/r/23372/#comment83452>

    Is this not going to collide with SAMZA-326?



samza-kv/src/main/scala/org/apache/samza/storage/kv/LoggedStoreMetrics.scala
<https://reviews.apache.org/r/23372/#comment83453>

    It looks like you have pretty aggressive reformatting options turned on.  Not a problem, but lots of churn.


- Jakob Homan


On July 9, 2014, 10:47 a.m., Chinmay Soman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23372/
> -----------------------------------------------------------
> 
> (Updated July 9, 2014, 10:47 a.m.)
> 
> 
> Review request for samza.
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> SAMZA-256: Provide in-memory data store implementation
> 
> 
> Diffs
> -----
> 
>   build.gradle f1a458b8c4cbe6e38f28ba3c5bd9099590c7abde 
>   gradle/dependency-versions-scala-2.10.gradle 7094d6cf86b9da454633525dc5da65e5a9f90f4c 
>   samza-core/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/storage/kv/inmemory/InMemoryKeyValueStore.scala PRE-CREATION 
>   samza-kv/src/main/java/org/apache/samza/storage/kv/Entry.java  
>   samza-kv/src/main/java/org/apache/samza/storage/kv/KeyValueIterator.java  
>   samza-kv/src/main/java/org/apache/samza/storage/kv/KeyValueStore.java  
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStore.scala 429f51a4d269e3d18627c183359af672c68b8b00 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStoreMetrics.scala 8ffcfb194b981a798fd19b05f9e6bff9fc2f79e2 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStoreMetrics.scala 8ffcfb194b981a798fd19b05f9e6bff9fc2f79e2 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngine.scala f42ea026cd4a2ae633d71149af6dacef68dab352 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineFactory.scala 36b7e97b427d3d0fd18f38609e2dac2b081a8dbc 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineMetrics.scala a7958f62582a2cc998f026c6818afd24936d46d4 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStore.scala dae3c2c8570d4add7bc3c01f0fb73c867c609740 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStoreMetrics.scala 8949f2fb6d8488e5e061139299e9ea97220ea180 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStoreMetrics.scala  
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LoggedStore.scala d3c1ae84b56827964ce10d2aa9dedcdc7221c596 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LoggedStoreMetrics.scala ea56e8c2e41efa0c59bedac9e489b399fc8813e4 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/NullSafeKeyValueStore.scala 00f9af319c780d9513b735016b1c3e016714f5c8 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/SerializedKeyValueStore.scala 2d3b6e55c20172aa5f149c6d6728cab8229312c3 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/SerializedKeyValueStoreMetrics.scala 2ad21c822f3d9c08dcc1e78d4b86c3fd314b7ed2 
>   samza-kv/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala ec23567570e2d7868390f570bd65d8cd7b18d4dc 
>   samza-leveldb/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineFactory.scala PRE-CREATION 
>   samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 0077af08ac33bd95b1d40c362f95dfd6208cf99c 
>   settings.gradle 6ba628059a3168abf80d072d68942f79bd237811 
> 
> Diff: https://reviews.apache.org/r/23372/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chinmay Soman
> 
>


Re: Review Request 23372: SAMZA-256: Provide in-memory data store implementation

Posted by Chinmay Soman <ch...@gmail.com>.

> On July 10, 2014, 2:45 a.m., Chris Riccomini wrote:
> > samza-kv-leveldb/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineFactory.scala, line 34
> > <https://reviews.apache.org/r/23372/diff/2/?file=627435#file627435line34>
> >
> >     Can we just call LevelDbKeyValueStorageEngineFactory.getKVStore() here? Code seems the same as what's in LevelDbKeyValueStorageEngineFactory, and they're in the same module/package.

consolidated the code as a static method which is invoked in both 'LevelDbKeyValueStorageEngineFactory' and 'KeyValueStorageEngineFactory'


- Chinmay


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


On July 10, 2014, 6:46 p.m., Chinmay Soman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23372/
> -----------------------------------------------------------
> 
> (Updated July 10, 2014, 6:46 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> SAMZA-256: Review comments (2) : Merging LevelDb and KeyValue StorageEngine factories
> 
> 
> SAMZA-256: Addressing review comments
> 
> 
> SAMZA-256: Adding missing KeyValueStorageEngineFactory.scala
> 
> 
> SAMZA-256: Provide in-memory data store implementation
> 
> 
> Diffs
> -----
> 
>   build.gradle c853fc067ab4d23d9fb215b349b8e7f3ca1c4dad 
>   gradle/dependency-versions.gradle 078725806f5d41066c6f6a63885d4f6acce8d23c 
>   samza-kv-inmemory/src/main/scala/org/apache/samza/storage/kv/inmemory/InMemoryKeyValueStore.scala PRE-CREATION 
>   samza-kv-leveldb/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStorageEngineFactory.scala PRE-CREATION 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala PRE-CREATION 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStore.scala 429f51a4d269e3d18627c183359af672c68b8b00 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStoreMetrics.scala 8ffcfb194b981a798fd19b05f9e6bff9fc2f79e2 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngine.scala f42ea026cd4a2ae633d71149af6dacef68dab352 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineFactory.scala 36b7e97b427d3d0fd18f38609e2dac2b081a8dbc 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineMetrics.scala a7958f62582a2cc998f026c6818afd24936d46d4 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStore.scala 72562cfcaccb99e35f84df87045271473bd82b04 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStoreMetrics.scala 8949f2fb6d8488e5e061139299e9ea97220ea180 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStoreMetrics.scala 8949f2fb6d8488e5e061139299e9ea97220ea180 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStoreMetrics.scala 8949f2fb6d8488e5e061139299e9ea97220ea180 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LoggedStore.scala d3c1ae84b56827964ce10d2aa9dedcdc7221c596 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LoggedStoreMetrics.scala ea56e8c2e41efa0c59bedac9e489b399fc8813e4 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/NullSafeKeyValueStore.scala 00f9af319c780d9513b735016b1c3e016714f5c8 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/SerializedKeyValueStore.scala 2d3b6e55c20172aa5f149c6d6728cab8229312c3 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/SerializedKeyValueStoreMetrics.scala 2ad21c822f3d9c08dcc1e78d4b86c3fd314b7ed2 
>   samza-kv/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala 4856be0a92c6c50cfee800e659cad9cc7b05f4d5 
>   samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 0077af08ac33bd95b1d40c362f95dfd6208cf99c 
>   settings.gradle 6ba628059a3168abf80d072d68942f79bd237811 
> 
> Diff: https://reviews.apache.org/r/23372/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chinmay Soman
> 
>


Re: Review Request 23372: SAMZA-256: Provide in-memory data store implementation

Posted by Chris Riccomini <cr...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23372/#review47544
-----------------------------------------------------------



samza-kv-leveldb/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineFactory.scala
<https://reviews.apache.org/r/23372/#comment83526>

    Can we just call LevelDbKeyValueStorageEngineFactory.getKVStore() here? Code seems the same as what's in LevelDbKeyValueStorageEngineFactory, and they're in the same module/package.


- Chris Riccomini


On July 10, 2014, 12:55 a.m., Chinmay Soman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23372/
> -----------------------------------------------------------
> 
> (Updated July 10, 2014, 12:55 a.m.)
> 
> 
> Review request for samza.
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> SAMZA-256: Addressing review comments
> 
> 
> SAMZA-256: Adding missing KeyValueStorageEngineFactory.scala
> 
> 
> SAMZA-256: Provide in-memory data store implementation
> 
> 
> Diffs
> -----
> 
>   build.gradle c853fc067ab4d23d9fb215b349b8e7f3ca1c4dad 
>   gradle/dependency-versions.gradle 078725806f5d41066c6f6a63885d4f6acce8d23c 
>   samza-kv-inmemory/src/main/scala/org/apache/samza/storage/kv/inmemory/InMemoryKeyValueStore.scala PRE-CREATION 
>   samza-kv-leveldb/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineFactory.scala PRE-CREATION 
>   samza-kv-leveldb/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStorageEngineFactory.scala PRE-CREATION 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala PRE-CREATION 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStore.scala 429f51a4d269e3d18627c183359af672c68b8b00 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStoreMetrics.scala 8ffcfb194b981a798fd19b05f9e6bff9fc2f79e2 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngine.scala f42ea026cd4a2ae633d71149af6dacef68dab352 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineFactory.scala 36b7e97b427d3d0fd18f38609e2dac2b081a8dbc 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineMetrics.scala a7958f62582a2cc998f026c6818afd24936d46d4 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStore.scala 72562cfcaccb99e35f84df87045271473bd82b04 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStoreMetrics.scala 8949f2fb6d8488e5e061139299e9ea97220ea180 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStoreMetrics.scala 8949f2fb6d8488e5e061139299e9ea97220ea180 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LoggedStore.scala d3c1ae84b56827964ce10d2aa9dedcdc7221c596 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LoggedStoreMetrics.scala ea56e8c2e41efa0c59bedac9e489b399fc8813e4 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/NullSafeKeyValueStore.scala 00f9af319c780d9513b735016b1c3e016714f5c8 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/SerializedKeyValueStore.scala 2d3b6e55c20172aa5f149c6d6728cab8229312c3 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/SerializedKeyValueStoreMetrics.scala 2ad21c822f3d9c08dcc1e78d4b86c3fd314b7ed2 
>   samza-kv/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala 4856be0a92c6c50cfee800e659cad9cc7b05f4d5 
>   samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 0077af08ac33bd95b1d40c362f95dfd6208cf99c 
>   settings.gradle 6ba628059a3168abf80d072d68942f79bd237811 
> 
> Diff: https://reviews.apache.org/r/23372/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chinmay Soman
> 
>


Re: Review Request 23372: SAMZA-256: Provide in-memory data store implementation

Posted by Chinmay Soman <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23372/
-----------------------------------------------------------

(Updated July 10, 2014, 6:46 p.m.)


Review request for samza.


Repository: samza


Description (updated)
-------

SAMZA-256: Review comments (2) : Merging LevelDb and KeyValue StorageEngine factories


SAMZA-256: Addressing review comments


SAMZA-256: Adding missing KeyValueStorageEngineFactory.scala


SAMZA-256: Provide in-memory data store implementation


Diffs (updated)
-----

  build.gradle c853fc067ab4d23d9fb215b349b8e7f3ca1c4dad 
  gradle/dependency-versions.gradle 078725806f5d41066c6f6a63885d4f6acce8d23c 
  samza-kv-inmemory/src/main/scala/org/apache/samza/storage/kv/inmemory/InMemoryKeyValueStore.scala PRE-CREATION 
  samza-kv-leveldb/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStorageEngineFactory.scala PRE-CREATION 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala PRE-CREATION 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStore.scala 429f51a4d269e3d18627c183359af672c68b8b00 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStoreMetrics.scala 8ffcfb194b981a798fd19b05f9e6bff9fc2f79e2 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngine.scala f42ea026cd4a2ae633d71149af6dacef68dab352 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineFactory.scala 36b7e97b427d3d0fd18f38609e2dac2b081a8dbc 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineMetrics.scala a7958f62582a2cc998f026c6818afd24936d46d4 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStore.scala 72562cfcaccb99e35f84df87045271473bd82b04 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStoreMetrics.scala 8949f2fb6d8488e5e061139299e9ea97220ea180 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStoreMetrics.scala 8949f2fb6d8488e5e061139299e9ea97220ea180 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStoreMetrics.scala 8949f2fb6d8488e5e061139299e9ea97220ea180 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/LoggedStore.scala d3c1ae84b56827964ce10d2aa9dedcdc7221c596 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/LoggedStoreMetrics.scala ea56e8c2e41efa0c59bedac9e489b399fc8813e4 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/NullSafeKeyValueStore.scala 00f9af319c780d9513b735016b1c3e016714f5c8 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/SerializedKeyValueStore.scala 2d3b6e55c20172aa5f149c6d6728cab8229312c3 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/SerializedKeyValueStoreMetrics.scala 2ad21c822f3d9c08dcc1e78d4b86c3fd314b7ed2 
  samza-kv/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala 4856be0a92c6c50cfee800e659cad9cc7b05f4d5 
  samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 0077af08ac33bd95b1d40c362f95dfd6208cf99c 
  settings.gradle 6ba628059a3168abf80d072d68942f79bd237811 

Diff: https://reviews.apache.org/r/23372/diff/


Testing
-------


Thanks,

Chinmay Soman


Re: Review Request 23372: SAMZA-256: Provide in-memory data store implementation

Posted by Chinmay Soman <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23372/
-----------------------------------------------------------

(Updated July 10, 2014, 6:45 p.m.)


Review request for samza.


Repository: samza


Description
-------

SAMZA-256: Addressing review comments


SAMZA-256: Adding missing KeyValueStorageEngineFactory.scala


SAMZA-256: Provide in-memory data store implementation


Diffs (updated)
-----

  build.gradle c853fc067ab4d23d9fb215b349b8e7f3ca1c4dad 
  gradle/dependency-versions.gradle 078725806f5d41066c6f6a63885d4f6acce8d23c 
  samza-kv-inmemory/src/main/scala/org/apache/samza/storage/kv/inmemory/InMemoryKeyValueStore.scala PRE-CREATION 
  samza-kv-leveldb/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineFactory.scala PRE-CREATION 
  samza-kv-leveldb/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStorageEngineFactory.scala PRE-CREATION 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala PRE-CREATION 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStore.scala 429f51a4d269e3d18627c183359af672c68b8b00 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStoreMetrics.scala 8ffcfb194b981a798fd19b05f9e6bff9fc2f79e2 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngine.scala f42ea026cd4a2ae633d71149af6dacef68dab352 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineFactory.scala 36b7e97b427d3d0fd18f38609e2dac2b081a8dbc 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineMetrics.scala a7958f62582a2cc998f026c6818afd24936d46d4 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStore.scala 72562cfcaccb99e35f84df87045271473bd82b04 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStoreMetrics.scala 8949f2fb6d8488e5e061139299e9ea97220ea180 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStoreMetrics.scala 8949f2fb6d8488e5e061139299e9ea97220ea180 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/LoggedStore.scala d3c1ae84b56827964ce10d2aa9dedcdc7221c596 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/LoggedStoreMetrics.scala ea56e8c2e41efa0c59bedac9e489b399fc8813e4 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/NullSafeKeyValueStore.scala 00f9af319c780d9513b735016b1c3e016714f5c8 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/SerializedKeyValueStore.scala 2d3b6e55c20172aa5f149c6d6728cab8229312c3 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/SerializedKeyValueStoreMetrics.scala 2ad21c822f3d9c08dcc1e78d4b86c3fd314b7ed2 
  samza-kv/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala 4856be0a92c6c50cfee800e659cad9cc7b05f4d5 
  samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 0077af08ac33bd95b1d40c362f95dfd6208cf99c 
  settings.gradle 6ba628059a3168abf80d072d68942f79bd237811 

Diff: https://reviews.apache.org/r/23372/diff/


Testing
-------


Thanks,

Chinmay Soman


Re: Review Request 23372: SAMZA-256: Provide in-memory data store implementation

Posted by Chinmay Soman <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23372/
-----------------------------------------------------------

(Updated July 10, 2014, 12:55 a.m.)


Review request for samza.


Repository: samza


Description (updated)
-------

SAMZA-256: Addressing review comments


SAMZA-256: Adding missing KeyValueStorageEngineFactory.scala


SAMZA-256: Provide in-memory data store implementation


Diffs (updated)
-----

  build.gradle c853fc067ab4d23d9fb215b349b8e7f3ca1c4dad 
  gradle/dependency-versions.gradle 078725806f5d41066c6f6a63885d4f6acce8d23c 
  samza-kv-inmemory/src/main/scala/org/apache/samza/storage/kv/inmemory/InMemoryKeyValueStore.scala PRE-CREATION 
  samza-kv-leveldb/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineFactory.scala PRE-CREATION 
  samza-kv-leveldb/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStorageEngineFactory.scala PRE-CREATION 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala PRE-CREATION 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStore.scala 429f51a4d269e3d18627c183359af672c68b8b00 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStoreMetrics.scala 8ffcfb194b981a798fd19b05f9e6bff9fc2f79e2 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngine.scala f42ea026cd4a2ae633d71149af6dacef68dab352 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineFactory.scala 36b7e97b427d3d0fd18f38609e2dac2b081a8dbc 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineMetrics.scala a7958f62582a2cc998f026c6818afd24936d46d4 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStore.scala 72562cfcaccb99e35f84df87045271473bd82b04 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStoreMetrics.scala 8949f2fb6d8488e5e061139299e9ea97220ea180 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStoreMetrics.scala 8949f2fb6d8488e5e061139299e9ea97220ea180 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/LoggedStore.scala d3c1ae84b56827964ce10d2aa9dedcdc7221c596 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/LoggedStoreMetrics.scala ea56e8c2e41efa0c59bedac9e489b399fc8813e4 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/NullSafeKeyValueStore.scala 00f9af319c780d9513b735016b1c3e016714f5c8 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/SerializedKeyValueStore.scala 2d3b6e55c20172aa5f149c6d6728cab8229312c3 
  samza-kv/src/main/scala/org/apache/samza/storage/kv/SerializedKeyValueStoreMetrics.scala 2ad21c822f3d9c08dcc1e78d4b86c3fd314b7ed2 
  samza-kv/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala 4856be0a92c6c50cfee800e659cad9cc7b05f4d5 
  samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 0077af08ac33bd95b1d40c362f95dfd6208cf99c 
  settings.gradle 6ba628059a3168abf80d072d68942f79bd237811 

Diff: https://reviews.apache.org/r/23372/diff/


Testing
-------


Thanks,

Chinmay Soman


Re: Review Request 23372: SAMZA-256: Provide in-memory data store implementation

Posted by Chinmay Soman <ch...@gmail.com>.

> On July 9, 2014, 7:28 p.m., Chris Riccomini wrote:
> > Overall, looks pretty good to me.
> > 
> > I think you had mentioned you had some thoughts on refactoring the getStorageEngine API. Are you planning on a follow-on JIRA for that?

Yes, I'll open a new ticket for that. 


> On July 9, 2014, 7:28 p.m., Chris Riccomini wrote:
> > build.gradle, line 77
> > <https://reviews.apache.org/r/23372/diff/1/?file=627052#file627052line77>
> >
> >     If memory requires guava, can we move it into it out of core?

Moved to samza-kv-inmemory


> On July 9, 2014, 7:28 p.m., Chris Riccomini wrote:
> > samza-core/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala, line 1
> > <https://reviews.apache.org/r/23372/diff/1/?file=627054#file627054line1>
> >
> >     License header.

Will do.


> On July 9, 2014, 7:28 p.m., Chris Riccomini wrote:
> > samza-core/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala, line 13
> > <https://reviews.apache.org/r/23372/diff/1/?file=627054#file627054line13>
> >
> >     Javadoc.

Will Do.


> On July 9, 2014, 7:28 p.m., Chris Riccomini wrote:
> > samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStoreMetrics.scala, line 27
> > <https://reviews.apache.org/r/23372/diff/1/?file=627066#file627066line27>
> >
> >     This seems like a copy/paste of LevelDbKeyValueStoreMetrics. Can we just move this into core as a generalized metrics class?
> >     
> >     Going one step further, we could make a wrapper around the KV store in BaseKeyValueStorageEngineFactory, which would handle these metrics for all underlying KV stores. I imagine we'll want these metrics whether it's in mem, level db, or rocksdb.

Moved it to samza-kv


> On July 9, 2014, 7:28 p.m., Chris Riccomini wrote:
> > samza-kv/src/main/scala/org/apache/samza/storage/kv/NullSafeKeyValueStore.scala, line 32
> > <https://reviews.apache.org/r/23372/diff/1/?file=627070#file627070line32>
> >
> >     Do we need the fully qualified path?

Cleaned up (I blame IntelliJ for doing this :) )


> On July 9, 2014, 7:28 p.m., Chris Riccomini wrote:
> > samza-kv/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala, line 42
> > <https://reviews.apache.org/r/23372/diff/1/?file=627073#file627073line42>
> >
> >     Could you doc a quick note on what these params are now? It was easy enough when we only tested LDB, but with in mem, I'm a bit confused at first glance.

Done


- Chinmay


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


On July 9, 2014, 5:47 p.m., Chinmay Soman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23372/
> -----------------------------------------------------------
> 
> (Updated July 9, 2014, 5:47 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> SAMZA-256: Provide in-memory data store implementation
> 
> 
> Diffs
> -----
> 
>   build.gradle f1a458b8c4cbe6e38f28ba3c5bd9099590c7abde 
>   gradle/dependency-versions-scala-2.10.gradle 7094d6cf86b9da454633525dc5da65e5a9f90f4c 
>   samza-core/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/storage/kv/inmemory/InMemoryKeyValueStore.scala PRE-CREATION 
>   samza-kv/src/main/java/org/apache/samza/storage/kv/Entry.java  
>   samza-kv/src/main/java/org/apache/samza/storage/kv/KeyValueIterator.java  
>   samza-kv/src/main/java/org/apache/samza/storage/kv/KeyValueStore.java  
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStore.scala 429f51a4d269e3d18627c183359af672c68b8b00 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStoreMetrics.scala 8ffcfb194b981a798fd19b05f9e6bff9fc2f79e2 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStoreMetrics.scala 8ffcfb194b981a798fd19b05f9e6bff9fc2f79e2 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngine.scala f42ea026cd4a2ae633d71149af6dacef68dab352 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineFactory.scala 36b7e97b427d3d0fd18f38609e2dac2b081a8dbc 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineMetrics.scala a7958f62582a2cc998f026c6818afd24936d46d4 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStore.scala dae3c2c8570d4add7bc3c01f0fb73c867c609740 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStoreMetrics.scala 8949f2fb6d8488e5e061139299e9ea97220ea180 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStoreMetrics.scala  
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LoggedStore.scala d3c1ae84b56827964ce10d2aa9dedcdc7221c596 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LoggedStoreMetrics.scala ea56e8c2e41efa0c59bedac9e489b399fc8813e4 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/NullSafeKeyValueStore.scala 00f9af319c780d9513b735016b1c3e016714f5c8 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/SerializedKeyValueStore.scala 2d3b6e55c20172aa5f149c6d6728cab8229312c3 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/SerializedKeyValueStoreMetrics.scala 2ad21c822f3d9c08dcc1e78d4b86c3fd314b7ed2 
>   samza-kv/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala ec23567570e2d7868390f570bd65d8cd7b18d4dc 
>   samza-leveldb/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineFactory.scala PRE-CREATION 
>   samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 0077af08ac33bd95b1d40c362f95dfd6208cf99c 
>   settings.gradle 6ba628059a3168abf80d072d68942f79bd237811 
> 
> Diff: https://reviews.apache.org/r/23372/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chinmay Soman
> 
>


Re: Review Request 23372: SAMZA-256: Provide in-memory data store implementation

Posted by Chris Riccomini <cr...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23372/#review47505
-----------------------------------------------------------


Overall, looks pretty good to me.

I think you had mentioned you had some thoughts on refactoring the getStorageEngine API. Are you planning on a follow-on JIRA for that?


build.gradle
<https://reviews.apache.org/r/23372/#comment83404>

    If memory requires guava, can we move it into it out of core?



samza-core/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala
<https://reviews.apache.org/r/23372/#comment83408>

    License header.



samza-core/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala
<https://reviews.apache.org/r/23372/#comment83409>

    Javadoc.



samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStoreMetrics.scala
<https://reviews.apache.org/r/23372/#comment83417>

    This seems like a copy/paste of LevelDbKeyValueStoreMetrics. Can we just move this into core as a generalized metrics class?
    
    Going one step further, we could make a wrapper around the KV store in BaseKeyValueStorageEngineFactory, which would handle these metrics for all underlying KV stores. I imagine we'll want these metrics whether it's in mem, level db, or rocksdb.



samza-kv/src/main/scala/org/apache/samza/storage/kv/NullSafeKeyValueStore.scala
<https://reviews.apache.org/r/23372/#comment83420>

    Do we need the fully qualified path?



samza-kv/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala
<https://reviews.apache.org/r/23372/#comment83421>

    Could you doc a quick note on what these params are now? It was easy enough when we only tested LDB, but with in mem, I'm a bit confused at first glance.



samza-kv/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala
<https://reviews.apache.org/r/23372/#comment83422>

    Just curious: how can dir be null here? Looks like it's defined at instance level in the class.


- Chris Riccomini


On July 9, 2014, 5:47 p.m., Chinmay Soman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23372/
> -----------------------------------------------------------
> 
> (Updated July 9, 2014, 5:47 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> SAMZA-256: Provide in-memory data store implementation
> 
> 
> Diffs
> -----
> 
>   build.gradle f1a458b8c4cbe6e38f28ba3c5bd9099590c7abde 
>   gradle/dependency-versions-scala-2.10.gradle 7094d6cf86b9da454633525dc5da65e5a9f90f4c 
>   samza-core/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/storage/kv/inmemory/InMemoryKeyValueStore.scala PRE-CREATION 
>   samza-kv/src/main/java/org/apache/samza/storage/kv/Entry.java  
>   samza-kv/src/main/java/org/apache/samza/storage/kv/KeyValueIterator.java  
>   samza-kv/src/main/java/org/apache/samza/storage/kv/KeyValueStore.java  
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStore.scala 429f51a4d269e3d18627c183359af672c68b8b00 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStoreMetrics.scala 8ffcfb194b981a798fd19b05f9e6bff9fc2f79e2 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStoreMetrics.scala 8ffcfb194b981a798fd19b05f9e6bff9fc2f79e2 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngine.scala f42ea026cd4a2ae633d71149af6dacef68dab352 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineFactory.scala 36b7e97b427d3d0fd18f38609e2dac2b081a8dbc 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineMetrics.scala a7958f62582a2cc998f026c6818afd24936d46d4 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStore.scala dae3c2c8570d4add7bc3c01f0fb73c867c609740 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStoreMetrics.scala 8949f2fb6d8488e5e061139299e9ea97220ea180 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStoreMetrics.scala  
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LoggedStore.scala d3c1ae84b56827964ce10d2aa9dedcdc7221c596 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/LoggedStoreMetrics.scala ea56e8c2e41efa0c59bedac9e489b399fc8813e4 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/NullSafeKeyValueStore.scala 00f9af319c780d9513b735016b1c3e016714f5c8 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/SerializedKeyValueStore.scala 2d3b6e55c20172aa5f149c6d6728cab8229312c3 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/SerializedKeyValueStoreMetrics.scala 2ad21c822f3d9c08dcc1e78d4b86c3fd314b7ed2 
>   samza-kv/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala ec23567570e2d7868390f570bd65d8cd7b18d4dc 
>   samza-leveldb/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineFactory.scala PRE-CREATION 
>   samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 0077af08ac33bd95b1d40c362f95dfd6208cf99c 
>   settings.gradle 6ba628059a3168abf80d072d68942f79bd237811 
> 
> Diff: https://reviews.apache.org/r/23372/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chinmay Soman
> 
>