You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@samza.apache.org by Naveen Somasundaram <na...@gmail.com> on 2014/09/26 01:16:51 UTC

Review Request 26059: Adding RocksDB key value store

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

Review request for samza.


Bugs: SAMZA-236
    https://issues.apache.org/jira/browse/SAMZA-236


Repository: samza


Description
-------

Adding RocksDB Key-value support


Diffs
-----

  build.gradle 357bb0e312c3812b55303847163f696f7c52ad78 
  samza-kv-leveldb/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala d6d437ab7a8b18002dbeb54d85c6c2b5eb43784c 
  samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStorageEngineFactory.scala PRE-CREATION 
  samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala PRE-CREATION 
  samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/TestRocksDbApi.scala PRE-CREATION 
  samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 7d0b8db0c3bf1e70fd6af03abb594c7000e6666b 
  settings.gradle 325cac27389093d87eb9e36d475e234ed52510ec 

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


Testing
-------

Unit testing


Thanks,

Naveen Somasundaram


Re: Review Request 26059: Adding RocksDB key value store

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

> On Sept. 26, 2014, 8:13 p.m., Chris Riccomini wrote:
> > build.gradle, line 342
> > <https://reviews.apache.org/r/26059/diff/1/?file=705775#file705775line342>
> >
> >     Seems odd that samza-kv-leveldb depends on inmemory and rocksdb. Maybe Chinmay can comment.

This is an artifact of the TestKeyValueStores not being moved to samza-kv. This is tracked here: https://issues.apache.org/jira/browse/SAMZA-346


- Chinmay


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


On Sept. 25, 2014, 11:17 p.m., Naveen Somasundaram wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26059/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2014, 11:17 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Bugs: SAMZA-236
>     https://issues.apache.org/jira/browse/SAMZA-236
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> Adding RocksDB Key-value support
> 
> 
> Diffs
> -----
> 
>   build.gradle 357bb0e312c3812b55303847163f696f7c52ad78 
>   samza-kv-leveldb/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala d6d437ab7a8b18002dbeb54d85c6c2b5eb43784c 
>   samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStorageEngineFactory.scala PRE-CREATION 
>   samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala PRE-CREATION 
>   samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/TestRocksDbApi.scala PRE-CREATION 
>   samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 7d0b8db0c3bf1e70fd6af03abb594c7000e6666b 
>   settings.gradle 325cac27389093d87eb9e36d475e234ed52510ec 
> 
> Diff: https://reviews.apache.org/r/26059/diff/
> 
> 
> Testing
> -------
> 
> Unit testing
> 
> 
> Thanks,
> 
> Naveen Somasundaram
> 
>


Re: Review Request 26059: Adding RocksDB key value store

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



build.gradle
<https://reviews.apache.org/r/26059/#comment95002>

    Seems odd that samza-kv-leveldb depends on inmemory and rocksdb. Maybe Chinmay can comment.


- Chris Riccomini


On Sept. 25, 2014, 11:17 p.m., Naveen Somasundaram wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26059/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2014, 11:17 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Bugs: SAMZA-236
>     https://issues.apache.org/jira/browse/SAMZA-236
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> Adding RocksDB Key-value support
> 
> 
> Diffs
> -----
> 
>   build.gradle 357bb0e312c3812b55303847163f696f7c52ad78 
>   samza-kv-leveldb/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala d6d437ab7a8b18002dbeb54d85c6c2b5eb43784c 
>   samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStorageEngineFactory.scala PRE-CREATION 
>   samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala PRE-CREATION 
>   samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/TestRocksDbApi.scala PRE-CREATION 
>   samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 7d0b8db0c3bf1e70fd6af03abb594c7000e6666b 
>   settings.gradle 325cac27389093d87eb9e36d475e234ed52510ec 
> 
> Diff: https://reviews.apache.org/r/26059/diff/
> 
> 
> Testing
> -------
> 
> Unit testing
> 
> 
> Thanks,
> 
> Naveen Somasundaram
> 
>


Re: Review Request 26059: Adding RocksDB key value store

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



build.gradle
<https://reviews.apache.org/r/26059/#comment94929>

    We aren't using grizzled any more. Have you rebased against master?
    
    Also, I'd recommend installing rbt, so you can just do rbt post rather than manually uploading RBs.



