Re: Review Request 33146: New KeyValueStore Features

2015-05-04 Thread Yi Pan (Data Infrastructure)
> On May 4, 2015, 8:14 p.m., Yi Pan (Data Infrastructure) wrote: > > samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala, > > line 320 > > > > > > This is a bit confusing to me:

Re: Review Request 33146: New KeyValueStore Features

2015-05-04 Thread Yi Pan (Data Infrastructure)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33146/#review82432 --- Ship it! Ship It! - Yi Pan (Data Infrastructure) On May 4, 2015,

Re: Review Request 33146: New KeyValueStore Features

2015-05-04 Thread Mohamed Mahmoud (El-Geish)
> On May 4, 2015, 8:14 p.m., Yi Pan (Data Infrastructure) wrote: > > samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala, > > line 320 > > > > > > This is a bit confusing to me:

Re: Review Request 33146: New KeyValueStore Features

2015-05-04 Thread Yi Pan (Data Infrastructure)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33146/#review82429 --- samza-test/src/main/scala/org/apache/samza/test/performance/TestKey

Re: Review Request 33146: New KeyValueStore Features

2015-05-03 Thread Mohamed Mahmoud (El-Geish)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33146/ --- (Updated May 4, 2015, 4:27 a.m.) Review request for samza. Changes --- A

Re: Review Request 33146: New KeyValueStore Features

2015-05-02 Thread Mohamed Mahmoud (El-Geish)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33146/ --- (Updated May 2, 2015, 10:04 a.m.) Review request for samza. Changes ---

Re: Review Request 33146: New KeyValueStore Features

2015-04-28 Thread Mohamed Mahmoud (El-Geish)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33146/#review81837 --- I'm sorry for the spam of emails, but please wait until I analyze th

Re: Review Request 33146: New KeyValueStore Features

2015-04-28 Thread Mohamed Mahmoud (El-Geish)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33146/ --- (Updated April 28, 2015, 10:50 a.m.) Review request for samza. Changes --

Re: Review Request 33146: New KeyValueStore Features

2015-04-28 Thread Mohamed Mahmoud (El-Geish)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33146/ --- (Updated April 28, 2015, 10:20 a.m.) Review request for samza. Changes --

Re: Review Request 33146: New KeyValueStore Features

2015-04-28 Thread Mohamed Mahmoud (El-Geish)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33146/#review81774 --- samza-kv/src/main/scala/org/apache/samza/storage/kv/SerializedKeyVa

Re: Review Request 33146: New KeyValueStore Features

2015-04-26 Thread Yi Pan (Data Infrastructure)
> On April 24, 2015, 5:01 p.m., Yi Pan (Data Infrastructure) wrote: > > Ship It! > > Mohamed Mahmoud (El-Geish) wrote: > I don't have access to commit. Can you please grant me access or commit > for me? Thanks! Hi, MOhamed, I was trying to go through all the tests w/ your patch. After all

Re: Review Request 33146: New KeyValueStore Features

2015-04-26 Thread Mohamed Mahmoud (El-Geish)
> On April 24, 2015, 5:01 p.m., Yi Pan (Data Infrastructure) wrote: > > Ship It! I don't have access to commit. Can you please grant me access or commit for me? Thanks! - Mohamed --- This is an automatically generated e-mail. To reply,

Re: Review Request 33146: New KeyValueStore Features

2015-04-24 Thread Yi Pan (Data Infrastructure)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33146/#review81497 --- Ship it! Ship It! - Yi Pan (Data Infrastructure) On April 24, 20

Re: Review Request 33146: New KeyValueStore Features

2015-04-24 Thread Mohamed Mahmoud (El-Geish)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33146/ --- (Updated April 24, 2015, 4:59 p.m.) Review request for samza. Changes ---

Re: Review Request 33146: New KeyValueStore Features

2015-04-23 Thread Mohamed Mahmoud (El-Geish)
> On April 23, 2015, 8:04 p.m., Yi Pan (Data Infrastructure) wrote: > > LGTM. Thanks! Thank you! - Mohamed --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33146/#review81381 -

Re: Review Request 33146: New KeyValueStore Features

2015-04-23 Thread Yi Pan (Data Infrastructure)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33146/#review81381 --- Ship it! LGTM. Thanks! - Yi Pan (Data Infrastructure) On April 2

Re: Review Request 33146: New KeyValueStore Features

2015-04-23 Thread Mohamed Mahmoud (El-Geish)
> On April 23, 2015, 6:09 p.m., Naveen Somasundaram wrote: > > samza-kv/src/main/java/org/apache/samza/storage/kv/BatchingKeyValueStore.java, > > line 52 > > > > > > Can you please document this? as an end-user, it wou

Re: Review Request 33146: New KeyValueStore Features

2015-04-23 Thread Naveen Somasundaram
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33146/#review81351 --- samza-kv/src/main/java/org/apache/samza/storage/kv/BatchingKeyValue

Re: Review Request 33146: New KeyValueStore Features

2015-04-23 Thread Mohamed Mahmoud (El-Geish)
> On April 21, 2015, 6:49 p.m., Yi Pan (Data Infrastructure) wrote: > > samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala, > > line 106 > > > > > > From the put() method below, nul

Re: Review Request 33146: New KeyValueStore Features

2015-04-23 Thread Mohamed Mahmoud (El-Geish)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33146/ --- (Updated April 23, 2015, 7:44 a.m.) Review request for samza. Changes ---

Re: Review Request 33146: New KeyValueStore Features

2015-04-23 Thread Mohamed Mahmoud (El-Geish)
> On April 14, 2015, 9:14 p.m., Chris Riccomini wrote: > > I'm concerned that there might be an issue with this approach. In > > BaseKeyValueStorageEngineFactory, we compose stores by nesting them. If > > this is the case, I think that the top-most store will implement the > > batching key val

Re: Review Request 33146: New KeyValueStore Features

2015-04-23 Thread Mohamed Mahmoud (El-Geish)
> On April 14, 2015, 9:43 p.m., Naveen Somasundaram wrote: > > samza-kv/src/main/java/org/apache/samza/storage/kv/BatchingKeyValueStore.java, > > line 52 > > > > > > What's the significance of the null check ? why not

Re: Review Request 33146: New KeyValueStore Features

2015-04-23 Thread Mohamed Mahmoud (El-Geish)
> On April 14, 2015, 9:16 p.m., Chris Riccomini wrote: > > samza-test/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala, > > line 26 > > > > > > Prefer not to introduce a dependency on google common.

Re: Review Request 33146: New KeyValueStore Features

2015-04-21 Thread Yi Pan (Data Infrastructure)
> On April 21, 2015, 6:49 p.m., Yi Pan (Data Infrastructure) wrote: > > samza-kv/src/main/java/org/apache/samza/storage/kv/KeyValueStore.java, line > > 33 > > > > > > The signature of close() and flush() functions from

Re: Review Request 33146: New KeyValueStore Features

2015-04-21 Thread Yi Pan (Data Infrastructure)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33146/#review81004 --- samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDb

Re: Review Request 33146: New KeyValueStore Features

2015-04-16 Thread Mohamed Mahmoud (El-Geish)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33146/ --- (Updated April 16, 2015, 10:43 a.m.) Review request for samza. Changes --