samza-kv-leveldb/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala
<https://reviews.apache.org/r/26059/#comment94930>

    Nit: can we just do one-per-line here.



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStorageEngineFactory.scala
<https://reviews.apache.org/r/26059/#comment94931>

    License header.



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStorageEngineFactory.scala
<https://reviews.apache.org/r/26059/#comment94933>

    Nit: single line spacing.



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStorageEngineFactory.scala
<https://reviews.apache.org/r/26059/#comment94932>

    Nit: single line spacing.



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStorageEngineFactory.scala
<https://reviews.apache.org/r/26059/#comment94938>

    We should delete this. It was a hack for LevelDB's .all() latency issues that we were seeing. It didn't really seem to work all that well.



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment94955>

    License.



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment94939>

    Should be able to do "RocksDbKeyValueStore with Logging", and then just call warn().



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment94942>

    Do we need to support the other compressions that RocksDB supports (e.g. bzip2)?



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment94943>

    Seems like these should be configurable: compaction style, filter, write buff num, cache size.



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment94945>

    Delete. See comment above.



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment94946>

    Nit: options, dir.toString (space)



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment94947>

    Nit: remove double space.



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment94948>

    because of?



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment94949>

    Is this true? I know it's not supported for LevelDB. Wonder if RocksDB added it?



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment94950>

    Remove //iter.remove();



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment94951>

    Clean this up.



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment94952>

    This looks like it was cargo culted in from LevelDB's implementation. Can we put this in one place, and share between the two? Either samza-kv or samza-core's util.



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment94953>

    Don't need these. They're from DBComparator, which is LevelDB's class.
    
    Seems we should make LexicographicaCompartor a shared class, and have LevelDB extend it with DBCompartor to add these two methods.



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/TestRocksDbApi.scala
<https://reviews.apache.org/r/26059/#comment94954>

    License.



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/TestRocksDbApi.scala
<https://reviews.apache.org/r/26059/#comment94956>

    Docs.
    
    Move to samza-test. I've been trying to keep CLI-based test tools isolated there.
    
    Alternatively, re-write as a Unit test.


- Chris Riccomini


On Sept. 25, 2014, 11:17 p.m., Naveen Somasundaram wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26059/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2014, 11:17 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Bugs: SAMZA-236
>     https://issues.apache.org/jira/browse/SAMZA-236
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> Adding RocksDB Key-value support
> 
> 
> Diffs
> -----
> 
>   build.gradle 357bb0e312c3812b55303847163f696f7c52ad78 
>   samza-kv-leveldb/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala d6d437ab7a8b18002dbeb54d85c6c2b5eb43784c 
>   samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStorageEngineFactory.scala PRE-CREATION 
>   samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala PRE-CREATION 
>   samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/TestRocksDbApi.scala PRE-CREATION 
>   samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 7d0b8db0c3bf1e70fd6af03abb594c7000e6666b 
>   settings.gradle 325cac27389093d87eb9e36d475e234ed52510ec 
> 
> Diff: https://reviews.apache.org/r/26059/diff/
> 
> 
> Testing
> -------
> 
> Unit testing
> 
> 
> Thanks,
> 
> Naveen Somasundaram
> 
>


Re: Review Request 26059: Adding RocksDB key value store

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



build.gradle
<https://reviews.apache.org/r/26059/#comment95013>

    Why do you need to depend on in-memory in RocksDB ?


- Chinmay Soman


On Sept. 25, 2014, 11:17 p.m., Naveen Somasundaram wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26059/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2014, 11:17 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Bugs: SAMZA-236
>     https://issues.apache.org/jira/browse/SAMZA-236
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> Adding RocksDB Key-value support
> 
> 
> Diffs
> -----
> 
>   build.gradle 357bb0e312c3812b55303847163f696f7c52ad78 
>   samza-kv-leveldb/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala d6d437ab7a8b18002dbeb54d85c6c2b5eb43784c 
>   samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStorageEngineFactory.scala PRE-CREATION 
>   samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala PRE-CREATION 
>   samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/TestRocksDbApi.scala PRE-CREATION 
>   samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 7d0b8db0c3bf1e70fd6af03abb594c7000e6666b 
>   settings.gradle 325cac27389093d87eb9e36d475e234ed52510ec 
> 
> Diff: https://reviews.apache.org/r/26059/diff/
> 
> 
> Testing
> -------
> 
> Unit testing
> 
> 
> Thanks,
> 
> Naveen Somasundaram
> 
>


Re: Review Request 26059: Adding RocksDB key value store

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



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment95016>

    Cosmetic suggestion:
    
    use a val for containerContext.taskNames.size



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment95017>

    Cosmetic: Remove comment



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment95019>

    Cosmetic: Delete comment



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment95020>

    Same as above



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment95021>

    Why do we have to do this explicitly ? (does newIterator do some level of caching ?



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment95022>

    Cosmetic: Indentation



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment95023>

    Cosmetic: change to log / trace



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/TestRocksDbApi.scala
<https://reviews.apache.org/r/26059/#comment95024>

    You could use the Java style of creating temp directories:
    
    File.createTempFile...



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/TestRocksDbApi.scala
<https://reviews.apache.org/r/26059/#comment95026>

    Can you provide a brief doc or a link to some open source page where these options are explained ? That'll be great for noobs like me :)


- Chinmay Soman


On Sept. 25, 2014, 11:17 p.m., Naveen Somasundaram wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26059/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2014, 11:17 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Bugs: SAMZA-236
>     https://issues.apache.org/jira/browse/SAMZA-236
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> Adding RocksDB Key-value support
> 
> 
> Diffs
> -----
> 
>   build.gradle 357bb0e312c3812b55303847163f696f7c52ad78 
>   samza-kv-leveldb/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala d6d437ab7a8b18002dbeb54d85c6c2b5eb43784c 
>   samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStorageEngineFactory.scala PRE-CREATION 
>   samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala PRE-CREATION 
>   samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/TestRocksDbApi.scala PRE-CREATION 
>   samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 7d0b8db0c3bf1e70fd6af03abb594c7000e6666b 
>   settings.gradle 325cac27389093d87eb9e36d475e234ed52510ec 
> 
> Diff: https://reviews.apache.org/r/26059/diff/
> 
> 
> Testing
> -------
> 
> Unit testing
> 
> 
> Thanks,
> 
> Naveen Somasundaram
> 
>


Re: Review Request 26059: Adding RocksDB key value store

Posted by Naveen Somasundaram <na...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26059/
-----------------------------------------------------------

(Updated Oct. 16, 2014, 2:27 a.m.)


Review request for samza.


Bugs: SAMZA-236
    https://issues.apache.org/jira/browse/SAMZA-236


Repository: samza


Description
-------

Adding RocksDB Key-value support


Diffs (updated)
-----

  build.gradle 1b37dbb4b9c74bbf8556a82da4e39ee7f3490dd6 
  gradle/dependency-versions.gradle fe2e446e0f9d0d89ec3f522a43d0acf921ae3801 
  samza-core/src/main/scala/org/apache/samza/util/LexicographicComparator.scala PRE-CREATION 
  samza-kv-leveldb/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStore.scala 853de121358c24c29111670c57423603acd09a7c 
  samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStorageEngineFactory.scala PRE-CREATION 
  samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala PRE-CREATION 
  samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 8fd33f1d4c25737bf6fb52627935f9157983b4e6 
  samza-test/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala eefe11494f4bd0a1710627614a9ab6bc2ee844d8 
  settings.gradle 325cac27389093d87eb9e36d475e234ed52510ec 

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


Testing
-------

Unit testing


Thanks,

Naveen Somasundaram


Re: Review Request 26059: Adding RocksDB key value store

Posted by Naveen Somasundaram <na...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26059/#review55169
-----------------------------------------------------------



build.gradle
<https://reviews.apache.org/r/26059/#comment95552>

    Discussed with Chinmay, I just copied it from LevelDB (where a test depends on it), but for RocksDB it's not necessary.



build.gradle
<https://reviews.apache.org/r/26059/#comment95554>

    I rebased against the master, removing dependency.
    
    Thanks, will give rbt a shot :)



samza-kv-leveldb/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala
<https://reviews.apache.org/r/26059/#comment95555>

    done!



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStorageEngineFactory.scala
<https://reviews.apache.org/r/26059/#comment97234>

    done!



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStorageEngineFactory.scala
<https://reviews.apache.org/r/26059/#comment97232>

    done!



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStorageEngineFactory.scala
<https://reviews.apache.org/r/26059/#comment97235>

    done!



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStorageEngineFactory.scala
<https://reviews.apache.org/r/26059/#comment97236>

    deleted this



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment97237>

    done



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment97238>

    done



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment97239>

    done



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment97240>

    I am exposing all of them for now, we can play with it later.



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment97262>

    Some of the configuration options have changed, I have changed them and also exposed configurations to change them.



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment97243>

    I have removed it. I was overriding it for testing.



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment97263>

    addressed



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment97264>

    addressed



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment97266>

    done



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment97267>

    pointed to the bug :)



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment97268>

    removed all comments



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment97269>

    By default, the iterator points to null, seekToFirst points it to the right position.
    https://github.com/facebook/rocksdb/blob/c8e70e6bf862f589d2b38a95bac3f03206d44ba8/table/iterator.cc



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment97273>

    I did some research and it's actually supported, but I don't see it exposed in Java :( I'll open a ticket for them.
    
    It's exposed in perl:
    http://search.cpan.org/~jiro/RocksDB-0.02/lib/RocksDB.pm



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment97274>

    fixed



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment97275>

    done



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment97276>

    done



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment97298>

    Moved it to samza-core and extended to use it both on leveldb and rocksdb as dicusssed offline.



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/TestRocksDbApi.scala
<https://reviews.apache.org/r/26059/#comment97297>

    I am going to get rid of this file, I just created this purely for testing the API.


- Naveen Somasundaram


On Oct. 16, 2014, 2:16 a.m., Naveen Somasundaram wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26059/
> -----------------------------------------------------------
> 
> (Updated Oct. 16, 2014, 2:16 a.m.)
> 
> 
> Review request for samza.
> 
> 
> Bugs: SAMZA-236
>     https://issues.apache.org/jira/browse/SAMZA-236
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> Adding RocksDB Key-value support
> 
> 
> Diffs
> -----
> 
>   build.gradle 1b37dbb4b9c74bbf8556a82da4e39ee7f3490dd6 
>   gradle/dependency-versions.gradle fe2e446e0f9d0d89ec3f522a43d0acf921ae3801 
>   samza-core/src/main/scala/org/apache/samza/util/LexicographicComparator.scala PRE-CREATION 
>   samza-kv-leveldb/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStore.scala 853de121358c24c29111670c57423603acd09a7c 
>   samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStorageEngineFactory.scala PRE-CREATION 
>   samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala PRE-CREATION 
>   samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 8fd33f1d4c25737bf6fb52627935f9157983b4e6 
>   samza-test/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala eefe11494f4bd0a1710627614a9ab6bc2ee844d8 
>   settings.gradle 325cac27389093d87eb9e36d475e234ed52510ec 
> 
> Diff: https://reviews.apache.org/r/26059/diff/
> 
> 
> Testing
> -------
> 
> Unit testing
> 
> 
> Thanks,
> 
> Naveen Somasundaram
> 
>


Re: Review Request 26059: Adding RocksDB key value store

Posted by Naveen Somasundaram <na...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26059/
-----------------------------------------------------------

(Updated Oct. 16, 2014, 2:16 a.m.)


Review request for samza.


Changes
-------

Addressing review comments


Bugs: SAMZA-236
    https://issues.apache.org/jira/browse/SAMZA-236


Repository: samza


Description
-------

Adding RocksDB Key-value support


Diffs (updated)
-----

  build.gradle 1b37dbb4b9c74bbf8556a82da4e39ee7f3490dd6 
  gradle/dependency-versions.gradle fe2e446e0f9d0d89ec3f522a43d0acf921ae3801 
  samza-core/src/main/scala/org/apache/samza/util/LexicographicComparator.scala PRE-CREATION 
  samza-kv-leveldb/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStore.scala 853de121358c24c29111670c57423603acd09a7c 
  samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStorageEngineFactory.scala PRE-CREATION 
  samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala PRE-CREATION 
  samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 8fd33f1d4c25737bf6fb52627935f9157983b4e6 
  samza-test/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala eefe11494f4bd0a1710627614a9ab6bc2ee844d8 
  settings.gradle 325cac27389093d87eb9e36d475e234ed52510ec 

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


Testing
-------

Unit testing


Thanks,

Naveen Somasundaram


Re: Review Request 26059: Adding RocksDB key value store

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



samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
<https://reviews.apache.org/r/26059/#comment95015>

    Is this 'setCacheSize' different from the one called above ? 
    
        options.setCacheSize(cacheSize / containerContext.taskNames.size) ?


- Chinmay Soman


On Sept. 25, 2014, 11:17 p.m., Naveen Somasundaram wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26059/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2014, 11:17 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Bugs: SAMZA-236
>     https://issues.apache.org/jira/browse/SAMZA-236
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> Adding RocksDB Key-value support
> 
> 
> Diffs
> -----
> 
>   build.gradle 357bb0e312c3812b55303847163f696f7c52ad78 
>   samza-kv-leveldb/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala d6d437ab7a8b18002dbeb54d85c6c2b5eb43784c 
>   samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStorageEngineFactory.scala PRE-CREATION 
>   samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala PRE-CREATION 
>   samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/TestRocksDbApi.scala PRE-CREATION 
>   samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 7d0b8db0c3bf1e70fd6af03abb594c7000e6666b 
>   settings.gradle 325cac27389093d87eb9e36d475e234ed52510ec 
> 
> Diff: https://reviews.apache.org/r/26059/diff/
> 
> 
> Testing
> -------
> 
> Unit testing
> 
> 
> Thanks,
> 
> Naveen Somasundaram
> 
>


Re: Review Request 26059: Adding RocksDB key value store

Posted by Naveen Somasundaram <na...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26059/
-----------------------------------------------------------

(Updated Sept. 25, 2014, 11:17 p.m.)


Review request for samza.


Bugs: SAMZA-236
    https://issues.apache.org/jira/browse/SAMZA-236


Repository: samza


Description
-------

Adding RocksDB Key-value support


Diffs
-----

  build.gradle 357bb0e312c3812b55303847163f696f7c52ad78 
  samza-kv-leveldb/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala d6d437ab7a8b18002dbeb54d85c6c2b5eb43784c 
  samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStorageEngineFactory.scala PRE-CREATION 
  samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala PRE-CREATION 
  samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/TestRocksDbApi.scala PRE-CREATION 
  samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 7d0b8db0c3bf1e70fd6af03abb594c7000e6666b 
  settings.gradle 325cac27389093d87eb9e36d475e234ed52510ec 

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


Testing
-------

Unit testing


Thanks,

Naveen Somasundaram


Re: Review Request 26059: Adding RocksDB key value store

Posted by Naveen Somasundaram <na...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26059/
-----------------------------------------------------------

(Updated Sept. 25, 2014, 11:17 p.m.)


Review request for samza.


Bugs: SAMZA-236
    https://issues.apache.org/jira/browse/SAMZA-236


Repository: samza


Description
-------

Adding RocksDB Key-value support


Diffs
-----

  build.gradle 357bb0e312c3812b55303847163f696f7c52ad78 
  samza-kv-leveldb/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala d6d437ab7a8b18002dbeb54d85c6c2b5eb43784c 
  samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStorageEngineFactory.scala PRE-CREATION 
  samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala PRE-CREATION 
  samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/TestRocksDbApi.scala PRE-CREATION 
  samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 7d0b8db0c3bf1e70fd6af03abb594c7000e6666b 
  settings.gradle 325cac27389093d87eb9e36d475e234ed52510ec 

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


Testing
-------

Unit testing


Thanks,

Naveen Somasundaram


Re: Review Request 26059: Adding RocksDB key value store

Posted by Naveen Somasundaram <na...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26059/
-----------------------------------------------------------

(Updated Sept. 25, 2014, 11:17 p.m.)


Review request for samza.


Bugs: SAMZA-236
    https://issues.apache.org/jira/browse/SAMZA-236


Repository: samza


Description
-------

Adding RocksDB Key-value support


Diffs
-----

  build.gradle 357bb0e312c3812b55303847163f696f7c52ad78 
  samza-kv-leveldb/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala d6d437ab7a8b18002dbeb54d85c6c2b5eb43784c 
  samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStorageEngineFactory.scala PRE-CREATION 
  samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala PRE-CREATION 
  samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/TestRocksDbApi.scala PRE-CREATION 
  samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 7d0b8db0c3bf1e70fd6af03abb594c7000e6666b 
  settings.gradle 325cac27389093d87eb9e36d475e234ed52510ec 

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


Testing
-------

Unit testing


Thanks,

Naveen Somasundaram


Re: Review Request 26059: Adding RocksDB key value store

Posted by Naveen Somasundaram <na...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26059/
-----------------------------------------------------------

(Updated Sept. 25, 2014, 11:17 p.m.)


Review request for samza.


Bugs: SAMZA-236
    https://issues.apache.org/jira/browse/SAMZA-236


Repository: samza


Description
-------

Adding RocksDB Key-value support


Diffs
-----

  build.gradle 357bb0e312c3812b55303847163f696f7c52ad78 
  samza-kv-leveldb/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala d6d437ab7a8b18002dbeb54d85c6c2b5eb43784c 
  samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStorageEngineFactory.scala PRE-CREATION 
  samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala PRE-CREATION 
  samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/TestRocksDbApi.scala PRE-CREATION 
  samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 7d0b8db0c3bf1e70fd6af03abb594c7000e6666b 
  settings.gradle 325cac27389093d87eb9e36d475e234ed52510ec 

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


Testing
-------

Unit testing


Thanks,

Naveen Somasundaram